From owner-cvs-all Fri Jan 31 13:42:55 2003 Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DBE9537B401; Fri, 31 Jan 2003 13:42:51 -0800 (PST) Received: from rwcrmhc51.attbi.com (rwcrmhc51.attbi.com [204.127.198.38]) by mx1.FreeBSD.org (Postfix) with ESMTP id 66DD743F75; Fri, 31 Jan 2003 13:42:51 -0800 (PST) (envelope-from julian@elischer.org) Received: from InterJet.elischer.org (12-232-168-4.client.attbi.com[12.232.168.4]) by rwcrmhc51.attbi.com (rwcrmhc51) with ESMTP id <2003013121425005100hsrdle>; Fri, 31 Jan 2003 21:42:51 +0000 Received: from localhost (localhost.elischer.org [127.0.0.1]) by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id NAA47078; Fri, 31 Jan 2003 13:42:50 -0800 (PST) Date: Fri, 31 Jan 2003 13:42:49 -0800 (PST) From: Julian Elischer To: John Baldwin Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org, "Tim J. Robbins" Subject: RE: cvs commit: src/sys/kern subr_trap.c In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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