Message-ID: <d9def9db0706220501k4a1bf8d2wdf62a9638a41829b@mail.gmail.com>
Date:	Fri, 22 Jun 2007 14:01:52 +0200
From:	"Markus Rechberger" <mrechberger@...il.com>
To:	"Natalie Protasevich" <protasnb@...il.com>
Cc:	"Adrian Bunk" <bunk@...sta.de>,
	"Stefan Richter" <stefanr@...6.in-berlin.de>,
	"Michal Piotrowski" <michal.k.k.piotrowski@...il.com>,
	"Oleg Verych" <olecom@...wer.upol.cz>,
	"Linus Torvalds" <torvalds@...ux-foundation.org>,
	"Andi Kleen" <andi@...stfloor.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	"Diego Calleja" <diegocg@...il.com>,
	"Chuck Ebbert" <cebbert@...hat.com>,
	"Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
	"Andrew Morton" <akpm@...ux-foundation.org>
Subject: Re: How to improve the quality of the kernel?

On 6/17/07, Natalie Protasevich <protasnb@...il.com> wrote:
> On 6/17/07, Adrian Bunk <bunk@...sta.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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists