Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 31 Jan 2003 16:09:08 -0500 (EST)
From:      John Baldwin <jhb@FreeBSD.org>
To:        Julian Elischer <julian@elischer.org>
Cc:        cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org, "Tim J. Robbins" <tjr@FreeBSD.org>
Subject:   RE: cvs commit: src/sys/kern subr_trap.c
Message-ID:  <XFMail.20030131160908.jhb@FreeBSD.org>
In-Reply-To: <Pine.BSF.4.21.0301311251330.45015-100000@InterJet.elischer.org>

next in thread | previous in thread | raw e-mail | index | archive | help

On 31-Jan-2003 Julian Elischer wrote:
> 
> 
> On Fri, 31 Jan 2003, John Baldwin wrote:
> 
>> 
>> On 31-Jan-2003 Julian Elischer wrote:
>> > 
>> > 
>> > On Fri, 31 Jan 2003, John Baldwin wrote:
>> > 
>> >> 
>> >> On 31-Jan-2003 Julian Elischer wrote:
>> >> > 
>> >> > do you know of any other problems?
>> >> 
>> >> thread_statclock() has a similar problem as others have noticed:
>> > 
>> > what others?
>> > 
>> > why doesn't anyone tell ME these things?
>> > is there some discussion going on in a places I don't see? like IRC?
>> 
>> -----
>> revision 1.77
>> date: 2003/01/26 11:41:34;  author: davidxu;  state: Exp;  lines: +646 -513
>> Move UPCALL related data structure out of kse, introduce a new
>> data structure called kse_upcall to manage UPCALL. All KSE binding
>> and loaning code are gone.
>> ...
>> 
>> Reviewed by: julian
>> -----
>> 
>> Read that last line.  What does that mean to you?  To me it means
>> that you have looked at the actual diff and approved it.  You didn't
>> see any bugs in it, etc.  Now, either you didn't look very hard, or
>> that last line is a lie.  Either way, I want this commit backed out
>> until such time as it has a 'Reviewed by' line that means something.
>> It's not enough to just keep fixing the bugs that others find when
>> they crop up.  Saying that you reviewed something means that you
>> should be taking responsibility to test things out and really look
>> at them before they are committed.
> 
> John I'm not saying that the patch shouldn't have been tested more, and
> in fact my name shouldn't have been there really because I didn't say 
> "go ahead and commit" I said "my machine is running, it's looking good
> so far, who will be responsible for the commit"? This was not as clear
> as I hoped and David took it (English is not his first language) to be
> "go ahead and commit".

You are David's mentor yes?  Ok, then you are responsible when this
happens.  If you do not hold David responsible, then the Project is
going to hold you responsible in his stead.  That's what being a mentor
means.

> Having had teh commit however and it seems t be
> working now, I don;t think it shouldbe backed out at this stage..

It's working for some value of working.  The fact is, given the subtle
bug that broke profiling and the fact that the same type of bug is
evident in other places and the fact that it was only tested on i386
UP, I feel that the probability is high that there are similar bugs in
this commit that we just haven't hit yet and that could be found with
a good review.

> What I want to know is where did you hear other people pointing out 
> that "thread_statclock() has a similar problem"?

I saw a note on IRC in my scrollback (I wasn't present at the time) that
it might have a similar problem and looked at the code in question along
with the diff's and annotates of previous versions to verify that it was
an actual bug.

> I seem to only be seeing a part of the discussion here..
> people keep refering to error reports by people that I have not seen.

These are bugs that you would hopefully be catching if you were reviewing
the things that get committed with your stamp of approval on them.

-- 

John Baldwin <jhb@FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20030131160908.jhb>