Re: How to improve the quality of the kernel?

Re: How to improve the quality of the kernel?

[Al Boldi]

Bartlomiej Zolnierkiewicz wrote:
> On Sunday 17 June 2007, Andrew Morton wrote:
> > We of course do want to minimise the amount of overhead for each
> > developer. I'm a strong believer in specialisation: rather than
> > requiring that *every* developer/maintainer integrate new steps in their
> > processes it would be better to allow them to proceed in a
> > close-to-usual fashion and to provide for a specialist person (or team)
> > to do the sorts of things which you're thinking about.
>
> Makes sense... however we need to educate each and every developer about
> importance of the code review and proper recognition of reviewers.

That's as easy to manage as is currently done with rc-regressions.

Maybe Adrian can introduce a "Patch Review Tacking" system akin to the his 
"rc-Regression Tracking" system.


Thanks!

--
Al

Re: How to improve the quality of the kernel?

[Michal Piotrowski]

On 18/06/07, Al Boldi <a1426z@gawab.com> wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > On Sunday 17 June 2007, Andrew Morton wrote:
> > > We of course do want to minimise the amount of overhead for each
> > > developer. I'm a strong believer in specialisation: rather than
> > > requiring that *every* developer/maintainer integrate new steps in their
> > > processes it would be better to allow them to proceed in a
> > > close-to-usual fashion and to provide for a specialist person (or team)
> > > to do the sorts of things which you're thinking about.
> >
> > Makes sense... however we need to educate each and every developer about
> > importance of the code review and proper recognition of reviewers.
>
> That's as easy to manage as is currently done with rc-regressions.

Are you a volunteer?

It's not an easy task, there are more patches than regressions.

Regards,
Michal

-- 
LOG
http://www.stardust.webpages.pl/log/

Re: How to improve the quality of the kernel?

[Al Boldi]

Michal Piotrowski wrote:
> On 18/06/07, Al Boldi <a1426z@gawab.com> wrote:
> > Bartlomiej Zolnierkiewicz wrote:
> > > On Sunday 17 June 2007, Andrew Morton wrote:
> > > > We of course do want to minimise the amount of overhead for each
> > > > developer. I'm a strong believer in specialisation: rather than
> > > > requiring that *every* developer/maintainer integrate new steps in
> > > > their processes it would be better to allow them to proceed in a
> > > > close-to-usual fashion and to provide for a specialist person (or
> > > > team) to do the sorts of things which you're thinking about.
> > >
> > > Makes sense... however we need to educate each and every developer
> > > about importance of the code review and proper recognition of
> > > reviewers.
> >
> > That's as easy to manage as is currently done with rc-regressions.
>
> Are you a volunteer?

Probably not, as this task requires a real PRO!

> It's not an easy task, there are more patches than regressions.

I didn't say it was an easy task, and it probably involves a lot of stamina.

But the management aspect looks rather straight forward, yet rewarding.


Thanks!

--
Al

Re: How to improve the quality of the kernel?

[Adrian Bunk]

On Mon, Jun 18, 2007 at 06:55:47AM +0300, Al Boldi wrote:
> Michal Piotrowski wrote:
> > On 18/06/07, Al Boldi <a1426z@gawab.com> wrote:
> > > Bartlomiej Zolnierkiewicz wrote:
> > > > On Sunday 17 June 2007, Andrew Morton wrote:
> > > > > We of course do want to minimise the amount of overhead for each
> > > > > developer. I'm a strong believer in specialisation: rather than
> > > > > requiring that *every* developer/maintainer integrate new steps in
> > > > > their processes it would be better to allow them to proceed in a
> > > > > close-to-usual fashion and to provide for a specialist person (or
> > > > > team) to do the sorts of things which you're thinking about.
> > > >
> > > > Makes sense... however we need to educate each and every developer
> > > > about importance of the code review and proper recognition of
> > > > reviewers.
> > >
> > > That's as easy to manage as is currently done with rc-regressions.
> >
> > Are you a volunteer?
> 
> Probably not, as this task requires a real PRO!
>...

That's wrong.

We are talking about _tracking_.

I'm not sure whether it makes much sense, and it would cost an enormous 
amount of time, but tracking patches should be possible without any 
knowledge of the kernel.

> Thanks!
> Al

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: How to improve the quality of the kernel?

[Al Boldi]

Adrian Bunk wrote:
> On Mon, Jun 18, 2007 at 06:55:47AM +0300, Al Boldi wrote:
> > Michal Piotrowski wrote:
> > > On 18/06/07, Al Boldi <a1426z@gawab.com> wrote:
> > > > Bartlomiej Zolnierkiewicz wrote:
> > > > > On Sunday 17 June 2007, Andrew Morton wrote:
> > > > > > We of course do want to minimise the amount of overhead for each
> > > > > > developer. I'm a strong believer in specialisation: rather than
> > > > > > requiring that *every* developer/maintainer integrate new steps
> > > > > > in their processes it would be better to allow them to proceed
> > > > > > in a close-to-usual fashion and to provide for a specialist
> > > > > > person (or team) to do the sorts of things which you're thinking
> > > > > > about.
> > > > >
> > > > > Makes sense... however we need to educate each and every developer
> > > > > about importance of the code review and proper recognition of
> > > > > reviewers.
> > > >
> > > > That's as easy to manage as is currently done with rc-regressions.
> > >
> > > Are you a volunteer?
> >
> > Probably not, as this task requires a real PRO!
> >...
>
> That's wrong.
>
> We are talking about _tracking_.
>
> I'm not sure whether it makes much sense, and it would cost an enormous
> amount of time, but tracking patches should be possible without any
> knowledge of the kernel.

If that's really true, which I can't imagine, then the proper way forward 
would probably involve a fully automated system.


Thanks!

--
Al

Re: How to improve the quality of the kernel?

[Adrian Bunk]

On Thu, Jun 21, 2007 at 06:26:20AM +0300, Al Boldi wrote:
> Adrian Bunk wrote:
> > On Mon, Jun 18, 2007 at 06:55:47AM +0300, Al Boldi wrote:
> > > Michal Piotrowski wrote:
> > > > On 18/06/07, Al Boldi <a1426z@gawab.com> wrote:
> > > > > Bartlomiej Zolnierkiewicz wrote:
> > > > > > On Sunday 17 June 2007, Andrew Morton wrote:
> > > > > > > We of course do want to minimise the amount of overhead for each
> > > > > > > developer. I'm a strong believer in specialisation: rather than
> > > > > > > requiring that *every* developer/maintainer integrate new steps
> > > > > > > in their processes it would be better to allow them to proceed
> > > > > > > in a close-to-usual fashion and to provide for a specialist
> > > > > > > person (or team) to do the sorts of things which you're thinking
> > > > > > > about.
> > > > > >
> > > > > > Makes sense... however we need to educate each and every developer
> > > > > > about importance of the code review and proper recognition of
> > > > > > reviewers.
> > > > >
> > > > > That's as easy to manage as is currently done with rc-regressions.
> > > >
> > > > Are you a volunteer?
> > >
> > > Probably not, as this task requires a real PRO!
> > >...
> >
> > That's wrong.
> >
> > We are talking about _tracking_.
> >
> > I'm not sure whether it makes much sense, and it would cost an enormous
> > amount of time, but tracking patches should be possible without any
> > knowledge of the kernel.
> 
> If that's really true, which I can't imagine, then the proper way forward 
> would probably involve a fully automated system.

If you consider any kind of patch tracking valuable, you should either 
do it yourself or write the tool yourself. In both cases, the 
interesting parts would be how to integrate it into the workflow of 
kernel development without creating extra work for anyone and how to get 
the information into the got commits.

"requires a real PRO" and "would probably involve" sound like cheap 
phrases for avoiding doing any work yourself.

Talk is cheap, but unless YOU will do it your emails will only be a 
waste of bandwidth.

> Thanks!
> Al

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: How to improve the quality of the kernel?

[Al Boldi]

Adrian Bunk wrote:
> On Thu, Jun 21, 2007 at 06:26:20AM +0300, Al Boldi wrote:
> > Adrian Bunk wrote:
> > > We are talking about _tracking_.
> > >
> > > I'm not sure whether it makes much sense, and it would cost an
> > > enormous amount of time, but tracking patches should be possible
> > > without any knowledge of the kernel.
> >
> > If that's really true, which I can't imagine, then the proper way
> > forward would probably involve a fully automated system.
>
> If you consider any kind of patch tracking valuable, you should either
> do it yourself or write the tool yourself. In both cases, the
> interesting parts would be how to integrate it into the workflow of
> kernel development without creating extra work for anyone and how to get
> the information into the got commits.

Integration is the easy part, really.  Just filter all the patches from the 
mailing list into a patch-bin, then sort, categorize, and prioritize them, 
responding with a validation status to all parties involved.

And after that comes the Tracking part.

> "requires a real PRO" and "would probably involve" sound like cheap
> phrases for avoiding doing any work yourself.

I have learned from this list that premature involvement is 
counterproductive.

> Talk is cheap, but unless YOU will do it your emails will only be a
> waste of bandwidth.

Thanks, and good luck with involving people with this kind of response!

--
Al

Re: How to improve the quality of the kernel?

[Adrian Bunk]

On Thu, Jun 21, 2007 at 04:41:28PM +0300, Al Boldi wrote:
> Adrian Bunk wrote:
> > On Thu, Jun 21, 2007 at 06:26:20AM +0300, Al Boldi wrote:
> > > Adrian Bunk wrote:
> > > > We are talking about _tracking_.
> > > >
> > > > I'm not sure whether it makes much sense, and it would cost an
> > > > enormous amount of time, but tracking patches should be possible
> > > > without any knowledge of the kernel.
> > >
> > > If that's really true, which I can't imagine, then the proper way
> > > forward would probably involve a fully automated system.
> >
> > If you consider any kind of patch tracking valuable, you should either
> > do it yourself or write the tool yourself. In both cases, the
> > interesting parts would be how to integrate it into the workflow of
> > kernel development without creating extra work for anyone and how to get
> > the information into the got commits.
> 
> Integration is the easy part, really.  Just filter all the patches from the 
> mailing list into a patch-bin, then sort, categorize, and prioritize them, 
> responding with a validation status to all parties involved.
> 
> And after that comes the Tracking part.

Tracking shouldn't be much more than seeing what different threads are 
about the same patch and then do more or less the same as what you 
called "the easy part".

> > "requires a real PRO" and "would probably involve" sound like cheap
> > phrases for avoiding doing any work yourself.
> 
> I have learned from this list that premature involvement is 
> counterproductive.
> 
> > Talk is cheap, but unless YOU will do it your emails will only be a
> > waste of bandwidth.
> 
> Thanks, and good luck with involving people with this kind of response!

It's simply how kernel development works - not by talking but by doing 
the work.

Many people thought long-term maintainance for 2.6.16 wouldn't make sense.
And I didn't start long discussions whether we need regression tracking -
I simply did it.

These are things that simply happened because I thought they were 
important - and because I got my ass up to do them myself.

Don't expect anyone to jump up to do it only because of your talk.
YOU must offer something, and it will work if it's then accepted by 
people.

If you think what you have in mind is both doable and important just do it.
You will find out where the problems lie yourself.
You might be able to prove me and all other people who think it would 
not work wrong.
You might fail, e.g. because people will not adopt whatever you have in 
mind because they don't like it for some reason, but that's part of how 
development works, and you'll never know unless you try it.

> Al

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: How to improve the quality of the kernel?

[Stefan Richter]

Al Boldi wrote:
> Adrian Bunk wrote:
>> On Mon, Jun 18, 2007 at 06:55:47AM +0300, Al Boldi wrote:
>> > Michal Piotrowski wrote:
>> > > On 18/06/07, Al Boldi <a1426z@gawab.com> wrote:
>> > > > Bartlomiej Zolnierkiewicz wrote:

[on the tracking of review status of patches]

>> > > > > however we need to educate each and every developer
>> > > > > about importance of the code review and proper recognition of
>> > > > > reviewers.
>> > > >
>> > > > That's as easy to manage as is currently done with rc-regressions.
>> > >
>> > > Are you a volunteer?
>> >
>> > Probably not, as this task requires a real PRO!
>> >...
>>
>> That's wrong.
>>
>> We are talking about _tracking_.
>>
>> I'm not sure whether it makes much sense, and it would cost an enormous
>> amount of time, but tracking patches should be possible without any
>> knowledge of the kernel.

I suspect you are...

> If that's really true, which I can't imagine, then the proper way forward 
> would probably involve a fully automated system.

...both wrong --- because patches have varying requirements WRT review
and testing.

What you discuss here under the label "patch tracking" blends into, how
shall I call it, "patch handling" as done by maintainers.  Neither a
layman nor an automaton is able to
  1. measure required vs. accomplished review and testing of a patch,
  2. recruit reviewers and testers.

And IMO *these* two are the points where we typically fail.  We
occasionally underestimate the required amount of review and testing,
but more importantly, we are chronically short of reviewers and
partially of testers.  (Hmm, I think Adrian and one or another guy
already said as much.)

A "Reviewed-by" tag in a patch is not a simple hard fact.  Neither a
layman nor an automaton can draw appropriate conclusions from it.  That
doesn't mean I'm against such tags, on the contrary.  They may help us
to (a) look harder for review, (b) have a better picture of actual lack
of review, patch by patch, subsystem by subsystem, and (c) get more
volunteer reviewers by emphasizing the merits of code review.  Alas,
experience and broad knowledge in kernel development are certainly
prerequisites to become a good reviewer, so don't get high hopes that
reviewers will suddenly come in droves when we appropriately credit
their work.
-- 
Stefan Richter
-=====-=-=== -==- =-=-=
http://arcgraph.de/sr/

Re: How to improve the quality of the kernel?

[Al Boldi]

Adrian Bunk wrote:
> On Thu, Jun 21, 2007 at 04:41:28PM +0300, Al Boldi wrote:
> > Adrian Bunk wrote:
> > > Talk is cheap, but unless YOU will do it your emails will only be a
> > > waste of bandwidth.
> >
> > Thanks, and good luck with involving people with this kind of response!
>
> It's simply how kernel development works - not by talking but by doing
> the work.

This sounds like a brute-force approach, akin to hacking.

I think it's much more productive to analyze the problem and then design a 
solution accordingly.

> Many people thought long-term maintainance for 2.6.16 wouldn't make sense.
> And I didn't start long discussions whether we need regression tracking -
> I simply did it.

Maybe because you are a PRO.

> These are things that simply happened because I thought they were
> important - and because I got my ass up to do them myself.
>
> Don't expect anyone to jump up to do it only because of your talk.
> YOU must offer something, and it will work if it's then accepted by
> people.
>
> If you think what you have in mind is both doable and important just do
> it. You will find out where the problems lie yourself.
> You might be able to prove me and all other people who think it would
> not work wrong.
> You might fail, e.g. because people will not adopt whatever you have in
> mind because they don't like it for some reason, but that's part of how
> development works, and you'll never know unless you try it.


Thanks!

--
Al

Re: How to improve the quality of the kernel?

[Adrian Bunk]

On Fri, Jun 22, 2007 at 12:33:13AM +0300, Al Boldi wrote:
> Adrian Bunk wrote:
> > On Thu, Jun 21, 2007 at 04:41:28PM +0300, Al Boldi wrote:
> > > Adrian Bunk wrote:
> > > > Talk is cheap, but unless YOU will do it your emails will only be a
> > > > waste of bandwidth.
> > >
> > > Thanks, and good luck with involving people with this kind of response!
> >
> > It's simply how kernel development works - not by talking but by doing
> > the work.
> 
> This sounds like a brute-force approach, akin to hacking.
> 
> I think it's much more productive to analyze the problem and then design a 
> solution accordingly.

Sure, that's part of creating your solution.

But when you've analyzed the problem and designed a solution, YOU have 
to implement it.

> > Many people thought long-term maintainance for 2.6.16 wouldn't make sense.
> > And I didn't start long discussions whether we need regression tracking -
> > I simply did it.
> 
> Maybe because you are a PRO.
>...

No, simply because I got my ass up.

> Thanks!
> Al

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: How to improve the quality of the kernel?

[Oleg Verych]

On Fri, Jun 22, 2007 at 04:19:34PM +0200, Stefan Richter wrote:
> Markus Rechberger wrote:
> > just reading through it and cleaning up some code isn't so difficult
> > and can be done by many people -
> 
> Doing cleanups is a good way to get into the matter, to become able to
> eventually fix bugs of the difficult type.
> 
> > if they're allowed/wanted to do so.
> 
> Everybody is allowed to submit.  But there is a certain degree of both
> persistence and adaptability required to get one's first submissions
> upstream.  However, these qualities are also required to fix difficult bugs.

Deja-kernel.

Just two messages:
<http://permalink.gmane.org/gmane.linux.debian.devel.general/116453>
<http://permalink.gmane.org/gmane.linux.debian.devel.general/116463>

Tell me, i'm wrong, if similar thing cannot be implemented here.
Again, key word is _tracking_ system...

Just trying attract attention, that time of ignorance and manual work
must be ended. There must be new time, time of *tracking*, *counting*
opinions and any kernel work anybody want to contribute. I just got bored
after repeatings like, not funny work, code, etc. The manager, who will
do that not funny work is automated tracking system. Based on e-mail,
with additional tools, like

* ``reportbug''-- reporting (imroved REPORTING-BUGS,
  EVERY-WORK-IS-APPRECIATED-THANK-YOU)

* ``bts''-- command line interface, etc.

I want to change it, and i will try to work on that. Important thing is
-- to be in the corner *alone*, even with good, open source example system 
as Debian BTS is not gonna work.

WRT this, opinions and doings of people in this thread, who spend in
Linux development much more time, than i, just counter productive (fine,
fine but i have a right to have different, wrong opinion on that :).

--
  Frenzy
-o--=O`C
 #oo'L O
<___=E M

Re: How to improve the quality of the kernel?

[Stefan Richter]

Markus Rechberger wrote:
> just reading through it and cleaning up some code isn't so difficult
> and can be done by many people -

Doing cleanups is a good way to get into the matter, to become able to
eventually fix bugs of the difficult type.

> if they're allowed/wanted to do so.

Everybody is allowed to submit.  But there is a certain degree of both
persistence and adaptability required to get one's first submissions
upstream.  However, these qualities are also required to fix difficult bugs.
-- 
Stefan Richter
-=====-=-=== -==- =-==-
http://arcgraph.de/sr/

Re: How to improve the quality of the kernel?

[Markus Rechberger]

On 6/17/07, Natalie Protasevich <protasnb@gmail.com> wrote:
> On 6/17/07, Adrian Bunk <bunk@stusta.de> wrote:
> > On Sun, Jun 17, 2007 at 06:26:55PM +0200, Stefan Richter wrote:
> > > Adrian Bunk wrote:
> > > >>> And we should be aware that reverting is only a workaround for the
> real
> > > >>> problem which lies in our bug handling.
> > > >> ...
> > > >
> > > > And this is something I want to emphasize again.
> > > >
> > > > How can we make any progress with the real problem and not only the
> > > > symptoms?
> > > ...
> > >
> > > Perhaps make lists of
> > >
> > >   - bug reports which never lead to any debug activity
> > >     (no responsible person/team was found, or a seemingly person/team
> > >     did not start to debug the report)
> > >
> > >   - known regressions on release,
> > >
> > >   - regressions that became known after release,
> > >
> > >   - subsystems with notable backlogs of old bugs,
> > >
> > >   - other categories?
> > >
> > > Select typical cases from each categories, analyze what went wrong in
> > > these cases, and try to identify practicable countermeasures.
> >
> > No maintainer or no maintainer who is debugging bug reports is the
> > major problem in all parts of your list.
> >
> > > Another approach:  Figure out areas where quality is exemplary and try
> > > to draw conclusions for areas where quality is lacking.
> >
> > ieee1394 has a maintainer who is looking after all bug reports he gets.
> >
> > Conclusion: We need such maintainers for all parts of the kernel.
> >
>
> I noticed some areas are well maintained because there is an awesome
> maintainer, or good and well coordinated team - and this is mostly in
> the "fun" areas ;) But there are "boring" areas that are about to be
> deprecated or no new development expected etc. It will be hard to get
> a dedicated person to take care of such. How about having people on
> rotation, or jury duty so to speak - for a period of time (completely
> voluntary!) Nice stats on the report about contributions in non-native
> areas for a developer would be great accomplishment and also good
> chance to look into other things! Besides, this way "old parts" will
> get attention to be be revised and re-implemented sooner. And we can
> post "Temp maintainer needed" list...
>

I'd vote for that, I've seen alot very bad code already within some
subsystems and critical problems which just have been ignored by some
maintainers.
It mostly helps if some volunteers read through existing code and
state out their considerations about implementations which they don't
like.

I just grep'ed some examples I noticed (note I do not want to jump
onto someone's toe here, just give some examples):

(sn9c102_ov7660.c)
...
        err += sn9c102_i2c_write(cam, 0x12, 0x80);
        err += sn9c102_i2c_write(cam, 0x11, 0x09);
        err += sn9c102_i2c_write(cam, 0x00, 0x0A);
        err += sn9c102_i2c_write(cam, 0x01, 0x80);
        err += sn9c102_i2c_write(cam, 0x02, 0x80);
        err += sn9c102_i2c_write(cam, 0x03, 0x00);
... (around 150 lines directly after each other doing such writes and
adding error values to a variable, I don't understand why someone
should add the errors but continue with sending 150 more updates, how
about one write failed but others succeeded for any reason)

(tvp5150.c)
static int tvp5150_read(struct i2c_client *c, unsigned char addr)
{
        unsigned char buffer[1];
        int rc;

        buffer[0] = addr;
        if (1 != (rc = i2c_master_send(c, buffer, 1)))
                tvp5150_dbg(0, "i2c i/o error: rc == %d (should be 1)\n", rc);

        msleep(10);

        if (1 != (rc = i2c_master_recv(c, buffer, 1)))
                tvp5150_dbg(0, "i2c i/o error: rc == %d (should be 1)\n", rc);

        tvp5150_dbg(2, "tvp5150: read 0x%02x = 0x%02x\n", addr, buffer[0]);

        return (buffer[0]);
}

(i2c issues within some driver)
        /* This code detects calls by card attach_inform */
        if (NULL == t->i2c.dev.driver) {
                tuner_dbg ("tuner 0x%02x: called during i2c_client
register by adapter's attach_inform\n", c->addr);

                return;
        }
... that code doesn't even work anymore since the i2c.dev.driver is
always initialized.

just reading through it and cleaning up some code isn't so difficult
and can be done by many people - if they're allowed/wanted to do so.

Markus

Re: How to improve the quality of the kernel?

[Stefan Richter]

Natalie Protasevich wrote:
> On 6/18/07, Fortier,Vincent [Montreal] <Vincent.Fortier1@ec.gc.ca> wrote:
[...]
>> In the end plenty of statistics and hardware compatibility list
>> could be made.  For example, that would make my life easier knowing
>> what level of compatibility Linux can offer for old HP9000 K-boxes
>> that we still have running at the office and presumably get people
>> to contact to get help?
> 
> This is definitely something that can be done (and should) - well,
> especially having ability search by certain criteria - then all sorts
> of statistics and databases can be created.

Hardware Compatibility Lists/ Databases already exist, for driver
subsystems, for distributions...

Some issues with those databases are:

  - Users typically can only test one specific combination of a
    hardware collection and software collection, at one or a few points
    in time.

  - Users have difficulties or don't have the means to identify chip
    revisions, used protocols etc.

  - The databases are typically not conceived to serve additional
    purposes like bidirectional contact between developer and user.

These issues notwithstanding, these databases are already highly useful
both for endusers and for developers.  That's why they exist.

> Everything that helps  to find a way to work on a patch and to test
> easier should be done to make bug fixing easier and even possible.
> Often times the most knowledgeable people are not able to make quick
> fix just because there is no way to reproduce the case or get access
> to HW.

As has been mentioned elsewhere in the thread,

  - bug---hardware associations are sometimes difficult or impossible
    to make.  For example, the x86-64 platform maintainers are bothered
    with "x86-64 bugs" which turn out to be driver bugs on all
    platforms.

    (We want details descriptions of the hardware environment in a bug
    report, but this means we must be able to handle the flood of
    false positives in bug---hardware associations, i.e. successively
    narrow down which parts of the hardware/software combo are actually
    affected, and what other combinations could be affected too.)

  - Patch---hardware associations, especially for preemptive regression
    tests, are virtually impossible to make.  Murphy says that the
    regression will hit hardware which the patch submitter or forwarder
    thought could never be affected by the patch.

Of course, /sensible/ patch---hardware associations are (1) to try out
fixes for known issues with a specific hardware, (2) to test that a
cleanup patch or refactoring patch or API changing patch to a driver of
very specific hardware ( = a single type or few types with little
variance) does not introduce regressions for this hardware.
-- 
Stefan Richter
-=====-=-=== -==- =--==
http://arcgraph.de/sr/

Re: How to improve the quality of the kernel?

[Natalie Protasevich]

On 6/18/07, Fortier,Vincent [Montreal] <Vincent.Fortier1@ec.gc.ca> wrote:
> > -----Message d'origine-----
> > De : Natalie Protasevich [mailto:protasnb@gmail.com]
> > Envoyé : 18 juin 2007 18:56
> >
> > On 6/18/07, Martin Bligh <mbligh@mbligh.org> wrote:
> > > >> > So if you make changes to random-driver.c you can do `git-log
> > > >> > random-driver.c|grep Tested-by" to find people who can test your
> > > >> > changes for you.
> > > >>
> > > >> You would'nt even need to search in GIT.  Maybie even when ever a
> > > >> patchset is being proposed a mail could be sent to appropriate
> > > >> hardware/or feature pseudo-auto-generated mailing-list?
> > > >>
> > > >> On lkml I mostly try to follow patches/bugs associated with
> > > >> hardware I use.  Why not try to automate the process and get more testers in?
> > > >>
> > > >
> > > > I think this is an excellent point. One data point could be a field
> > > > in bugzilla to input the hardware information. Simple query can
> > > > select common hardware and platform. So far it's not working when
> > > > hardware is just mentioned in the text part.
> > >
> > > if it's free text it'll be useless for search ... I suppose we could
> > > do drop-downs for architecture at least? Not sure much beyond that
> > > would work ... *possibly* the common drivers, but I don't think we'd
> > > get enough coverage for it to be of use.
> > >
> > How about several buckets for model/BIOS version/chipset
> > etc., at least optional, and some will be relevant some not
> > for particular cases. But at least people will make an
> > attempt to collect such data from their system, boards, etc.
>
> How about an easy way to send multiple hardware profiles to your bugzilla user account simultaniously linked to an online pciutils database and/or an hardware list database similar to overclocking web sites and why not even with a link to the git repository when possible?
>
> A some sort of really usefull "send your profile" of RHN that would link the driver with the discovered hardware and add you to appropriate mailing lists to test patches/help reproducing & solving problems/etc.
>
> In the end plenty of statistics and hardware compatibility list could be made.  For example, that would make my life easier knowing what level of compatibility Linux can offer for old HP9000 K-boxes that we still have running at the office and presumably get people to contact to get help?

This is definitely something that can be done (and should) - well,
especially having ability search by certain criteria - then all sorts
of statistics and databases can be created.
Everything that helps  to find a way to work on a patch and to test
easier should be done to make bug fixing easier and even possible.
Often times the most knowledgeable people are not able to make quick
fix just because there is no way to reproduce the case or get access
to HW.

--Natalie

RE: How to improve the quality of the kernel?

[Fortier,Vincent [Montreal]]

> -----Message d'origine-----
> De : Natalie Protasevich [mailto:protasnb@gmail.com] 
> Envoyé : 18 juin 2007 18:56
> 
> On 6/18/07, Martin Bligh <mbligh@mbligh.org> wrote:
> > >> > So if you make changes to random-driver.c you can do `git-log 
> > >> > random-driver.c|grep Tested-by" to find people who can test your 
> > >> > changes for you.
> > >>
> > >> You would'nt even need to search in GIT.  Maybie even when ever a 
> > >> patchset is being proposed a mail could be sent to appropriate 
> > >> hardware/or feature pseudo-auto-generated mailing-list?
> > >>
> > >> On lkml I mostly try to follow patches/bugs associated with 
> > >> hardware I use.  Why not try to automate the process and get more testers in?
> > >>
> > >
> > > I think this is an excellent point. One data point could be a field 
> > > in bugzilla to input the hardware information. Simple query can 
> > > select common hardware and platform. So far it's not working when 
> > > hardware is just mentioned in the text part.
> >
> > if it's free text it'll be useless for search ... I suppose we could 
> > do drop-downs for architecture at least? Not sure much beyond that 
> > would work ... *possibly* the common drivers, but I don't think we'd 
> > get enough coverage for it to be of use.
> >
> How about several buckets for model/BIOS version/chipset 
> etc., at least optional, and some will be relevant some not 
> for particular cases. But at least people will make an 
> attempt to collect such data from their system, boards, etc.

How about an easy way to send multiple hardware profiles to your bugzilla user account simultaniously linked to an online pciutils database and/or an hardware list database similar to overclocking web sites and why not even with a link to the git repository when possible?

A some sort of really usefull "send your profile" of RHN that would link the driver with the discovered hardware and add you to appropriate mailing lists to test patches/help reproducing & solving problems/etc.

In the end plenty of statistics and hardware compatibility list could be made.  For example, that would make my life easier knowing what level of compatibility Linux can offer for old HP9000 K-boxes that we still have running at the office and presumably get people to contact to get help?

- vin

Re: How to improve the quality of the kernel?

[Martin Bligh]

>> > Maybe searching free text fields can then be implemented.  Then every
>> > message exchange in bugzilla can be used for extracting such info -
>> > questions about HW specifics are asked a lot, almost in every one.
>> > It's a shame we cant' use this information. I was once searching for
>> > "VIA" and got "zero bugs found", but in reality there are hundreds!
>> > Probably something that makes sense to bring up with bugzilla project?
>>
>> That should work now ... seems to for me.
>>
>> http://bugzilla.kernel.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&long_desc_type=substring&long_desc=VIA&kernel_version_type=allwordssubstr&kernel_version=&bug_status=NEW&bug_status=REOPENED&bug_status=ASSIGNED&bug_status=RESOLVED&bug_status=VERIFIED&bug_status=REJECTED&bug_status=DEFERRED&bug_status=NEEDINFO&bug_status=CLOSED&emailassigned_to1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&bugidtype=include&bug_id=&chfieldfrom=&chfieldto=Now&chfieldvalue=&regression=both&cmdtype=doit&order=Bug+Number&field0-0-0=noop&type0-0-0=noop&value0-0-0= 
>>
>>
>> Produces a metric-buttload of results. Go to the advanced search option
>> and do "A Comment" contains the string "VIA". By default "Status" is
>> only set to do open bugs, which you might want to change to all types.
> 
> Yes it works great! Thanks... I'd say this should be really a default
> search, because first search screen is misleading - it promises find
> all for any "words" :)

OK, or at the very least we can fix the text at least to indicate it'll
only search summaries (and likely only of open bugs at that ...)

M.

Re: How to improve the quality of the kernel?

[Natalie Protasevich]

On 6/18/07, Martin Bligh <mbligh@mbligh.org> wrote:
> > Sure, simplicity is a key - but most of reporters on bugs are pretty
> > professional folks (or very well rounded amateurs :) We can try still
> > why not? the worst that can happen will be empty fields.
>
> mmm. added complexity and interface clutter for little or no benefit
> is what I'm trying to avoid - they did that in the IBM bugzilla and
> it turned into a big ugly unusable monster. You can call me either
> "experienced" or "bitter" depending what mood you're in ;-)
>
> Not sure I'd agree that most of the bug submitters are all that
> professional, it's a very mixed bag.
>
> > Maybe searching free text fields can then be implemented.  Then every
> > message exchange in bugzilla can be used for extracting such info -
> > questions about HW specifics are asked a lot, almost in every one.
> > It's a shame we cant' use this information. I was once searching for
> > "VIA" and got "zero bugs found", but in reality there are hundreds!
> > Probably something that makes sense to bring up with bugzilla project?
>
> That should work now ... seems to for me.
>
> http://bugzilla.kernel.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&long_desc_type=substring&long_desc=VIA&kernel_version_type=allwordssubstr&kernel_version=&bug_status=NEW&bug_status=REOPENED&bug_status=ASSIGNED&bug_status=RESOLVED&bug_status=VERIFIED&bug_status=REJECTED&bug_status=DEFERRED&bug_status=NEEDINFO&bug_status=CLOSED&emailassigned_to1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&bugidtype=include&bug_id=&chfieldfrom=&chfieldto=Now&chfieldvalue=&regression=both&cmdtype=doit&order=Bug+Number&field0-0-0=noop&type0-0-0=noop&value0-0-0=
>
> Produces a metric-buttload of results. Go to the advanced search option
> and do "A Comment" contains the string "VIA". By default "Status" is
> only set to do open bugs, which you might want to change to all types.

Yes it works great! Thanks... I'd say this should be really a default
search, because first search screen is misleading - it promises find
all for any "words" :)

>
> > However, I've been working with other bugzillas (have to admit they
> > were mostly company/corporate), where this was a required field that
> > didn't seem to cause difficulties. I am planning to do some more
> > research and get some more ideas from other bugzillas. I suppose we
> > can have them discussed and revised sometime.
>
> Would be good, thanks. I tend to favour keeping things as simple as
> possible though, we have very little control over our users, and they're
> a very broad base. Making the barrier to entry for use as low as
> possible is the design we've been pursuing.

Actually, as long as search above is possible - it is going to work.

I must say the new bugzilla interface is very nice in general, and
well designed and easy to use.

--Natalie

Re: How to improve the quality of the kernel?

[Martin Bligh]

> Sure, simplicity is a key - but most of reporters on bugs are pretty
> professional folks (or very well rounded amateurs :) We can try still
> why not? the worst that can happen will be empty fields.

mmm. added complexity and interface clutter for little or no benefit
is what I'm trying to avoid - they did that in the IBM bugzilla and
it turned into a big ugly unusable monster. You can call me either
"experienced" or "bitter" depending what mood you're in ;-)

Not sure I'd agree that most of the bug submitters are all that
professional, it's a very mixed bag.

> Maybe searching free text fields can then be implemented.  Then every
> message exchange in bugzilla can be used for extracting such info -
> questions about HW specifics are asked a lot, almost in every one.
> It's a shame we cant' use this information. I was once searching for
> "VIA" and got "zero bugs found", but in reality there are hundreds!
> Probably something that makes sense to bring up with bugzilla project?

That should work now ... seems to for me.

http://bugzilla.kernel.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&long_desc_type=substring&long_desc=VIA&kernel_version_type=allwordssubstr&kernel_version=&bug_status=NEW&bug_status=REOPENED&bug_status=ASSIGNED&bug_status=RESOLVED&bug_status=VERIFIED&bug_status=REJECTED&bug_status=DEFERRED&bug_status=NEEDINFO&bug_status=CLOSED&emailassigned_to1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&bugidtype=include&bug_id=&chfieldfrom=&chfieldto=Now&chfieldvalue=&regression=both&cmdtype=doit&order=Bug+Number&field0-0-0=noop&type0-0-0=noop&value0-0-0=

Produces a metric-buttload of results. Go to the advanced search option
and do "A Comment" contains the string "VIA". By default "Status" is
only set to do open bugs, which you might want to change to all types.

> However, I've been working with other bugzillas (have to admit they
> were mostly company/corporate), where this was a required field that
> didn't seem to cause difficulties. I am planning to do some more
> research and get some more ideas from other bugzillas. I suppose we
> can have them discussed and revised sometime.

Would be good, thanks. I tend to favour keeping things as simple as
possible though, we have very little control over our users, and they're
a very broad base. Making the barrier to entry for use as low as
possible is the design we've been pursuing.

M.

Re: How to improve the quality of the kernel?

[Natalie Protasevich]

On 6/18/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Mon, 18 Jun 2007, Martin Bligh wrote:
> >
> > Sorry to be a wet blanket, but I've seen those sort of things
> > before, and they just don't seem to work, especially in the
> > environment we're in with such a massive diversity of hardware.
>
> I do agree. It _sounds_ like a great idea to try to control the flow of
> patches better, but in the end, it needs to also be easy and painfree to
> the people involved, and also make sure that any added workflow doesn't
> require even *more* load and expertise on the already often overworked
> maintainers..
>
> In many cases, I think it tends to *sound* great to try to avoid
> regressions in the first place - but it also sounds like one of those "I
> wish the world didn't work the way it did" kind of things. A worthy goal,
> but not necessarily one that is compatible with reality.
>
>                         Linus
>

Sure, simplicity is a key - but most of reporters on bugs are pretty
professional folks (or very well rounded amateurs :) We can try still
why not? the worst that can happen will be empty fields.

Maybe searching free text fields can then be implemented.  Then every
message exchange in bugzilla can be used for extracting such info -
questions about HW specifics are asked a lot, almost in every one.
It's a shame we cant' use this information. I was once searching for
"VIA" and got "zero bugs found", but in reality there are hundreds!
Probably something that makes sense to bring up with bugzilla project?

However, I've been working with other bugzillas (have to admit they
were mostly company/corporate), where this was a required field that
didn't seem to cause difficulties. I am planning to do some more
research and get some more ideas from other bugzillas. I suppose we
can have them discussed and revised sometime.

--Natalie

Re: How to improve the quality of the kernel?

[Linus Torvalds]

On Mon, 18 Jun 2007, Martin Bligh wrote:
> 
> Sorry to be a wet blanket, but I've seen those sort of things
> before, and they just don't seem to work, especially in the
> environment we're in with such a massive diversity of hardware.

I do agree. It _sounds_ like a great idea to try to control the flow of 
patches better, but in the end, it needs to also be easy and painfree to 
the people involved, and also make sure that any added workflow doesn't 
require even *more* load and expertise on the already often overworked 
maintainers..

In many cases, I think it tends to *sound* great to try to avoid 
regressions in the first place - but it also sounds like one of those "I 
wish the world didn't work the way it did" kind of things. A worthy goal, 
but not necessarily one that is compatible with reality.

			Linus

Re: How to improve the quality of the kernel?

[Martin Bligh]

Natalie Protasevich wrote:
> On 6/18/07, Martin Bligh <mbligh@mbligh.org> wrote:
>> >> > So if you make changes to random-driver.c you can do `git-log
>> >> > random-driver.c|grep Tested-by" to find people who can test
>> >> > your changes for you.
>> >>
>> >> You would'nt even need to search in GIT.  Maybie even when ever a
>> >> patchset is being proposed a mail could be sent to appropriate
>> >> hardware/or feature pseudo-auto-generated mailing-list?
>> >>
>> >> On lkml I mostly try to follow patches/bugs associated with hardware I
>> >> use.  Why not try to automate the process and get more testers in?
>> >>
>> >
>> > I think this is an excellent point. One data point could be a field in
>> > bugzilla to input the hardware information. Simple query can select
>> > common hardware and platform. So far it's not working when hardware is
>> > just mentioned in the text part.
>>
>> if it's free text it'll be useless for search ... I suppose we could
>> do drop-downs for architecture at least? Not sure much beyond that
>> would work ... *possibly* the common drivers, but I don't think
>> we'd get enough coverage for it to be of use.
>
> How about several buckets for model/BIOS version/chipset etc., at
> least optional, and some will be relevant some not for particular
> cases. But at least people will make an attempt to collect such data
> from their system, boards, etc.

Mmm. the problem is that either they're:

1. free text, in which case they're useless, as everyone types
mis-spelled random crud into them. However, free-text search
through the comment fields might work out.

2. Drop downs, in which case someone has to manage the lists
etc, they're horribly crowded with lots of options. trying to
do that for model/BIOS version/chipset would be a nightmare.

