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

[Zbigniew Jędrzejewski-Szmek]

On Fri, Feb 27, 2015 at 08:58:04AM -0800, Shawn Landden wrote:
> 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.
greedy_realloc0() is not that messy. And it would be just a few lines
of code. We have similar patterns in many other places, and
consistency is good.

> 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.
Nope. The array needs to be sorted to do a binary search. So the
upfront cost of sorting kills any gain you get later on.

Zbyszek