Re: HEADS-UP: starting to commit linuxolator (SoC 2006) changes...

Quoting Robert Watson <rwatson_at_FreeBSD.org> (Tue, 15 Aug 2006 21:28:58 +0100 (BST)):

> 
> On Tue, 15 Aug 2006, Alexander Leidinger wrote:
> 
> >>> How many percent of the code I committed is not reviewed and how much is 
> >>> this related to the amount of unreviewed code committed in a busy week?
> >>
> >> I don't want to sound harsh or like I'm doubting Roman's ability, but most 
> >> of the unreviewed code that gets committed comes from people who have more 
> >> experience than Roman and who usually "own" the part of the kernel they are 
> >> touching.
> >
> > This doesn't help when the code in question isn't owned like the 
> > linuxolator. Because most of the new code will not affect the rest of the 
> > system (only linux programs, if at all), I don't think committing this now 
> > was a big mistake.
> 
> I'd like to see this handled better in the future, myself.  In the past 24 
> hours several reviews have come in -- unfortunately, all after the commit, 
> because almost no time was given for the reviews to be done before the commit. 
> Given that there have been several replies on the perforce list indicating 
> that sections of the changes weren't yet in an acceptable form for committing 
> (i.e., broken), waiting for that to settle out before committing makes a lot 
> of sense.  -CURRENT is here to provide communal testing, but that doesn't mean 
> that broken code should be committed intentionally.

A lot of the new code isn't used at all if nobody enables the use
(sysctl compat.linux.osrelease=2.6.16). The code which is changed and
is used even with osrelease=2.4.2 is not much. Reviews of those parts
of the code didn't revealed much (mainly name change suggestions and
style issues). Reviews of the unused by default part of the code
revealed some issues. Some of them may be related to what we see as
bugs. The parts of the unused by default code which we know is not bug
free is important (futexes), but not totally broken. When activating
this unused by default code, a lot of programs work, which didn't
worked when changing osrelease before. These programs are some small
ones, like bash, ls and other stuff which comes with linux_base-fc4 and
some large and complex ones like Firefox and Opera.

So a lot of the new code already works (if you activate it) and only
parts of it aren't bug free (ok, this may be a little bit over
enthusiastic, I can't prove that there are no bugs, but I think you get
the point).

I also checked if the programs with issues just work fine as before
when the new code isn't activated. So I decided to commit this, after I
checked that I don't introduce a regression (it is very easy to let
acroread fail if you have a PDF file which exposes the bug). If this
wouldn't have been the case, I wouldn't have committed this.

So yes, I have committed code which is known to be broken. But not
everything is broken, and it only breaks programs, if you tell them to
use new features *and* if you use programs which trigger the bugs. And
the best thing is, you don't need to rebuild your kernel to switch
(like with SCHED_BSD and SCHED_ULE, for example). Apart from this, the
linuxolator is not the VM, VFS or some other high risk subsystem. If it
breaks, don't use it. I'm not aware of a linux application which is
necessary to develop changes for the rest of FreeBSD. So this does not
slow down other developers (except they want to participate in this
discussion...).

Robert, I agree with you about the higher standards in -current, and as
such I made sure that it doesn't break the world or panics the system
in the default case. But after all, this is -current and not -stable. I
use it on my desktop system since 3-current, but I don't expect it to
work after an update. If a change by someone else breaks my desktop, it
is my own fault. It is not a production quality branch.

We can't expect everything to be bug free right away, and to fix some
bugs we may need some more use cases until we can narrow down the
problem. If this can be done by committing code to -current and
enabling it with a switch, a lot of people will give it a try. If it is
only available as a patch, you will not get nearly as much feedback.

Instead of talking about the commits, I suggest to give it a try and to
immediately report regressions in the osrelease=2.4.2 case (that's the
default). So far we are not aware of regressions.

Roman already fixed some issues in p4 which showed up in immediate
reviews. Some of them are NOPs from a functional point of view (style,
moving code to other places, ...), some of them fix races in the new
(and unused by default) code.


For the curious ones: the code is "activated" by changing osrelease,
because the glibc behaves differently depending on the osrelease. It
makes different syscalls based upon the linux kernel version. So if
there's a program which tries if a syscall does not return ENOSYS
instead of checking the osrelease, it may fail. If you are aware of
such a program, please tell us about it.

Bye,
Alexander (going to bed now).

-- 
It is better to be part of the idle rich class
than be part of the idle poor class.
http://www.Leidinger.net  Alexander _at_ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org     netchild _at_ FreeBSD.org  : PGP ID = 72077137