[systemd-devel] [v1] shutdown: add kexec loading, ?avoid calling `kexec` binary unnessecarily

[Shawn Landden]

On Thu, Feb 26, 2015 at 6:22 PM, Zbigniew Jędrzejewski-Szmek <
zbyszek at in.waw.pl> wrote:

> On Thu, Feb 26, 2015 at 08:04:08AM +0000, Jan Janssen wrote:
> > Shawn Landden <shawn <at> churchofgit.com> writes:
> >
> > >  void strv_free(char **l) {
> > > -        strv_clear(l);
> > > +        char **k;
> > > +
> > > +        if (!l)
> > > +                return;
> > > +
> > > +        for (k = l; *k; k++)
> > > +                free(*k);
> > > +
> > >          free(l);
> > >  }
> > What are you trying to achieve here? I see no point in optimizing out
> the *l
> > = NULL from strv_clear.
> >
> > > +                                entry->linux_loc  = l + strspn(l,
> > WHITESPACE);
> > > +                        else if ((l = startswith(m, "initrd ")))
> > > +                                entry->initrd     = l + strspn(l,
> > WHITESPACE);
> > You need to support more than one initrd per kernel, see
> > https://wiki.archlinux.org/index.php/Microcode for why. Also, I am
> pretty
> > sure you can have a initrd=/path/to/initrd in the kernel options entry.
> > Since the efi bootloader just appends each given initrd to the kernel
> > command line.
> >
> >
> > All in all I am wondering why you need a rbtree for all this in the first
> > place? A simple hashmap should do just fine.
> A hashmap does not keep order. But a simple array + qsort_safe() should
> work too. I'm wary of introducing yet another data structure into systemd
> which raises the bar for people editing the code later on or making
> changes.
>
> We need two operations: sorting kernels to list them, and picking (I
> presume) the
> latest kernel or the kernel with the given version. Both of those
> operations
> are done once over the lifetime of the program, so any speedup in using
> a data structure should take into account the time to set up the structure.
> Neither of those operations is speed sensitive, and the more common
> operation
> of picking a specific version can be done in O(n) over an array. So using
> an rbtree will not save any time actually.
>
I was initially using a vector of pointers here for the same reasons you
reiterated, but I felt the use of greedy_realloc0() was messy and
error-prone. The rbtree does not require the use of realloc(). There is no
way to know how long the array needs to be from the start. Even the O(n)
you mention could be turned into O(logn) by using a binary search.

>
> > Also, you're not taking multi-boot into account (the machine-id field).
> > You're just discriminating based on the kernel version, but different
> > installations could have the same version field.
> This high-level design questions should probably be addressed first...
>
> Zbyszek
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20150227/f01153de/attachment-0001.html>