Re: First kernel patch (optimization)

[Raymond Jennings <shentino@gmail.com>]

On 09/18/15 00:42, Greg KH wrote:
> On Thu, Sep 17, 2015 at 11:12:51PM -0400, Theodore Ts'o wrote:
>> On Wed, Sep 16, 2015 at 01:26:51PM -0400, Josh Boyer wrote:
>>> That isn't true.  It helps the submitter understand the workflow and
>>> expectations.  What you meant to say is that it doesn't help you.
>> The problem is that workflow isn't the hard part.  It's the part that
>> can be taught most easily, sure.  But people seem to get really hung
>> up on it, and I fear that we have people who never progress beyond
>> sending trivial patches and spelling fixes and white space fixes and
>> micro-optimizations.
>>
>> If the "you too can be a kernel developer" classes and web sites and
>> tutorials also taught people how to take performance measurements, and
>> something about the scientific measurement, that would be something.
>> Or if it taught people how to create tests and to run regression
>> testing.  Or if it taught people how to try to do fuzz testing, and
>> then once they find a sequence which causes crash, how to narrow down
>> the failure to a specific part of the kernel, and how to fix and
>> confirm that the kernel no longer crashes with the fix --- that would
>> be useful.
>>
>> If they can understand kernel code; if they can understand the
>> scientific measurement; if they can understand how to do performance
>> measurements --- being able to properly format patches is something
>> which most kernel developers can very easily guide a new contributor
>> to do correctly.  Or in the worst case, it doesn't take much time for
>> me to fix a whitespace problem and just tell the contributor --- by
>> the way, I fixed up this minor issue; could you please make sure you
>> do this in the future?
>>
>> But if a test hasn't been tested, or if the contributor things it's a
>> micro-optimization, but it actually takes more CPU time and/or more
>> stack space and/or bloats the kernel --- that's much more work for the
>> kernel maintainer to have to deal with when reviewing a patch.
>>
>> So I have a very strong disagreement with the belief that teaching
>> people the workflow is the more important thing.  In my mind, that's
>> like first focusing on the proper how to properly fill out a golf
>> score card, and the ettiquette and traditions around handicaps, etc
>> --- before making sure the prospective player is good at putting and
>> driving.  Personally, I'm terrible at putting and driving, so spending
>> a lot of time learning how to fill out a golf score card would be a
>> waste of my time.
>>
>> A good kernel programmer has to understand systems thinking; how to
>> figure out abstractions and when it's a good thing to add a new layer
>> of abstraction and when it's better to rework an exsting abstraction
>> layer.
>>
>> If we have someone who knows the workflow, but which doesn't
>> understand systems thinking, or how to do testing, then what?  Great,
>> we've just created another Nick Krause.  Do you think encouraging a
>> Nick Krause helps anyone?
>>
>> If people really are hung up on learning the workflow, I don't mind if
>> they want to learn that part and send some silly micro-optimization or
>> spelling fix or whitespace fix.  But it's really, really important
>> that they move beyond that.  And if they aren't capable of moving
>> beyond that, trying to inflate are recruitment numbers by encouraging
>> someone who can only do trivial fixes means that we may be get what we
>> can easily measure --- but it may not be what we really need as a
>> community.
> Ted, you are full of crap.
>
> Where do you think that "new developers" come from?  Do they show up in
> our inbox, with full knowledge of kernel internals and OS theory yet
> they somehow just can't grasp how to submit a patch correctly?  Yes,
> they sometimes rarely do.  But for the majority of people who got into
> Linux, that is not the case at all.
>
> People need to start with something simple, and easy, to get over the
> hurdles of:
> 	- generating a patch
> 	- sending an email
> 	- fixing the email client as it just corrupted the patch
> 	- fix the subject line as it was incorrect
> 	- fix the changelog as it was missing
> 	- fix the email client again as it corrupted the patch in a
> 	  different way
> 	- giving up on using a web email client as it just will not work
> 	- figuring out who to send the patch to
> 	- fixing the email client as the mailing list bounced the email
>
> Those are non-trivial tasks.  And by starting with "remove this space"
> you take the worry away from the specific content of the patch, and let
> them worry about the "hard" part first.
+1 for this.

For example, I for one cannot tell you how many times gmail snuck html 
sections into my outgoing emails before I finally caught it red handed 
and switched to using a local native client.

> Then, after all of the above is finished, and working, then they can
> start submitting real patches, that do real things, in patch series, as
> they can focus on the content much more, as the problems of how to make
> the patch into an acceptable format is not an issue anymore.

Did anyone read linus torvald's post that I linked to earlier?

It was very much relevant to the present situation and covers something 
like this that happened before with a newbie developer.

> I see this every single day with patches in the staging tree, which is
> why it is there, and is why I don't just go and remove all coding style
> errors in one fell-swoop tomorrow.  We need to provide those "simple"
> tasks to get people involved in our community and over the hurdle of
> sending a patch in.
>
> And from there, then people go on to contribute "real" things.  There
> are _many_ subsystem maintainers that started out submitting whitespace
> patches.  There are even more developers who have gotten "real" jobs by
> starting out submitting whitespace patches and personally I find that a
> much more satisfying metric than more subsystem maintainers.
>
> Yes, not everyone who sends in cleanup patches will go on to be a
> maintainer, or get a job, but the thing is, you can't judge who will, or
> will not, be that person.  What we have to do, and what we must do, is
> accept everyone into our community as somewhere, someone is out there
> that you will want to take over for your subsystem when you are tired of
> it.  And by turning away people from doing things to get over our first
> hurdles by adding to it by forcing someone to make a "real"
> contribution, you just lost that person forever.
>
> So don't take cleanup patches for your code, that's fine, and I totally
> understand why _you_ don't want to do that.  But to blow off the effort
> as being somehow trivial and not worthy of us, that's totally missing
> the point, and making things harder on yourself in the end.
>
> Just point people at staging, that's what it is there for, and is what
> I, and the developers who help maintain it, do every single day because
> we know it is essential for the future of Linux to do so.
>
> And if they don't move on beyond whitespace cleanups, that's fine too,
> I'll still gladly take those patches, and do so.  But almost always the
> person moves from that into doing something "real" or they tire of it
> and stop contributing.  By somehow pre-rejecting these people, you are
> pre-judging them as well, a _VERY_ dangerous thing to do to anyone.
>
> Do you know what _my_ first email was to lkml all those years ago?  "How
> do you make a patch in the proper format?"  And I asked it because I saw
> a "trivial" change that I was able to make.  And do you know who
> answered that question?  Someone who ended up being my manager for many
> years, and now runs SuSE Labs.  He was wise enough to take the time to
> help a "newbie" like myself because he knew that we all started
> somewhere, and that we never know just who that "newbie" might turn out
> to be in the end.
>
> So don't be a jerk, and spout off as to how we only need "skilled"
> people contributing.  Of course we need that, but the way we get those
> people is to help them become skilled, not requiring them to show up
> with those skills automatically.
>
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/