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

Alexander Leidinger wrote:
> Quoting Suleiman Souhlal <ssouhlal_at_FreeBSD.org> (Tue, 15 Aug 2006 14:53:56 +0200):
> 
> I'm doing post-commit checks now (everything is commited and I'm doing
> a kernel build now)...
> 
> 
>>Alexander Leidinger wrote:
>>
>>
>>>I already started... and I don't want to commit some parts (the linker
>>>stuff which allows to use the module on amd64).
>>
>> >
>>
>>>It is also not used by default, as long as we have 2.4.2 for the linux
>>>version, no new functions will be used by glibc. So there is no change
>>>in behavior after the commits. I tested with acroread (which has issues
>>>when run with a 2.6.16 compat.linux.osrelease version, where the new
>>>functions are used by glibc).
>>
>> >
>>
>>>So this gives us:
>>> - coverity reports for the code *before the end of the SoC*
>>
>>Why the rush? I'm sure Roman will be around even when the SoC is over.
> 
> 
> Yes, but first he will take a vacation, and then he will not be
> available as much as he is now. And maybe Coverity will detect a bug
> which we search currently but can't find (don't know how likely this
> is, but it is at least possible).

Then maybe we need to change our Coverity setup so that it's able to 
look at trees in perforce as well.

> 
>>Also, I'm not asking you to wait a month, just a couple of days until 
>>more people have had a chance to look at the changes.
> 
> 
> I thought about this before deciding to commit it. Most of the code was
> available in P4. Those people with real interest already had a chance
> to look at it, or they didn't had time to do so. If they didn't had
> time to have a look at it, who knows if they have time to do it now?

Not really. I haven't looked at the changes in p4 not because I'm not 
interested, but because they were a work in progress. I'd rather look at 
"commit canditates".

> There's always someone who says "oh... if you waited 2 days longer...".
> 
> That's not the only reason. Tomorrow my holiday is over and I have to
> catch up at work. Today I have plenty of time to commit urgent issues
> (e.g., tinderbox breakage). And at the upcomming WE I may not have
> enough time to do what I do today.

If you don't have time to commit something you can always ask someone 
else to do it for you.

> Additionally, the work is a little bit stalled currently, we need more
> testers of the new code. There are a lot of people out there which want
> to test, but don't know how to patch or fear to do something wrong when
> they patch. With the approach I've taken, they just need to run a
> sysctl (compat.linux.osrelease=2.6.16) and they can test. All which
> don't want to test just don't need to care about it.

You could have released a sys/ snapshot tarball?

> 
>>It's a bit unreasonable to say "here's a patch, please look at it" and 
>>commit it less than a day later.
> 
> 
> Yes, it was a little bit fast...
> 
> 
>>> - no change in behavior in the default case, since the new calls
>>>   aren't used by glibs as long as... see following entry
>>> - easy testing possibility (sysctl compat.linux.osrelease=2.6.16,
>>>   defaults to 2.4.2)
>>> - more eyes on the code
>>
>>Those are not valid reasons to commit unreviewed and potentially wrong
>>changes.
> 
> 
> Uhm... how many percent of code committed to current is reviewed before
> it is commited?

Not enough.

> 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.

> I'm running a kernel with this changes here and since I didn't enabled
> the new code with the sysctl, I don't see any difference in the
> behavior of the linuxolator. The potentially wrong code isn't active.
> It's like loading the linprocfs module but not using it.
> 
> Bye,
> Alexander.
> 

-- Suleiman