If they're mandatory, they're a pain in the butt, and often
irrelevant ... if they're optional, nobody will fill them in.
Either way, they clutter the interface ;-(

Sorry to be a wet blanket, but I've seen those sort of things
before, and they just don't seem to work, especially in the
environment we're in with such a massive diversity of hardware.

If we can come up with some very clear, tightly constrained
choices, that's a decent possibility. Nothing other than
kernel architecture (i386 / x86_64 / ia64) or whatever springs
to mind, but perhaps I'm being unimaginative.

Nothing complicated ever seems to work ... even the simple
stuff is difficult ;-(

M.

Re: How to improve the quality of the kernel?

[Natalie Protasevich]

On 6/18/07, Martin Bligh <mbligh@mbligh.org> wrote:
> >> > So if you make changes to random-driver.c you can do `git-log
> >> > random-driver.c|grep Tested-by" to find people who can test
> >> > your changes for you.
> >>
> >> You would'nt even need to search in GIT.  Maybie even when ever a
> >> patchset is being proposed a mail could be sent to appropriate
> >> hardware/or feature pseudo-auto-generated mailing-list?
> >>
> >> On lkml I mostly try to follow patches/bugs associated with hardware I
> >> use.  Why not try to automate the process and get more testers in?
> >>
> >
> > I think this is an excellent point. One data point could be a field in
> > bugzilla to input the hardware information. Simple query can select
> > common hardware and platform. So far it's not working when hardware is
> > just mentioned in the text part.
>
> if it's free text it'll be useless for search ... I suppose we could
> do drop-downs for architecture at least? Not sure much beyond that
> would work ... *possibly* the common drivers, but I don't think
> we'd get enough coverage for it to be of use.
>
How about several buckets for model/BIOS version/chipset etc., at
least optional, and some will be relevant some not for particular
cases. But at least people will make an attempt to collect such data
from their system, boards, etc.

--Natalie

Re: How to improve the quality of the kernel?

[Martin Bligh]

>> > So if you make changes to random-driver.c you can do `git-log
>> > random-driver.c|grep Tested-by" to find people who can test
>> > your changes for you.
>>
>> You would'nt even need to search in GIT.  Maybie even when ever a
>> patchset is being proposed a mail could be sent to appropriate
>> hardware/or feature pseudo-auto-generated mailing-list?
>>
>> On lkml I mostly try to follow patches/bugs associated with hardware I
>> use.  Why not try to automate the process and get more testers in?
>>
> 
> I think this is an excellent point. One data point could be a field in
> bugzilla to input the hardware information. Simple query can select
> common hardware and platform. So far it's not working when hardware is
> just mentioned in the text part.

if it's free text it'll be useless for search ... I suppose we could
do drop-downs for architecture at least? Not sure much beyond that
would work ... *possibly* the common drivers, but I don't think
we'd get enough coverage for it to be of use.

M.

Re: How to improve the quality of the kernel?

[Natalie Protasevich]

On 6/18/07, Fortier,Vincent [Montreal] <Vincent.Fortier1@ec.gc.ca> wrote:
> > -----Message d'origine-----
> > De : linux-kernel-owner@vger.kernel.org
> > [mailto:linux-kernel-owner@vger.kernel.org] De la part de
> > Andrew Morton
> >
> > On Mon, 18 Jun 2007 01:15:15 +0200 Stefan Richter
> > <stefanr@s5r6.in-berlin.de> wrote:
> >
> > > Tested-by
> >
> > Tested-by would be good too.  Because over time, we will
> > generate a list of people who own the relevant hardware and
> > who are prepared to test changes.
>
> Why not include a user-space tool that, when invoked, if you agree to
> send personnal info, sends your hardware vs driver info to a web
> database + your email address (maybie even you .config, etc..) ... In
> case of help for testing new patches/finding a bug/etc.. your email
> could be used by maintainers to ask for help...
>
> > So if you make changes to random-driver.c you can do `git-log
> > random-driver.c|grep Tested-by" to find people who can test
> > your changes for you.
>
> You would'nt even need to search in GIT.  Maybie even when ever a
> patchset is being proposed a mail could be sent to appropriate
> hardware/or feature pseudo-auto-generated mailing-list?
>
> On lkml I mostly try to follow patches/bugs associated with hardware I
> use.  Why not try to automate the process and get more testers in?
>

I think this is an excellent point. One data point could be a field in
bugzilla to input the hardware information. Simple query can select
common hardware and platform. So far it's not working when hardware is
just mentioned in the text part.

--Natalie

> - vin
> -
> 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/
>

RE: How to improve the quality of the kernel?

[Fortier,Vincent [Montreal]]

> -----Message d'origine-----
> De : linux-kernel-owner@vger.kernel.org 
> [mailto:linux-kernel-owner@vger.kernel.org] De la part de 
> Andrew Morton
> 
> On Mon, 18 Jun 2007 01:15:15 +0200 Stefan Richter 
> <stefanr@s5r6.in-berlin.de> wrote:
> 
> > Tested-by
> 
> Tested-by would be good too.  Because over time, we will 
> generate a list of people who own the relevant hardware and 
> who are prepared to test changes. 

Why not include a user-space tool that, when invoked, if you agree to
send personnal info, sends your hardware vs driver info to a web
database + your email address (maybie even you .config, etc..) ... In
case of help for testing new patches/finding a bug/etc.. your email
could be used by maintainers to ask for help...

> So if you make changes to random-driver.c you can do `git-log 
> random-driver.c|grep Tested-by" to find people who can test 
> your changes for you.

You would'nt even need to search in GIT.  Maybie even when ever a
patchset is being proposed a mail could be sent to appropriate
hardware/or feature pseudo-auto-generated mailing-list?

On lkml I mostly try to follow patches/bugs associated with hardware I
use.  Why not try to automate the process and get more testers in?

- vin

Re: How to improve the quality of the kernel?

[Andrew Morton]

On Mon, 18 Jun 2007 01:15:15 +0200 Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

> Tested-by

Tested-by would be good too.  Because over time, we will generate a list of
people who own the relevant hardware and who are prepared to test changes. 

So if you make changes to random-driver.c you can do `git-log
random-driver.c|grep Tested-by" to find people who can test your changes
for you.

Not that many people are likely to bother.  The consequences of being
slack are negligible, hence there is little incentive to do the extra
work.

Re: How to improve the quality of the kernel?

[Bartlomiej Zolnierkiewicz]

On Monday 18 June 2007, Rafael J. Wysocki wrote:
> On Sunday, 17 June 2007 23:49, Bartlomiej Zolnierkiewicz wrote:
> > On Sunday 17 June 2007, Andrew Morton wrote:
> > > On Sun, 17 Jun 2007 20:53:41 +0200 Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> [--snip--]
> > > 
> > > yup, Reviewed-by: is good and I do think we should start adopting it,
> > > although I haven't thought through exactly how.
> > 
> > Adding Reviewed-by for reviews which highlighted real issues is obvious
> > (with more detailed credits for noticed problems in the patch description).
> 
> Suppose you have modified the patch as a result of a review and you post the
> modified version.  Is that still right to put "Reviewed-by" into it?
> Personally, I don't think so, because that suggests that this particular
> version of the patch has been reviewed and not the previous one.

Well, if you got the "fix issues" part right it in the modified version
it shouldn't really matter. ;)

But yes, we may wait with adding "Reviewed-by" after the modified patch
has been posted and reviewed.

> > Also when somebody reviewed your patch but the discussions it turned out
> > that the patch is valid - the review itself was still valuable so it would
> > be appropriate to credit the reviewer by adding Reviewed-by:.
> 
> Yes, IMO in such a case it would be appropriate to do that.
> 
> Also, the review need not lead to any negative comments from the reviewer,
> but in that case it's also appropriate to add a "Reviewed-by" to the patch.
> 
> Generally, if someone comments my patches, I add his/her address to the next
> version's CC list, which sort of documents that the reviewer was involved.
> Then, if the reviewer ACKs the patch, that will be recorded.

Same approach here.

> I think that for "Reviewed-by" to work correctly, we ought to have a two-stage
> process of accepting patches, where in the first stage the patch is reviewed
> and if there are no objections, the "Reviewed-by" (or "Acked-by") records are
> added to it in the next stage (the patch itself remains unmodified).
> 
> > > On my darker days I consider treating a Reviewed-by: as a prerequisite for
> > > merging.  I suspect that would really get the feathers flying.
> > 
> > Easy to workaround by a friendly mine "Reviewed-by:" for yours "Reviewed-by:"
> > deals (without any _proper_ review being done in reality)... ;)
> > 
> > > > I also encourage other maintainers/developers to pay more attention to
> > > > adding "Acked-by"/"Reviewed-by" tags and crediting reviewers.  I hope
> > > > that maintainers will promote changes that have been reviewed by others
> > > > by giving them priority over other ones (if the changes are on more-or-less
> > > > the same importance level of course, you get the idea).
> > > > 
> > > > Now what to do with people who ignore reviews and/or have rather high
> > > > regressions/patches ratio?
> > > 
> > > Ignoring a review would be a wildly wrong thing to do.  It's so unusual
> > > that I'd be suspecting a lost email or an i-sent-the-wrong-patch.
> > 
> > It is not unusual et all.  I mean patches which affect code in such way
> > that it is difficult to prove it's (in)correctness without doing time
> > consuming audit.
> > 
> > ie. lets imagine doing a small patch affecting many drivers - you've tested
> > it quickly on your driver/hardware, then you skip the part of verifying
> > correctness of new code in other drivers and just push the patch
> > 
> > As a patch author you can either assume "works for me" and push the patch
> > or do the audit (requires good understanding of the changed code and could
> > be time consuming).  It is usually quite easy to find out which approach
> > the author has choosen - the very sparse patch description combined with
> > the changes in code behavior not mentioned in the patch description should
> > raise the red flag. :)
> 
> First of all, the author should have a good understanding of what he's doing
> and why.  If there are any doubts with respect to that, the patch is likely to
> introduce bugs.
> 
> This also depends on who will be handling the bug reports related to the patch.
> If that will be the patch author, then so be it. ;-)

The problem is that usually Andrew/Adrian/Michal would also be involved.

> > As a reviewer having enough knowledge in the area of code affected by patch
> > you can see the potential problems but you can't prove them without doing
> > the time consuming part.  You may try to NACK the patch if you have enough
> > power but you will end up being bypassed by not proving incorrectness of
> > the patch (not to mention that developer will feel bad about you NACKing
> > his patch).
> 
> Well, IMHO, the author of the patch should convince _you_ that the patch is
> correct, not the other way around.  If you have doubts and make him think
> twice of the code and he still can't prove his point, this means that he
> doesn't understand what he's doing well enough.

This is a nice theory, practise differs greatly.

Sometimes you are not in position to prevent suspicious patches from being
merged and sometimes you just don't want to do it for various reasons (not
discouring the developer and preventing his personal vendetta against you :).

> > Now the funny thing is that despite the fact that audit takes 
> > more time/knowledge then making the patch you will end up with zero credit
> > if patch turns out to be (luckily) correct.  Even if you find out issues
> > and report them you are still on mercy of author for being credited so
> > from personal POV you are much better to wait and fix issues after they
> > hit mainline kernel.  You have to choose between being a good citizen and
> > preventing kernel regressions or being bastard and getting the credit. ;)
> 
> Unless you are the poor soul having to handle bug reports related to the
> problem.
> 
> > If you happen to be maintainer of the affected code the choice is similar
> > with more pros for letting the patch in especially if you can't afford the
> > time to do audit (and by being maintainer you are guaranteed to be heavily
> > time constrained).
> > 
> > I hope this makes people see the importance of proper review and proper
> > recognition of reviewers in preventing kernel regressions.
> > 
> > > As for high regressions/patches ratio: that'll be hard to calculate and
> > > tends to be dependent upon the code which is being altered rather than who
> > > is doing the altering: some stuff is just fragile, for various reasons.
> > > 
> > > One ratio which we might want to have a think about is the patches-sent
> > > versus reviews-done ratio ;)
> > 
> > Sounds like a good idea.
> > 
> > > > I think that we should have info about regressions integrated into SCM,
> > > > i.e. in git we should have optional "fixes-commit" tag and we should be
> > > > able to do some reverse data colletion.   This feature combined with
> > > > "Author:" info after some time should give us some very interesting
> > > > statistics (Top Ten "Regressors").  It wouldn't be ideal (ie. we need some
> > > > patches threshold to filter out people with 1 patch and >= 1 regression(s),
> > > > we need to remember that some code areas are more difficult than the others
> > > > and that patches are not equal per se etc.) however I believe than making it
> > > > into Top Ten "Regressors" should give the winners some motivation to improve
> > > > their work ethic.  Well, in the worst case we would just get some extra
> > > > trivial/documentation patches. ;-)
> > > 
> > > We of course do want to minimise the amount of overhead for each developer. 
> > > I'm a strong believer in specialisation: rather than requiring that *every*
> > > developer/maintainer integrate new steps in their processes it would be
> > > better to allow them to proceed in a close-to-usual fashion and to provide
> > > for a specialist person (or team) to do the sorts of things which you're
> > > thinking about.
> > 
> > Makes sense... however we need to educate each and every developer about
> > importance of the code review and proper recognition of reviewers.
> 
> I don't think that the education alone will be enough.  IMO we need to have a
> system that promotes the reviewing of code.

Sure, we need to start somewhere...

Bart

Re: How to improve the quality of the kernel?

[Stefan Richter]

Bartlomiej Zolnierkiewicz wrote:
> In case of being maintainer "bastard approach" is more about not discouraging
> developers by holding patches for too long than about getting credit.

The maintainer who is about to suffocate in newly contributed code is
actually a lucky guy:  He can ask his eager contributors to also help
with cross-reviewing and bug fixing, otherwise all the fine work will be
stuck in the clogged pipeline.  (E.g. post a subsystem todo-list now and
then, as a subtle hint.)
-- 
Stefan Richter
-=====-=-=== -==- =--=-
http://arcgraph.de/sr/

Re: How to improve the quality of the kernel?

[Bartlomiej Zolnierkiewicz]

On Monday 18 June 2007, Stefan Richter wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > despite the fact that audit takes
> > more time/knowledge then making the patch you will end up with zero credit
> > if patch turns out to be (luckily) correct.  Even if you find out issues
> > and report them you are still on mercy of author for being credited
> 
> If we introduce a "Reviewed-by" with reasonably clear semantics
> (different from Signed-off-by; e.g. the reviewer is not a middle-man in
> patch forwarding; the reviewer might have had remaining reservations...
> very similar to but not entirely the same as "Acked-by" as currently
> defined in -mm) --- and also make the already somewhat established
> "Tested-by" more official, --- then the maintainers could start to make
> it a habit to add Reviewed-by and Tested-by.
> 
> Plus, reviewers and testers could formally reply with Reviewed-by and
> Tested-by lines to patch postings and even could explicitly ask the
> maintainer to add these lines.

Sounds great.

> > so from personal POV you are much better to wait and fix issues after they
> > hit mainline kernel.  You have to choose between being a good citizen and
> > preventing kernel regressions or being bastard and getting the credit. ;)
> > 
> > If you happen to be maintainer of the affected code the choice is similar
> > with more pros for letting the patch in especially if you can't afford the
> > time to do audit (and by being maintainer you are guaranteed to be heavily
> > time constrained).
> 
> I don't think that a maintainer (who signs off on patches after all) can
> easily afford to take the "bastard approach".  I may be naive.

Well, I'm not doing it myself but I find it tempting... ;)

In case of being maintainer "bastard approach" is more about not discouraging
developers by holding patches for too long than about getting credit.

Bart

Re: How to improve the quality of the kernel?

[Rafael J. Wysocki]

On Sunday, 17 June 2007 23:49, Bartlomiej Zolnierkiewicz wrote:
> On Sunday 17 June 2007, Andrew Morton wrote:
> > On Sun, 17 Jun 2007 20:53:41 +0200 Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
[--snip--]
> > 
> > yup, Reviewed-by: is good and I do think we should start adopting it,
> > although I haven't thought through exactly how.
> 
> Adding Reviewed-by for reviews which highlighted real issues is obvious
> (with more detailed credits for noticed problems in the patch description).

Suppose you have modified the patch as a result of a review and you post the
modified version.  Is that still right to put "Reviewed-by" into it?
Personally, I don't think so, because that suggests that this particular
version of the patch has been reviewed and not the previous one.

> Also when somebody reviewed your patch but the discussions it turned out
> that the patch is valid - the review itself was still valuable so it would
> be appropriate to credit the reviewer by adding Reviewed-by:.

Yes, IMO in such a case it would be appropriate to do that.

Also, the review need not lead to any negative comments from the reviewer,
but in that case it's also appropriate to add a "Reviewed-by" to the patch.

Generally, if someone comments my patches, I add his/her address to the next
version's CC list, which sort of documents that the reviewer was involved.
Then, if the reviewer ACKs the patch, that will be recorded.

I think that for "Reviewed-by" to work correctly, we ought to have a two-stage
process of accepting patches, where in the first stage the patch is reviewed
and if there are no objections, the "Reviewed-by" (or "Acked-by") records are
added to it in the next stage (the patch itself remains unmodified).

> > On my darker days I consider treating a Reviewed-by: as a prerequisite for
> > merging.  I suspect that would really get the feathers flying.
> 
> Easy to workaround by a friendly mine "Reviewed-by:" for yours "Reviewed-by:"
> deals (without any _proper_ review being done in reality)... ;)
> 
> > > I also encourage other maintainers/developers to pay more attention to
> > > adding "Acked-by"/"Reviewed-by" tags and crediting reviewers.  I hope
> > > that maintainers will promote changes that have been reviewed by others
> > > by giving them priority over other ones (if the changes are on more-or-less
> > > the same importance level of course, you get the idea).
> > > 
> > > Now what to do with people who ignore reviews and/or have rather high
> > > regressions/patches ratio?
> > 
> > Ignoring a review would be a wildly wrong thing to do.  It's so unusual
> > that I'd be suspecting a lost email or an i-sent-the-wrong-patch.
> 
> It is not unusual et all.  I mean patches which affect code in such way
> that it is difficult to prove it's (in)correctness without doing time
> consuming audit.
> 
> ie. lets imagine doing a small patch affecting many drivers - you've tested
> it quickly on your driver/hardware, then you skip the part of verifying
> correctness of new code in other drivers and just push the patch
> 
> As a patch author you can either assume "works for me" and push the patch
> or do the audit (requires good understanding of the changed code and could
> be time consuming).  It is usually quite easy to find out which approach
> the author has choosen - the very sparse patch description combined with
> the changes in code behavior not mentioned in the patch description should
> raise the red flag. :)

First of all, the author should have a good understanding of what he's doing
and why.  If there are any doubts with respect to that, the patch is likely to
introduce bugs.

This also depends on who will be handling the bug reports related to the patch.
If that will be the patch author, then so be it. ;-)

> As a reviewer having enough knowledge in the area of code affected by patch
> you can see the potential problems but you can't prove them without doing
> the time consuming part.  You may try to NACK the patch if you have enough
> power but you will end up being bypassed by not proving incorrectness of
> the patch (not to mention that developer will feel bad about you NACKing
> his patch).

Well, IMHO, the author of the patch should convince _you_ that the patch is
correct, not the other way around.  If you have doubts and make him think
twice of the code and he still can't prove his point, this means that he
doesn't understand what he's doing well enough.

> Now the funny thing is that despite the fact that audit takes 
> more time/knowledge then making the patch you will end up with zero credit
> if patch turns out to be (luckily) correct.  Even if you find out issues
> and report them you are still on mercy of author for being credited so
> from personal POV you are much better to wait and fix issues after they
> hit mainline kernel.  You have to choose between being a good citizen and
> preventing kernel regressions or being bastard and getting the credit. ;)

Unless you are the poor soul having to handle bug reports related to the
problem.
 
> If you happen to be maintainer of the affected code the choice is similar
> with more pros for letting the patch in especially if you can't afford the
> time to do audit (and by being maintainer you are guaranteed to be heavily
> time constrained).
> 
> I hope this makes people see the importance of proper review and proper
> recognition of reviewers in preventing kernel regressions.
> 
> > As for high regressions/patches ratio: that'll be hard to calculate and
> > tends to be dependent upon the code which is being altered rather than who
> > is doing the altering: some stuff is just fragile, for various reasons.
> > 
> > One ratio which we might want to have a think about is the patches-sent
> > versus reviews-done ratio ;)
> 
> Sounds like a good idea.
> 
> > > I think that we should have info about regressions integrated into SCM,
> > > i.e. in git we should have optional "fixes-commit" tag and we should be
> > > able to do some reverse data colletion.   This feature combined with
> > > "Author:" info after some time should give us some very interesting
> > > statistics (Top Ten "Regressors").  It wouldn't be ideal (ie. we need some
> > > patches threshold to filter out people with 1 patch and >= 1 regression(s),
> > > we need to remember that some code areas are more difficult than the others
> > > and that patches are not equal per se etc.) however I believe than making it
> > > into Top Ten "Regressors" should give the winners some motivation to improve
> > > their work ethic.  Well, in the worst case we would just get some extra
> > > trivial/documentation patches. ;-)
> > 
> > We of course do want to minimise the amount of overhead for each developer. 
> > I'm a strong believer in specialisation: rather than requiring that *every*
> > developer/maintainer integrate new steps in their processes it would be
> > better to allow them to proceed in a close-to-usual fashion and to provide
> > for a specialist person (or team) to do the sorts of things which you're
> > thinking about.
> 
> Makes sense... however we need to educate each and every developer about
> importance of the code review and proper recognition of reviewers.

I don't think that the education alone will be enough.  IMO we need to have a
system that promotes the reviewing of code.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: How to improve the quality of the kernel?

[Stefan Richter]

Bartlomiej Zolnierkiewicz wrote:
> despite the fact that audit takes
> more time/knowledge then making the patch you will end up with zero credit
> if patch turns out to be (luckily) correct.  Even if you find out issues
> and report them you are still on mercy of author for being credited

If we introduce a "Reviewed-by" with reasonably clear semantics
(different from Signed-off-by; e.g. the reviewer is not a middle-man in
patch forwarding; the reviewer might have had remaining reservations...
very similar to but not entirely the same as "Acked-by" as currently
defined in -mm) --- and also make the already somewhat established
"Tested-by" more official, --- then the maintainers could start to make
it a habit to add Reviewed-by and Tested-by.

Plus, reviewers and testers could formally reply with Reviewed-by and
Tested-by lines to patch postings and even could explicitly ask the
maintainer to add these lines.

> so from personal POV you are much better to wait and fix issues after they
> hit mainline kernel.  You have to choose between being a good citizen and
> preventing kernel regressions or being bastard and getting the credit. ;)
> 
> If you happen to be maintainer of the affected code the choice is similar
> with more pros for letting the patch in especially if you can't afford the
> time to do audit (and by being maintainer you are guaranteed to be heavily
> time constrained).

I don't think that a maintainer (who signs off on patches after all) can
easily afford to take the "bastard approach".  I may be naive.
-- 
Stefan Richter
-=====-=-=== -==- =--=-
http://arcgraph.de/sr/

Re: How to improve the quality of the kernel?

[Bartlomiej Zolnierkiewicz]

On Sunday 17 June 2007, Andrew Morton wrote:
> On Sun, 17 Jun 2007 20:53:41 +0200 Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> 
> > 
> > 
> > IMO we should concentrate more on preventing regressions than on fixing them.
> > In the long-term preventing bugs is cheaper than fixing them afterwards.
> > 
> > First let me tell you all a little story...
> > 
> > Over two years ago I've reviewed some _cleanup_ patch and noticed three bugs
> > in it (in other words I potentially prevented three regressions).  I also
> > asked for more thorough verification of the patch as I suspected that it may
> > have more problems.  The author fixed the issues and replied that he hasn't
> > done the full verification yet but he doesn't suspect any problems...
> > 
> > Fast forward...
> > 
> > Year later I discover that the final version of the patch hit the mainline.
> > I don't remember ever seeing the final version in my mailbox (there are no
> > cc: lines in the patch description) and I saw that I'm not credited in the
> > patch description.  However the worse part is that it seems that the full
> > verification has never been done.  The result?   Regression in the release
> > kernel (exactly the issue that I was worried about) which required three
> > patches and over a month to be fixed completely.  It seems that a year
> > was not enough to get this ~70k _cleanup_ patch fully verified and tested
> > (it hit -mm soon before being merged)...
> 
> crap.  Commit ID, please ;)

Will send in pm.

I don't want to reveal the "guilty" person identify in public.

> > >From reviewer's POV: I have invested my time into review, discovered real
> > issues and as a reward I got no credit et all and extra frustration from the
> > fact that part of my review was forgotten/ignored (the part which resulted in
> > real regression in the release kernel)...  Oh and in the past the said
> > developer has already been asked (politely in private message) to pay more
> > attention to his changes (after I silently fixed some other regression caused
> > by his other patch).
> > 
> > But wait there is more, I happend to be the maintainer of the subsystem which
> > got directly hit by the issue and I was getting bugreports from the users about
> > the problem...  :-)
> > 
> > It wasn't my first/last bad experience as a reviewer... finally I just gave up
> > on reviewing other people patches unless they are stricly for IDE subsystem.
> > 
> > The moral of the story is that currently it just doesn't pay off to do
> > code reviews.
> 
> I dunno.  I suspect (hope) that this was an exceptional case, hence one
> should not draw general conclusions from it.  It certainly sounds very bad.

I've been too long around to not learn a few things...

rule #3 of successful kernel developer

Ignore reviewers - fix the bugs but don't credit reviewers (crediting them
makes your patch and you look less perfect), if they are asking question
requiring you to do the work (verification of taken assumptions etc.) do not
check anything - answer in a misleading way and present the assumptions you've
taken as a truth written in the stone - eventually they will do verification
themselves.

I really shouldn't be giving these rules out (at least for free 8) so this
time only #3 but there are much more rules and they are as dead serious as
Linus' advices on Linux kernel management style...

> >  From personal POV it pays much more to wait until buggy patch
> > hits the mainline and then fix the issues yourself (at least you will get
> > some credit).  To change this we should put more ephasize on the importance
> > of code reviews by "rewarding" people investing their time into reviews
> > and "rewarding" developers/maintainers taking reviews seriously.
> > 
> > We should credit reviewers more, sometimes it takes more time/knowledge to
> > review the patch than to make it so getting near to zero credit for review
> > doesn't sound too attractive.  Hmm, wait it can be worse - your review
> > may be ignored... ;-)
> > 
> > >From my side I think I'll start adding less formal "Reviewed-by" to IDE
> > patches even if the review resulted in no issues being found (in additon to
> > explicit "Acked-by" tags and crediting people for finding real issues - which
> > I currently always do as a way for showing my appreciation for their work).
> 
> yup, Reviewed-by: is good and I do think we should start adopting it,
> although I haven't thought through exactly how.

Adding Reviewed-by for reviews which highlighted real issues is obvious
(with more detailed credits for noticed problems in the patch description).

Also when somebody reviewed your patch but the discussions it turned out
that the patch is valid - the review itself was still valuable so it would
be appropriate to credit the reviewer by adding Reviewed-by:.

> On my darker days I consider treating a Reviewed-by: as a prerequisite for
> merging.  I suspect that would really get the feathers flying.

Easy to workaround by a friendly mine "Reviewed-by:" for yours "Reviewed-by:"
deals (without any _proper_ review being done in reality)... ;)

> > I also encourage other maintainers/developers to pay more attention to
> > adding "Acked-by"/"Reviewed-by" tags and crediting reviewers.  I hope
> > that maintainers will promote changes that have been reviewed by others
> > by giving them priority over other ones (if the changes are on more-or-less
> > the same importance level of course, you get the idea).
> > 
> > Now what to do with people who ignore reviews and/or have rather high
> > regressions/patches ratio?
> 
> Ignoring a review would be a wildly wrong thing to do.  It's so unusual
> that I'd be suspecting a lost email or an i-sent-the-wrong-patch.

It is not unusual et all.  I mean patches which affect code in such way
that it is difficult to prove it's (in)correctness without doing time
consuming audit.

ie. lets imagine doing a small patch affecting many drivers - you've tested
it quickly on your driver/hardware, then you skip the part of verifying
correctness of new code in other drivers and just push the patch

As a patch author you can either assume "works for me" and push the patch
or do the audit (requires good understanding of the changed code and could
be time consuming).  It is usually quite easy to find out which approach
the author has choosen - the very sparse patch description combined with
the changes in code behavior not mentioned in the patch description should
raise the red flag. :)

As a reviewer having enough knowledge in the area of code affected by patch
you can see the potential problems but you can't prove them without doing
the time consuming part.  You may try to NACK the patch if you have enough
power but you will end up being bypassed by not proving incorrectness of
the patch (not to mention that developer will feel bad about you NACKing
his patch).  Now the funny thing is that despite the fact that audit takes
more time/knowledge then making the patch you will end up with zero credit
if patch turns out to be (luckily) correct.  Even if you find out issues
and report them you are still on mercy of author for being credited so
from personal POV you are much better to wait and fix issues after they
hit mainline kernel.  You have to choose between being a good citizen and
preventing kernel regressions or being bastard and getting the credit. ;)

If you happen to be maintainer of the affected code the choice is similar
with more pros for letting the patch in especially if you can't afford the
time to do audit (and by being maintainer you are guaranteed to be heavily
time constrained).

I hope this makes people see the importance of proper review and proper
recognition of reviewers in preventing kernel regressions.

> As for high regressions/patches ratio: that'll be hard to calculate and
> tends to be dependent upon the code which is being altered rather than who
> is doing the altering: some stuff is just fragile, for various reasons.
> 
> One ratio which we might want to have a think about is the patches-sent
> versus reviews-done ratio ;)

Sounds like a good idea.

> > I think that we should have info about regressions integrated into SCM,
> > i.e. in git we should have optional "fixes-commit" tag and we should be
> > able to do some reverse data colletion.   This feature combined with
> > "Author:" info after some time should give us some very interesting
> > statistics (Top Ten "Regressors").  It wouldn't be ideal (ie. we need some
> > patches threshold to filter out people with 1 patch and >= 1 regression(s),
> > we need to remember that some code areas are more difficult than the others
> > and that patches are not equal per se etc.) however I believe than making it
> > into Top Ten "Regressors" should give the winners some motivation to improve
> > their work ethic.  Well, in the worst case we would just get some extra
> > trivial/documentation patches. ;-)
> 
> We of course do want to minimise the amount of overhead for each developer. 
> I'm a strong believer in specialisation: rather than requiring that *every*
> developer/maintainer integrate new steps in their processes it would be
> better to allow them to proceed in a close-to-usual fashion and to provide
> for a specialist person (or team) to do the sorts of things which you're
> thinking about.

Makes sense... however we need to educate each and every developer about
importance of the code review and proper recognition of reviewers.

Thanks,
Bart

Re: How to improve the quality of the kernel?

[Michal Piotrowski]

On 17/06/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Carlo Wood wrote:
> > On Sun, Jun 17, 2007 at 09:18:17PM +0200, Rafael J. Wysocki wrote:
> >> where n is a number that needs to be determined (I think that n could be 3).
> >> Well, "negative comments" should also be defined more precisely. ;-)
> >
> > I think that n should be a function of the number of accepted patches
> > that this person sent in before, and the number of regressions he
> > caused in the past.
>
> The character of the patch (potential impacts, size...) and availability
> of reviewers and testers influence the required review time so much that
> other factors, like reputation of the submitter, hardly matter.

So we need a bug/regression/patch tracking system based on MMORPG game ;)

Regards,
Michal

-- 
LOG
http://www.stardust.webpages.pl/log/

Re: How to improve the quality of the kernel?

[Stefan Richter]

Carlo Wood wrote:
> On Sun, Jun 17, 2007 at 09:18:17PM +0200, Rafael J. Wysocki wrote:
>> where n is a number that needs to be determined (I think that n could be 3).
>> Well, "negative comments" should also be defined more precisely. ;-)
> 
> I think that n should be a function of the number of accepted patches
> that this person sent in before, and the number of regressions he
> caused in the past.

The character of the patch (potential impacts, size...) and availability
of reviewers and testers influence the required review time so much that
other factors, like reputation of the submitter, hardly matter.
-- 
Stefan Richter
-=====-=-=== -==- =---=
http://arcgraph.de/sr/

Re: How to improve the quality of the kernel?

[Carlo Wood]

On Sun, Jun 17, 2007 at 09:18:17PM +0200, Rafael J. Wysocki wrote:
> where n is a number that needs to be determined (I think that n could be 3).
> Well, "negative comments" should also be defined more precisely. ;-)

I think that n should be a function of the number of accepted patches
that this person sent in before, and the number of regressions he
caused in the past.

Ie, new developers have to wait a considerable amount of time - while
experienced developers who never caused a regression should be able
to write patches that are immediately applied. Also, if anyone causes
a regression - that would lead to them having to wait longer the
next time before they can apply the patch - a good reason for a
developer to put extra time into making sure there are no regressions.

-- 
Carlo Wood <carlo@alinoe.com>

Re: How to improve the quality of the kernel?

[Adrian Bunk]

On Sun, Jun 17, 2007 at 07:31:01PM +0200, Rafael J. Wysocki wrote:
> On Sunday, 17 June 2007 16:29, Adrian Bunk wrote:
> > On Sun, Jun 17, 2007 at 03:17:58PM +0200, Michal Piotrowski wrote:
> > > On 17/06/07, Adrian Bunk <bunk@stusta.de> wrote:
> > >...
> > >> Fine with me, but:
> > >>
> > >> There are not so simple cases like big infrastructure patches with
> > >> 20 other patches in the tree depending on it causing a regression, or
> > >> even worse, a big infrastructure patch exposing a latent old bug in some
> > >> completely different area of the kernel.
> > >
> > > It is different case.
> > >
> > > "If the patch introduces a new regression"
> > >
> > > introduces != exposes an old bug
> > 
> > My remark was meant as a note "this sentence can't handle all 
> > regressions" (and for a user it doesn't matter whether a new 
> > regression is introduced or an old regression exposed).
> > 
> > It could be we simply agree on this one.  ;-)
> > 
> > > Removal of 20 patches will be painful, but sometimes you need to
> > > "choose minor evil to prevent a greater one" [1].
> > > 
> > >> And we should be aware that reverting is only a workaround for the real
> > >> problem which lies in our bug handling.
> > >...
> > 
> > And this is something I want to emphasize again.
> > 
> > How can we make any progress with the real problem and not only the 
> > symptoms?
> 
> I think that we can handle bug reports like we handle modifications of code.
> 
> Namely, for each subsystem there can be a person (or a team) responsible
> for handling bugs, by which I don't mean fixing them, but directing bug reports
> at the right developers or subsystem maintainers, following the history of each
> bug report etc.  [Of course, these people can choose to use the bugzilla or any
> other bug tracking system they want, as long as it works for them.]
> 
> The email addresses of these people should be known (and even documented),
> so that everyone can notify them if need be and so that it's clear who should
> handle given bug reports.

Currently, these people are "Andrew Morton" and the addresses are
linux-kernel@vger.kernel.org and http://bugzilla.kernel.org/ - and this 
part is working.

Although there is room for improvement in this area, the problem in the 
pipeline is really to find developers who know the code in question and 
who are willing to debug bug reports.

There are unmaintained parts of the kernel.

And there are parts of the kernel where the maintainers are developing 
code, reviewing code and handling patches but are not willing or simply 
not capable of looking at bug reports. That's not against these people 
and they might do great work, but then there's simply an additional 
person missing who would be willing to learn the subsystem in question 
and handle bug reports.

All bug handling becomes moot and every request for more information 
from the submitter a waste of time if there's noone available for 
looking deeper into a bug.

> Just an idea. :-)
> 
> Greetings,
> Rafael

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: How to improve the quality of the kernel?

[Rafael J. Wysocki]

On Sunday, 17 June 2007 20:52, Andrew Morton wrote:
> On Sun, 17 Jun 2007 20:53:41 +0200 Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> 
> > 
> > 
> > IMO we should concentrate more on preventing regressions than on fixing them.
> > In the long-term preventing bugs is cheaper than fixing them afterwards.
> > 
> > First let me tell you all a little story...
> > 
> > Over two years ago I've reviewed some _cleanup_ patch and noticed three bugs
> > in it (in other words I potentially prevented three regressions).  I also
> > asked for more thorough verification of the patch as I suspected that it may
> > have more problems.  The author fixed the issues and replied that he hasn't
> > done the full verification yet but he doesn't suspect any problems...
> > 
> > Fast forward...
> > 
> > Year later I discover that the final version of the patch hit the mainline.
> > I don't remember ever seeing the final version in my mailbox (there are no
> > cc: lines in the patch description) and I saw that I'm not credited in the
> > patch description.  However the worse part is that it seems that the full
> > verification has never been done.  The result?   Regression in the release
> > kernel (exactly the issue that I was worried about) which required three
> > patches and over a month to be fixed completely.  It seems that a year
> > was not enough to get this ~70k _cleanup_ patch fully verified and tested
> > (it hit -mm soon before being merged)...
> 
> crap.  Commit ID, please ;)
> 
> > >From reviewer's POV: I have invested my time into review, discovered real
> > issues and as a reward I got no credit et all and extra frustration from the
> > fact that part of my review was forgotten/ignored (the part which resulted in
> > real regression in the release kernel)...  Oh and in the past the said
> > developer has already been asked (politely in private message) to pay more
> > attention to his changes (after I silently fixed some other regression caused
> > by his other patch).
> > 
> > But wait there is more, I happend to be the maintainer of the subsystem which
> > got directly hit by the issue and I was getting bugreports from the users about
> > the problem...  :-)
> > 
> > It wasn't my first/last bad experience as a reviewer... finally I just gave up
> > on reviewing other people patches unless they are stricly for IDE subsystem.
> > 
> > The moral of the story is that currently it just doesn't pay off to do
> > code reviews.
> 
> I dunno.  I suspect (hope) that this was an exceptional case, hence one
> should not draw general conclusions from it.  It certainly sounds very bad.
> 
> >  From personal POV it pays much more to wait until buggy patch
> > hits the mainline and then fix the issues yourself (at least you will get
> > some credit).  To change this we should put more ephasize on the importance
> > of code reviews by "rewarding" people investing their time into reviews
> > and "rewarding" developers/maintainers taking reviews seriously.
> > 
> > We should credit reviewers more, sometimes it takes more time/knowledge to
> > review the patch than to make it so getting near to zero credit for review
> > doesn't sound too attractive.  Hmm, wait it can be worse - your review
> > may be ignored... ;-)
> > 
> > >From my side I think I'll start adding less formal "Reviewed-by" to IDE
> > patches even if the review resulted in no issues being found (in additon to
> > explicit "Acked-by" tags and crediting people for finding real issues - which
> > I currently always do as a way for showing my appreciation for their work).
> 
> yup, Reviewed-by: is good and I do think we should start adopting it,
> although I haven't thought through exactly how.
> 
> On my darker days I consider treating a Reviewed-by: as a prerequisite for
> merging.  I suspect that would really get the feathers flying.

How about the following "algorithm":

* Step 1: Send a patch as an RFC to the relevant lists/people and only if there
  are no negative comments within at least n days, you are allowed to proceed
  to the next step.  If anyone has reviewed/acked the patch, add their names
  and email addresses as "Reviewed-by"/"Acked-by" to the patch in the next
  step.
* Step 2: Send the patch as an RC to the relevant lists/people _and_ LKML and
  if there are no negative comments within at least n days, you can proceed to
  the next step.  If anyone has reviewed/acked the patch, add their names
  and email addresses as "Reviewed-by"/"Acked-by" to the patch in the next
  step.
* Step 3: Submit the patch for merging to the right maintainer (keeping the
  previous CC list).

where n is a number that needs to be determined (I think that n could be 3).
Well, "negative comments" should also be defined more precisely. ;-)

> > I also encourage other maintainers/developers to pay more attention to
> > adding "Acked-by"/"Reviewed-by" tags and crediting reviewers.  I hope
> > that maintainers will promote changes that have been reviewed by others
> > by giving them priority over other ones (if the changes are on more-or-less
> > the same importance level of course, you get the idea).
> > 
> > Now what to do with people who ignore reviews and/or have rather high
> > regressions/patches ratio?
> 
> Ignoring a review would be a wildly wrong thing to do.  It's so unusual
> that I'd be suspecting a lost email or an i-sent-the-wrong-patch.
> 
> As for high regressions/patches ratio: that'll be hard to calculate and
> tends to be dependent upon the code which is being altered rather than who
> is doing the altering: some stuff is just fragile, for various reasons.
> 
> One ratio which we might want to have a think about is the patches-sent
> versus reviews-done ratio ;)
> 
> > I think that we should have info about regressions integrated into SCM,
> > i.e. in git we should have optional "fixes-commit" tag and we should be
> > able to do some reverse data colletion.   This feature combined with
> > "Author:" info after some time should give us some very interesting
> > statistics (Top Ten "Regressors").  It wouldn't be ideal (ie. we need some
> > patches threshold to filter out people with 1 patch and >= 1 regression(s),
> > we need to remember that some code areas are more difficult than the others
> > and that patches are not equal per se etc.) however I believe than making it
> > into Top Ten "Regressors" should give the winners some motivation to improve
> > their work ethic.  Well, in the worst case we would just get some extra
> > trivial/documentation patches. ;-)
> 
> We of course do want to minimise the amount of overhead for each developer. 
> I'm a strong believer in specialisation: rather than requiring that *every*
> developer/maintainer integrate new steps in their processes it would be
> better to allow them to proceed in a close-to-usual fashion and to provide
> for a specialist person (or team) to do the sorts of things which you're
> thinking about.

Still, even very experienced developers make trivial mistakes, so there should
be a way to catch such things before they hit -rc or even -mm kernels

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: How to improve the quality of the kernel?

[Michal Piotrowski]

On 17/06/07, Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
>
> Hi,
>
> On Sunday 17 June 2007, Adrian Bunk wrote:
> > On Sun, Jun 17, 2007 at 03:17:58PM +0200, Michal Piotrowski wrote:
> > > On 17/06/07, Adrian Bunk <bunk@stusta.de> wrote:
> > >...
> > >> Fine with me, but:
> > >>
> > >> There are not so simple cases like big infrastructure patches with
> > >> 20 other patches in the tree depending on it causing a regression, or
> > >> even worse, a big infrastructure patch exposing a latent old bug in some
> > >> completely different area of the kernel.
> > >
> > > It is different case.
> > >
> > > "If the patch introduces a new regression"
> > >
> > > introduces != exposes an old bug
> >
> > My remark was meant as a note "this sentence can't handle all
> > regressions" (and for a user it doesn't matter whether a new
> > regression is introduced or an old regression exposed).
> >
> > It could be we simply agree on this one.  ;-)
> >
> > > Removal of 20 patches will be painful, but sometimes you need to
> > > "choose minor evil to prevent a greater one" [1].
> > >
> > >> And we should be aware that reverting is only a workaround for the real
> > >> problem which lies in our bug handling.
> > >...
> >
> > And this is something I want to emphasize again.
> >
> > How can we make any progress with the real problem and not only the
> > symptoms?
> >
> > There's now much money in the Linux market, and the kernel quality
> > problems might result in real costs in the support of companies like
> > IBM, SGI, Redhat or Novell (plus it harms the Linux image which might
> > result in lower revenues).
> >
> > If [1] this is true, it might even pay pay off for them to each assign
> > X man hours per month of experienced kernel developers to upstream
> > kernel bug handling?
> >
> > This is just a wild thought and it might be nonsense - better
> > suggestions for solving our quality problems would be highly welcome...
>
> IMO we should concentrate more on preventing regressions than on fixing them.
> In the long-term preventing bugs is cheaper than fixing them afterwards.
>
> First let me tell you all a little story...
>
> Over two years ago I've reviewed some _cleanup_ patch and noticed three bugs
> in it (in other words I potentially prevented three regressions).  I also
> asked for more thorough verification of the patch as I suspected that it may
> have more problems.  The author fixed the issues and replied that he hasn't
> done the full verification yet but he doesn't suspect any problems...
>
> Fast forward...
>
> Year later I discover that the final version of the patch hit the mainline.
> I don't remember ever seeing the final version in my mailbox (there are no
> cc: lines in the patch description) and I saw that I'm not credited in the
> patch description.  However the worse part is that it seems that the full
> verification has never been done.  The result?   Regression in the release
> kernel (exactly the issue that I was worried about) which required three
> patches and over a month to be fixed completely.  It seems that a year
> was not enough to get this ~70k _cleanup_ patch fully verified and tested
> (it hit -mm soon before being merged)...
>
> From reviewer's POV: I have invested my time into review, discovered real
> issues and as a reward I got no credit et all and extra frustration from the
> fact that part of my review was forgotten/ignored (the part which resulted in
> real regression in the release kernel)...  Oh and in the past the said
> developer has already been asked (politely in private message) to pay more
> attention to his changes (after I silently fixed some other regression caused
> by his other patch).
>
> But wait there is more, I happend to be the maintainer of the subsystem which
> got directly hit by the issue and I was getting bugreports from the users about
> the problem...  :-)
>
> It wasn't my first/last bad experience as a reviewer... finally I just gave up
> on reviewing other people patches unless they are stricly for IDE subsystem.
>
> The moral of the story is that currently it just doesn't pay off to do
> code reviews.  From personal POV it pays much more to wait until buggy patch
> hits the mainline and then fix the issues yourself (at least you will get
> some credit).  To change this we should put more ephasize on the importance
> of code reviews by "rewarding" people investing their time into reviews
> and "rewarding" developers/maintainers taking reviews seriously.
>
> We should credit reviewers more, sometimes it takes more time/knowledge to
> review the patch than to make it so getting near to zero credit for review
> doesn't sound too attractive.  Hmm, wait it can be worse - your review
> may be ignored... ;-)
>
> From my side I think I'll start adding less formal "Reviewed-by" to IDE
> patches even if the review resulted in no issues being found (in additon to
> explicit "Acked-by" tags and crediting people for finding real issues - which
> I currently always do as a way for showing my appreciation for their work).
>
> I also encourage other maintainers/developers to pay more attention to
> adding "Acked-by"/"Reviewed-by" tags and crediting reviewers.  I hope
> that maintainers will promote changes that have been reviewed by others
> by giving them priority over other ones (if the changes are on more-or-less
> the same importance level of course, you get the idea).

I think that this is a very good idea - especially for large, intrusive patches.
Long {Acked,Reviewed,Signed-off,Tested}-by list will be welcome.

>
> Now what to do with people who ignore reviews and/or have rather high
> regressions/patches ratio?
>
> I think that we should have info about regressions integrated into SCM,
> i.e. in git we should have optional "fixes-commit" tag and we should be
> able to do some reverse data colletion.   This feature combined with
> "Author:" info after some time should give us some very interesting
> statistics (Top Ten "Regressors").  It wouldn't be ideal (ie. we need some
> patches threshold to filter out people with 1 patch and >= 1 regression(s),
> we need to remember that some code areas are more difficult than the others
> and that patches are not equal per se etc.) however I believe than making it
> into Top Ten "Regressors" should give the winners some motivation to improve
> their work ethic.  Well, in the worst case we would just get some extra
> trivial/documentation patches. ;-)
>
> Sorry for a bit chaotic mail but I hope that message is clear.
>
> Thanks,
> Bart
>

Regards,
Michal

-- 
LOG
http://www.stardust.webpages.pl/log/

Re: How to improve the quality of the kernel?

[Bartlomiej Zolnierkiewicz]

Hi,

On Sunday 17 June 2007, Adrian Bunk wrote:
> On Sun, Jun 17, 2007 at 03:17:58PM +0200, Michal Piotrowski wrote:
> > On 17/06/07, Adrian Bunk <bunk@stusta.de> wrote:
> >...
> >> Fine with me, but:
> >>
> >> There are not so simple cases like big infrastructure patches with
> >> 20 other patches in the tree depending on it causing a regression, or
> >> even worse, a big infrastructure patch exposing a latent old bug in some
> >> completely different area of the kernel.
> >
> > It is different case.
> >
> > "If the patch introduces a new regression"
> >
> > introduces != exposes an old bug
> 
> My remark was meant as a note "this sentence can't handle all 
> regressions" (and for a user it doesn't matter whether a new 
> regression is introduced or an old regression exposed).
> 
> It could be we simply agree on this one.  ;-)
> 
> > Removal of 20 patches will be painful, but sometimes you need to
> > "choose minor evil to prevent a greater one" [1].
> > 
> >> And we should be aware that reverting is only a workaround for the real
> >> problem which lies in our bug handling.
> >...
> 
> And this is something I want to emphasize again.
> 
> How can we make any progress with the real problem and not only the 
> symptoms?
> 
> There's now much money in the Linux market, and the kernel quality 
> problems might result in real costs in the support of companies like
> IBM, SGI, Redhat or Novell (plus it harms the Linux image which might 
> result in lower revenues).
> 
> If [1] this is true, it might even pay pay off for them to each assign 
> X man hours per month of experienced kernel developers to upstream 
> kernel bug handling?
> 
> This is just a wild thought and it might be nonsense - better 
> suggestions for solving our quality problems would be highly welcome...

IMO we should concentrate more on preventing regressions than on fixing them.
In the long-term preventing bugs is cheaper than fixing them afterwards.

First let me tell you all a little story...

Over two years ago I've reviewed some _cleanup_ patch and noticed three bugs
in it (in other words I potentially prevented three regressions).  I also
asked for more thorough verification of the patch as I suspected that it may
have more problems.  The author fixed the issues and replied that he hasn't
done the full verification yet but he doesn't suspect any problems...

Fast forward...

Year later I discover that the final version of the patch hit the mainline.
I don't remember ever seeing the final version in my mailbox (there are no
cc: lines in the patch description) and I saw that I'm not credited in the
patch description.  However the worse part is that it seems that the full
verification has never been done.  The result?   Regression in the release
kernel (exactly the issue that I was worried about) which required three
patches and over a month to be fixed completely.  It seems that a year
was not enough to get this ~70k _cleanup_ patch fully verified and tested
(it hit -mm soon before being merged)...

>From reviewer's POV: I have invested my time into review, discovered real
issues and as a reward I got no credit et all and extra frustration from the
fact that part of my review was forgotten/ignored (the part which resulted in
real regression in the release kernel)...  Oh and in the past the said
developer has already been asked (politely in private message) to pay more
attention to his changes (after I silently fixed some other regression caused
by his other patch).

But wait there is more, I happend to be the maintainer of the subsystem which
got directly hit by the issue and I was getting bugreports from the users about
the problem...  :-)

It wasn't my first/last bad experience as a reviewer... finally I just gave up
on reviewing other people patches unless they are stricly for IDE subsystem.

The moral of the story is that currently it just doesn't pay off to do
code reviews.  From personal POV it pays much more to wait until buggy patch
hits the mainline and then fix the issues yourself (at least you will get
some credit).  To change this we should put more ephasize on the importance
of code reviews by "rewarding" people investing their time into reviews
and "rewarding" developers/maintainers taking reviews seriously.

We should credit reviewers more, sometimes it takes more time/knowledge to
review the patch than to make it so getting near to zero credit for review
doesn't sound too attractive.  Hmm, wait it can be worse - your review
may be ignored... ;-)

>From my side I think I'll start adding less formal "Reviewed-by" to IDE
patches even if the review resulted in no issues being found (in additon to
explicit "Acked-by" tags and crediting people for finding real issues - which
I currently always do as a way for showing my appreciation for their work).

I also encourage other maintainers/developers to pay more attention to
adding "Acked-by"/"Reviewed-by" tags and crediting reviewers.  I hope
that maintainers will promote changes that have been reviewed by others
by giving them priority over other ones (if the changes are on more-or-less
the same importance level of course, you get the idea).

Now what to do with people who ignore reviews and/or have rather high
regressions/patches ratio?

I think that we should have info about regressions integrated into SCM,
i.e. in git we should have optional "fixes-commit" tag and we should be
able to do some reverse data colletion.   This feature combined with
"Author:" info after some time should give us some very interesting
statistics (Top Ten "Regressors").  It wouldn't be ideal (ie. we need some
patches threshold to filter out people with 1 patch and >= 1 regression(s),
we need to remember that some code areas are more difficult than the others
and that patches are not equal per se etc.) however I believe than making it
into Top Ten "Regressors" should give the winners some motivation to improve
their work ethic.  Well, in the worst case we would just get some extra
trivial/documentation patches. ;-)

Sorry for a bit chaotic mail but I hope that message is clear.

Thanks,
Bart

Re: How to improve the quality of the kernel?

[Andrew Morton]

On Sun, 17 Jun 2007 20:53:41 +0200 Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

> 
> 
> IMO we should concentrate more on preventing regressions than on fixing them.
> In the long-term preventing bugs is cheaper than fixing them afterwards.
> 
> First let me tell you all a little story...
> 
> Over two years ago I've reviewed some _cleanup_ patch and noticed three bugs
> in it (in other words I potentially prevented three regressions).  I also
> asked for more thorough verification of the patch as I suspected that it may
> have more problems.  The author fixed the issues and replied that he hasn't
> done the full verification yet but he doesn't suspect any problems...
> 
> Fast forward...
> 
> Year later I discover that the final version of the patch hit the mainline.
> I don't remember ever seeing the final version in my mailbox (there are no
> cc: lines in the patch description) and I saw that I'm not credited in the
> patch description.  However the worse part is that it seems that the full
> verification has never been done.  The result?   Regression in the release
> kernel (exactly the issue that I was worried about) which required three
> patches and over a month to be fixed completely.  It seems that a year
> was not enough to get this ~70k _cleanup_ patch fully verified and tested
> (it hit -mm soon before being merged)...

crap.  Commit ID, please ;)

> >From reviewer's POV: I have invested my time into review, discovered real
> issues and as a reward I got no credit et all and extra frustration from the
> fact that part of my review was forgotten/ignored (the part which resulted in
> real regression in the release kernel)...  Oh and in the past the said
> developer has already been asked (politely in private message) to pay more
> attention to his changes (after I silently fixed some other regression caused
> by his other patch).
> 
> But wait there is more, I happend to be the maintainer of the subsystem which
> got directly hit by the issue and I was getting bugreports from the users about
> the problem...  :-)
> 
> It wasn't my first/last bad experience as a reviewer... finally I just gave up
> on reviewing other people patches unless they are stricly for IDE subsystem.
> 
> The moral of the story is that currently it just doesn't pay off to do
> code reviews.

I dunno.  I suspect (hope) that this was an exceptional case, hence one
should not draw general conclusions from it.  It certainly sounds very bad.

>  From personal POV it pays much more to wait until buggy patch
> hits the mainline and then fix the issues yourself (at least you will get
> some credit).  To change this we should put more ephasize on the importance
> of code reviews by "rewarding" people investing their time into reviews
> and "rewarding" developers/maintainers taking reviews seriously.
> 
> We should credit reviewers more, sometimes it takes more time/knowledge to
> review the patch than to make it so getting near to zero credit for review
> doesn't sound too attractive.  Hmm, wait it can be worse - your review
> may be ignored... ;-)
> 
> >From my side I think I'll start adding less formal "Reviewed-by" to IDE
> patches even if the review resulted in no issues being found (in additon to
> explicit "Acked-by" tags and crediting people for finding real issues - which
> I currently always do as a way for showing my appreciation for their work).

yup, Reviewed-by: is good and I do think we should start adopting it,
although I haven't thought through exactly how.

On my darker days I consider treating a Reviewed-by: as a prerequisite for
merging.  I suspect that would really get the feathers flying.

> I also encourage other maintainers/developers to pay more attention to
> adding "Acked-by"/"Reviewed-by" tags and crediting reviewers.  I hope
> that maintainers will promote changes that have been reviewed by others
> by giving them priority over other ones (if the changes are on more-or-less
> the same importance level of course, you get the idea).
> 
> Now what to do with people who ignore reviews and/or have rather high
> regressions/patches ratio?

Ignoring a review would be a wildly wrong thing to do.  It's so unusual
that I'd be suspecting a lost email or an i-sent-the-wrong-patch.

As for high regressions/patches ratio: that'll be hard to calculate and
tends to be dependent upon the code which is being altered rather than who
is doing the altering: some stuff is just fragile, for various reasons.

One ratio which we might want to have a think about is the patches-sent
versus reviews-done ratio ;)

> I think that we should have info about regressions integrated into SCM,
> i.e. in git we should have optional "fixes-commit" tag and we should be
> able to do some reverse data colletion.   This feature combined with
> "Author:" info after some time should give us some very interesting
> statistics (Top Ten "Regressors").  It wouldn't be ideal (ie. we need some
> patches threshold to filter out people with 1 patch and >= 1 regression(s),
> we need to remember that some code areas are more difficult than the others
> and that patches are not equal per se etc.) however I believe than making it
> into Top Ten "Regressors" should give the winners some motivation to improve
> their work ethic.  Well, in the worst case we would just get some extra
> trivial/documentation patches. ;-)

We of course do want to minimise the amount of overhead for each developer. 
I'm a strong believer in specialisation: rather than requiring that *every*
developer/maintainer integrate new steps in their processes it would be
better to allow them to proceed in a close-to-usual fashion and to provide
for a specialist person (or team) to do the sorts of things which you're
thinking about.

Re: How to improve the quality of the kernel?

[Natalie Protasevich]

On 6/17/07, Adrian Bunk <bunk@stusta.de> wrote:
> On Sun, Jun 17, 2007 at 06:26:55PM +0200, Stefan Richter wrote:
> > Adrian Bunk wrote:
> > >>> And we should be aware that reverting is only a workaround for the real
> > >>> problem which lies in our bug handling.
> > >> ...
> > >
> > > And this is something I want to emphasize again.
> > >
> > > How can we make any progress with the real problem and not only the
> > > symptoms?
> > ...
> >
> > Perhaps make lists of
> >
> >   - bug reports which never lead to any debug activity
> >     (no responsible person/team was found, or a seemingly person/team
> >     did not start to debug the report)
> >
> >   - known regressions on release,
> >
> >   - regressions that became known after release,
> >
> >   - subsystems with notable backlogs of old bugs,
> >
> >   - other categories?
> >
> > Select typical cases from each categories, analyze what went wrong in
> > these cases, and try to identify practicable countermeasures.
>
> No maintainer or no maintainer who is debugging bug reports is the
> major problem in all parts of your list.
>
> > Another approach:  Figure out areas where quality is exemplary and try
> > to draw conclusions for areas where quality is lacking.
>
> ieee1394 has a maintainer who is looking after all bug reports he gets.
>
> Conclusion: We need such maintainers for all parts of the kernel.
>

I noticed some areas are well maintained because there is an awesome
maintainer, or good and well coordinated team - and this is mostly in
the "fun" areas ;) But there are "boring" areas that are about to be
deprecated or no new development expected etc. It will be hard to get
a dedicated person to take care of such. How about having people on
rotation, or jury duty so to speak - for a period of time (completely
voluntary!) Nice stats on the report about contributions in non-native
areas for a developer would be great accomplishment and also good
chance to look into other things! Besides, this way "old parts" will
get attention to be be revised and re-implemented sooner. And we can
post "Temp maintainer needed" list...

--Natalie

> > Stefan Richter
>
> cu
> Adrian

Re: How to improve the quality of the kernel?

[Stefan Richter]

Adrian Bunk wrote:
> On Sun, Jun 17, 2007 at 06:26:55PM +0200, Stefan Richter wrote:
>> Another approach:  Figure out areas where quality is exemplary and try
>> to draw conclusions for areas where quality is lacking.
> 
> ieee1394 has a maintainer who is looking after all bug reports he gets.

...but doesn't fix them all, and is usually slow with fixes.  He should
spend less time conversing on LKML.  :-)
-- 
Stefan Richter
-=====-=-=== -==- =---=
http://arcgraph.de/sr/

Re: How to improve the quality of the kernel?

[Adrian Bunk]

On Sun, Jun 17, 2007 at 06:26:55PM +0200, Stefan Richter wrote:
> Adrian Bunk wrote:
> >>> And we should be aware that reverting is only a workaround for the real
> >>> problem which lies in our bug handling.
> >> ...
> > 
> > And this is something I want to emphasize again.
> > 
> > How can we make any progress with the real problem and not only the 
> > symptoms?
> ...
> 
> Perhaps make lists of
> 
>   - bug reports which never lead to any debug activity
>     (no responsible person/team was found, or a seemingly person/team
>     did not start to debug the report)
> 
>   - known regressions on release,
> 
>   - regressions that became known after release,
> 
>   - subsystems with notable backlogs of old bugs,
> 
>   - other categories?
> 
> Select typical cases from each categories, analyze what went wrong in
> these cases, and try to identify practicable countermeasures.

No maintainer or no maintainer who is debugging bug reports is the 
major problem in all parts of your list.

> Another approach:  Figure out areas where quality is exemplary and try
> to draw conclusions for areas where quality is lacking.

ieee1394 has a maintainer who is looking after all bug reports he gets.

Conclusion: We need such maintainers for all parts of the kernel.

> Stefan Richter

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: How to improve the quality of the kernel?

[Rafael J. Wysocki]

On Sunday, 17 June 2007 19:42, Natalie Protasevich wrote:
> On 6/17/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Sunday, 17 June 2007 16:29, Adrian Bunk wrote:
> > > On Sun, Jun 17, 2007 at 03:17:58PM +0200, Michal Piotrowski wrote:
> > > > On 17/06/07, Adrian Bunk <bunk@stusta.de> wrote:
> > > >...
> > > >> Fine with me, but:
> > > >>
> > > >> There are not so simple cases like big infrastructure patches with
> > > >> 20 other patches in the tree depending on it causing a regression, or
> > > >> even worse, a big infrastructure patch exposing a latent old bug in some
> > > >> completely different area of the kernel.
> > > >
> > > > It is different case.
> > > >
> > > > "If the patch introduces a new regression"
> > > >
> > > > introduces != exposes an old bug
> > >
> > > My remark was meant as a note "this sentence can't handle all
> > > regressions" (and for a user it doesn't matter whether a new
> > > regression is introduced or an old regression exposed).
> > >
> > > It could be we simply agree on this one.  ;-)
> > >
> > > > Removal of 20 patches will be painful, but sometimes you need to
> > > > "choose minor evil to prevent a greater one" [1].
> > > >
> > > >> And we should be aware that reverting is only a workaround for the real
> > > >> problem which lies in our bug handling.
> > > >...
> > >
> > > And this is something I want to emphasize again.
> > >
> > > How can we make any progress with the real problem and not only the
> > > symptoms?
> >
> > I think that we can handle bug reports like we handle modifications of code.
> >
> > Namely, for each subsystem there can be a person (or a team) responsible
> > for handling bugs, by which I don't mean fixing them, but directing bug reports
> > at the right developers or subsystem maintainers, following the history of each
> > bug report etc.  [Of course, these people can choose to use the bugzilla or any
> > other bug tracking system they want, as long as it works for them.]
> >
> > The email addresses of these people should be known (and even documented),
> > so that everyone can notify them if need be and so that it's clear who should
> > handle given bug reports.
> >
> > Just an idea. :-)
> >
> 
> Those are very good ideas indeed. The whole development process came
> to the point when all realize that something needs to be done for the
> team to balance out new development and old and recent unresolved
> issues that are piling up...
> 
> I've looked through a number of bugzillas recently and here is my
> scoop on shortcomings and some ideas. I am not sure how realistic they
> are, probably might fall into "wishful thinking" category.
> 
> The way bugs get tracked and resolved is definitely a "no guarantee",
> and main reasons are:
>   not enough time for a maintainer to attend to them all
>   nobody else (except at best very few busy people) knows about
> majority of the problems. Andrew and Adrian and Michal post the most
> pressing ones. But there are many many smaller ones that are not
> assessed and not being taken care of.
>   many problems are not easily reproducible and not easy to verify
> because there is no identical system, motherboard, application, etc.
> in case if reporter doesn't stick around until the end of the bug's
> life.

I agree.  In addition, there is only a limited time window in which it makes
sense to debug given problem before the kernel changes too much (that of
course depends on the subsystem in question).

> Maybe along with bugzilla there should be another tracking tool - for
> resources and systems that are available to individual developers.

Yes, that would be very nice to have.

> Someone might have same or similar system to verify fixes in case if
> the reporter disappears or "the system is gone now". Requests for
> specific hardware can be automatically generated by the bugzilla say.
> Those can be posted once in a while for everyone to see and chip in
> and acknowledge if they happen to have such hardware and able to run a
> quick test to at least verify the patch. Statistically, such need
> doesn't happen often for each type of hardware, so it shouldn't be a
> big burden for owners.
> 
> Besides, the database and resources can be useful for developers who
> want to test their new patches on variety of hardware. This might
> prevent future regressions which often caused by lack of testing as we
> all know.

For that, I think, some "professional testers" would be needed ...

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: How to improve the quality of the kernel?

[Natalie Protasevich]

On 6/17/07, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Sunday, 17 June 2007 16:29, Adrian Bunk wrote:
> > On Sun, Jun 17, 2007 at 03:17:58PM +0200, Michal Piotrowski wrote:
> > > On 17/06/07, Adrian Bunk <bunk@stusta.de> wrote:
> > >...
> > >> Fine with me, but:
> > >>
> > >> There are not so simple cases like big infrastructure patches with
> > >> 20 other patches in the tree depending on it causing a regression, or
> > >> even worse, a big infrastructure patch exposing a latent old bug in some
> > >> completely different area of the kernel.
> > >
> > > It is different case.
> > >
> > > "If the patch introduces a new regression"
> > >
> > > introduces != exposes an old bug
> >
> > My remark was meant as a note "this sentence can't handle all
> > regressions" (and for a user it doesn't matter whether a new
> > regression is introduced or an old regression exposed).
> >
> > It could be we simply agree on this one.  ;-)
> >
> > > Removal of 20 patches will be painful, but sometimes you need to
> > > "choose minor evil to prevent a greater one" [1].
> > >
> > >> And we should be aware that reverting is only a workaround for the real
> > >> problem which lies in our bug handling.
> > >...
> >
> > And this is something I want to emphasize again.
> >
> > How can we make any progress with the real problem and not only the
> > symptoms?
>
> I think that we can handle bug reports like we handle modifications of code.
>
> Namely, for each subsystem there can be a person (or a team) responsible
> for handling bugs, by which I don't mean fixing them, but directing bug reports
> at the right developers or subsystem maintainers, following the history of each
> bug report etc.  [Of course, these people can choose to use the bugzilla or any
> other bug tracking system they want, as long as it works for them.]
>
> The email addresses of these people should be known (and even documented),
> so that everyone can notify them if need be and so that it's clear who should
> handle given bug reports.
>
> Just an idea. :-)
>

Those are very good ideas indeed. The whole development process came
to the point when all realize that something needs to be done for the
team to balance out new development and old and recent unresolved
issues that are piling up...

I've looked through a number of bugzillas recently and here is my
scoop on shortcomings and some ideas. I am not sure how realistic they
are, probably might fall into "wishful thinking" category.

The way bugs get tracked and resolved is definitely a "no guarantee",
and main reasons are:
  not enough time for a maintainer to attend to them all
  nobody else (except at best very few busy people) knows about
majority of the problems. Andrew and Adrian and Michal post the most
pressing ones. But there are many many smaller ones that are not
assessed and not being taken care of.
  many problems are not easily reproducible and not easy to verify
because there is no identical system, motherboard, application, etc.
in case if reporter doesn't stick around until the end of the bug's
life.

Maybe along with bugzilla there should be another tracking tool - for
resources and systems that are available to individual developers.
Someone might have same or similar system to verify fixes in case if
the reporter disappears or "the system is gone now". Requests for
specific hardware can be automatically generated by the bugzilla say.
Those can be posted once in a while for everyone to see and chip in
and acknowledge if they happen to have such hardware and able to run a
quick test to at least verify the patch. Statistically, such need
doesn't happen often for each type of hardware, so it shouldn't be a
big burden for owners.

Besides, the database and resources can be useful for developers who
want to test their new patches on variety of hardware. This might
prevent future regressions which often caused by lack of testing as we
all know.

There are problems that require more research and thinking such as
implementing new features or redesigning old ones. Those should be
posted as a wish list I think as invitation for constructive
discussion and as possible project for takers. They also can be
extracted from bugzilla, I ran into several ones in intermission state
like that.

And finally, the most wishful would probably be collecting test tools
that are written by and can be reused by and available to developers.
It's normally possible to find something on the Internet or write a
quick test program - and probably lots of people end up writing little
programs to allocate shared memory and exercise it in certain way or
some affinity tool etc. But sometimes people come up with pretty
elaborate ones - why won't we attempt to sort out those test programs,
have them contributed (and maybe not code reviewed! - just as is, take
it or leave it :) and have them handy for better and faster bug
fixing/testing. And again - there are times we wish for such tool or
emulator and don't have spare cycles, so those type of requests for
custom test scripts and programs can also be posted.

I also had on mind what to do about maintainers and project teams and
alternative contacts who can handle issues on a particular module or
subsystem. Probably list or database of volunteers can be arranged,
this is something that is really needed. I can relate after trying to
get hold of alternative people myself...

Regards,
--Natalie

Re: How to improve the quality of the kernel?

[Rafael J. Wysocki]

On Sunday, 17 June 2007 16:29, Adrian Bunk wrote:
> On Sun, Jun 17, 2007 at 03:17:58PM +0200, Michal Piotrowski wrote:
> > On 17/06/07, Adrian Bunk <bunk@stusta.de> wrote:
> >...
> >> Fine with me, but:
> >>
> >> There are not so simple cases like big infrastructure patches with
> >> 20 other patches in the tree depending on it causing a regression, or
> >> even worse, a big infrastructure patch exposing a latent old bug in some
> >> completely different area of the kernel.
> >
> > It is different case.
> >
> > "If the patch introduces a new regression"
> >
> > introduces != exposes an old bug
> 
> My remark was meant as a note "this sentence can't handle all 
> regressions" (and for a user it doesn't matter whether a new 
> regression is introduced or an old regression exposed).
> 
> It could be we simply agree on this one.  ;-)
> 
> > Removal of 20 patches will be painful, but sometimes you need to
> > "choose minor evil to prevent a greater one" [1].
> > 
> >> And we should be aware that reverting is only a workaround for the real
> >> problem which lies in our bug handling.
> >...
> 
> And this is something I want to emphasize again.
> 
> How can we make any progress with the real problem and not only the 
> symptoms?

I think that we can handle bug reports like we handle modifications of code.

Namely, for each subsystem there can be a person (or a team) responsible
for handling bugs, by which I don't mean fixing them, but directing bug reports
at the right developers or subsystem maintainers, following the history of each
bug report etc.  [Of course, these people can choose to use the bugzilla or any
other bug tracking system they want, as long as it works for them.]

The email addresses of these people should be known (and even documented),
so that everyone can notify them if need be and so that it's clear who should
handle given bug reports.

Just an idea. :-)

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: How to improve the quality of the kernel?

[Michal Piotrowski]

On 17/06/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Adrian Bunk wrote:
> >>> And we should be aware that reverting is only a workaround for the real
> >>> problem which lies in our bug handling.
> >> ...
> >
> > And this is something I want to emphasize again.
> >
> > How can we make any progress with the real problem and not only the
> > symptoms?
> ...
>
> Perhaps make lists of
>
>   - bug reports which never lead to any debug activity
>     (no responsible person/team was found, or a seemingly person/team
>     did not start to debug the report)
>
>   - known regressions on release,
>
>   - regressions that became known after release,
>
>   - subsystems with notable backlogs of old bugs,
>
>   - other categories?

It is unworkable in wiki.

There is a new regression field in bugzilla, but it is only the first
step to implement regression tracking feature.

>
> Select typical cases from each categories, analyze what went wrong in
> these cases, and try to identify practicable countermeasures.
>
> Another approach:  Figure out areas where quality is exemplary and try
> to draw conclusions for areas where quality is lacking.
> --
> Stefan Richter
> -=====-=-=== -==- =---=
> http://arcgraph.de/sr/
>

Regards,
Michal

-- 
LOG
http://www.stardust.webpages.pl/log/

Re: How to improve the quality of the kernel?

[Stefan Richter]

Adrian Bunk wrote:
>>> And we should be aware that reverting is only a workaround for the real
>>> problem which lies in our bug handling.
>> ...
> 
> And this is something I want to emphasize again.
> 
> How can we make any progress with the real problem and not only the 
> symptoms?
...

Perhaps make lists of

  - bug reports which never lead to any debug activity
    (no responsible person/team was found, or a seemingly person/team
    did not start to debug the report)

  - known regressions on release,

  - regressions that became known after release,

  - subsystems with notable backlogs of old bugs,

  - other categories?

Select typical cases from each categories, analyze what went wrong in
these cases, and try to identify practicable countermeasures.

Another approach:  Figure out areas where quality is exemplary and try
to draw conclusions for areas where quality is lacking.
-- 
Stefan Richter
-=====-=-=== -==- =---=
http://arcgraph.de/sr/

Re: How to improve the quality of the kernel?

[Michal Piotrowski]

On 17/06/07, Adrian Bunk <bunk@stusta.de> wrote:
> On Sun, Jun 17, 2007 at 03:17:58PM +0200, Michal Piotrowski wrote:
> > On 17/06/07, Adrian Bunk <bunk@stusta.de> wrote:
> >...
> >> Fine with me, but:
> >>
> >> There are not so simple cases like big infrastructure patches with
> >> 20 other patches in the tree depending on it causing a regression, or
> >> even worse, a big infrastructure patch exposing a latent old bug in some
> >> completely different area of the kernel.
> >
> > It is different case.
> >
> > "If the patch introduces a new regression"
> >
> > introduces != exposes an old bug
>
> My remark was meant as a note "this sentence can't handle all
> regressions" (and for a user it doesn't matter whether a new
> regression is introduced or an old regression exposed).
>
> It could be we simply agree on this one.  ;-)
>
> > Removal of 20 patches will be painful, but sometimes you need to
> > "choose minor evil to prevent a greater one" [1].
> >
> >> And we should be aware that reverting is only a workaround for the real
> >> problem which lies in our bug handling.
> >...
>
> And this is something I want to emphasize again.
>
> How can we make any progress with the real problem and not only the
> symptoms?
>
> There's now much money in the Linux market, and the kernel quality
> problems might result in real costs in the support of companies like
> IBM, SGI, Redhat or Novell (plus it harms the Linux image which might
> result in lower revenues).
>
> If [1] this is true, it might even pay pay off for them to each assign
> X man hours per month of experienced kernel developers to upstream
> kernel bug handling?
>
> This is just a wild thought and it might be nonsense - better
> suggestions for solving our quality problems would be highly welcome...

Just one comment.

We don't try to recruit new skilled testers - it's a big problem.
Skilled tester can narrow down the problem, try to fix it etc. There
are too many "something between 2.6.10 and 2.6.21 broke my laptop"
reports...

Regards,
Michal

-- 
LOG
http://www.stardust.webpages.pl/log/

How to improve the quality of the kernel?

[Adrian Bunk]

On Sun, Jun 17, 2007 at 03:17:58PM +0200, Michal Piotrowski wrote:
> On 17/06/07, Adrian Bunk <bunk@stusta.de> wrote:
>...
>> Fine with me, but:
>>
>> There are not so simple cases like big infrastructure patches with
>> 20 other patches in the tree depending on it causing a regression, or
>> even worse, a big infrastructure patch exposing a latent old bug in some
>> completely different area of the kernel.
>
> It is different case.
>
> "If the patch introduces a new regression"
>
> introduces != exposes an old bug

My remark was meant as a note "this sentence can't handle all 
regressions" (and for a user it doesn't matter whether a new 
regression is introduced or an old regression exposed).

It could be we simply agree on this one.  ;-)

> Removal of 20 patches will be painful, but sometimes you need to
> "choose minor evil to prevent a greater one" [1].
> 
>> And we should be aware that reverting is only a workaround for the real
>> problem which lies in our bug handling.
>...

And this is something I want to emphasize again.

How can we make any progress with the real problem and not only the 
symptoms?

There's now much money in the Linux market, and the kernel quality 
problems might result in real costs in the support of companies like
IBM, SGI, Redhat or Novell (plus it harms the Linux image which might 
result in lower revenues).

If [1] this is true, it might even pay pay off for them to each assign 
X man hours per month of experienced kernel developers to upstream 
kernel bug handling?

This is just a wild thought and it might be nonsense - better 
suggestions for solving our quality problems would be highly welcome...

cu
Adrian

[1] note that this is an "if"

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed