Message-ID: <CANpvso49z0cbfioOV05RXCnaw=zXW5PGbvUa=V3WUd5mxtSGRg@mail.gmail.com>
Date:	Tue, 22 Sep 2015 19:18:21 +0100
From:	Eric Curtin <ericcurtin17@...il.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Valentina Manea <valentina.manea.m@...il.com>,
	Shuah Khan <shuah.kh@...sung.com>,
	USB list <linux-usb@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: First kernel patch (optimization)

On 22 September 2015 at 18:38, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Tue, Sep 15, 2015 at 12:53 PM, Eric Curtin <ericcurtin17@...il.com> wrote:
>> My first kernel patch, hope I did everything correctly! Instead of calling strlen on every iteration of the for loop, just call it once instead and store in a variable.
>
> Heh. Ok, that resulted in a rather long email thread.
>
> Anyway, I'd actually prefer to merge this patch, for two reasons:
>
>  - the "termination calculation is expensive" problem is a real
> problem, and while in this case the compiler may be able to notice
> that the "strlen()" is constant over the loop and can be hoisted up,
> that is not at all necessarily the case most of the time.
>
>     So I actually think patches like this are good things. Not because
> this particular code site necessarily matters, but because people who
> write code with things like "strlen()" in the terminating condition
> need to learn that it's *wrong*.
>
>  - I'd much rather see this kind of trivial patch than the usual
> trivial patch that is clearly just "let's run some script on the
> kernel and fix up warnings it generates mindlessly".
>
>    In contrast to that, *this* trivial patch was about somebody who
> thought about code generation and efficiency. And *that* is the kind
> of trivial patches we want to encourage, not necessariyl because this
> particular case was so important, but because that's the kind of
> people and thinking we want to encourage.
>
> So quite frankly, I'd just take this directly, but I'd like more of
> real changelog.
>
> But I wonder if Eric is even reading the emails any more ;)
>
>                  Linus

Hi Linus & Everyone else,

I can deal with the patronizing remarks made at the start of the
thread, I've been called worse names! But I would encourage people to
avoid this behavior especially with newbies, as many may leave the
community and never attempt to submit a patch again. It does not leave
a great first impression.

We so not come out the womb expert kernel developers, you gotta learn
how to crawl, before you can walk.

Yeah, I'm still reading this email thread and learned lots from it.
I'm working on something more meaningful, but it's not going to be
ground breaking of course, there is a led on my capslock key on a new
machine I won at work that does not switch off properly after it is
switched on. I think it is something to do with the LED_CAPSL variable
in here:

drivers/hid/usbhid/usbkbd.c

I'll figure it out, and get it fixed. Decided, I'd do something more
meaty. Then I'll start looking at the drivers/staging stuff. I can
only look at this stuff for a few hours every second day, with my
normal 9 to 5 job being first priority.

I can clean up this patch re-submit no problem, in fact I already did
and got more feedback, it's subject is "tools: usbip: detach: avoid
calling strlen() at each iteration". But I moved on to the keyboard
driver because of the feedback I received about wasting peoples time!

Regards,

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists