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

[Shawn Landden]

On Fri, Feb 27, 2015 at 9:03 AM, Zbigniew Jędrzejewski-Szmek <
zbyszek at in.waw.pl> wrote:
> > We need two operations: sorting kernels to list them, and picking (I

> 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.
> > >
> > > 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.
>
Oh I see, you don't have to sort in that case. But the code would be
longer, so perhaps I will sort in all cases.

>
> 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/fab76f1c/attachment.html>