Re: [PATCH] module: Use binary search in lookup_symbol()

[Rusty Russell <rusty@rustcorp.com.au>]

On Tue, 17 May 2011 16:22:41 -0700, Greg KH <greg@kroah.com> wrote:
> On Tue, May 17, 2011 at 10:56:03PM +0200, Alessio Igor Bogani wrote:
> > This work was supported by a hardware donation from the CE Linux Forum.
> > 
> > Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
> > ---
> 
> That's nice, but _why_ do this change?  What does it buy us?
> 
> Please explain why you make a change, not just who sponsored the change,
> that's not very interesting to developers.

I was going to let this pass, but since Greg flagged it...

It's sufficient given the context (it's the tail end of a series of
patches), but it's preferable to allude to the other patches in a case
like this.  For example:

        Now we have sorted symbols, we can use binary search for
        kallsyms lookups as well.

(1) "Now we have sorted symbols" indicates to the reader that this has
    just recently become possible.
(2) "as well." indicates that this was not the main justification for
    sorting the symbols.
 
Ideally you would add some numbers, like so:

        On my machine 'cat /proc/kallsyms' only takes 0.02 seconds, but
        this halves it to 0.01 seconds.

(That's my results under kvm, which is a poor way to do timing, but you
get the idea).

Thanks,
Rusty.