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

Zbigniew Jędrzejewski-Szmek zbyszek at in.waw.pl
Thu Feb 26 18:22:28 PST 2015


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.

> 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


More information about the systemd-devel mailing list