Date: Fri, 31 Jan 2003 13:42:49 -0800 (PST) From: Julian Elischer <julian@elischer.org> To: John Baldwin <jhb@FreeBSD.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: <Pine.BSF.4.21.0301311326340.45015-100000@InterJet.elischer.org> In-Reply-To: <XFMail.20030131160908.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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: > >> > > >> > > >> > 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. You aren't reading are you? > > > 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. So what's stopping you? > > > 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. OK so what we are saying is that to be a developer we need to be on IRC? if this is so, then the project should supply an IRC server for us to use. > > > 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. I object to your continual comment that it was committed with my stamp of aproval. I have already said that the commit was in error and due to a misunderstanding. I never said to commit it, I just asked him if he was going to do it or I was going to do it. At this stage, I've gone through it as much as I'm ever going to go through it and the only outstanding thing I see is the use of 'tick' in thread_statclock() (which I will shortly correct). I have a slight doubt as to whether using a thread* instead of a pid in lockmgr is correct, but I can not see any reason why it won't work at this time. 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?Pine.BSF.4.21.0301311326340.45015-100000>