Date: Wed, 1 Mar 2000 01:04:01 +0100 From: Bernd Walter <ticso@cicely5.cicely.de> To: Alfred Perlstein <bright@wintelcom.net> Cc: Greg Lehey <grog@lemis.com>, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org, Bernd Walter <ticso@cicely.de> Subject: Re: cvs commit: src/sys/dev/vinum vinumrequest.c Message-ID: <20000301010400.A30259@cicely5.cicely.de> In-Reply-To: <20000229011737.C21720@fw.wintelcom.net> References: <200002290614.WAA16809@freefall.freebsd.org> <20000229002459.B21720@fw.wintelcom.net> <20000229183706.D16629@freebie.lemis.com> <20000229011737.C21720@fw.wintelcom.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Feb 29, 2000 at 01:17:37AM -0800, Alfred Perlstein wrote: > * Greg Lehey <grog@lemis.com> [000229 00:37] wrote: > > On Tuesday, 29 February 2000 at 0:25:00 -0800, Alfred Perlstein wrote: > > > * Greg Lehey <grog@FreeBSD.org> [000228 22:45] wrote: > > >> grog 2000/02/28 22:14:45 PST > > >> > > >> Modified files: > > >> sys/dev/vinum vinumrequest.c > > >> Log: > > >> launch_requests: If too many requests are active, include PCATCH in > > >> the tsleep call flags. > > >> > > >> Submitted-by: Bernd Walter <ticso@cicely.de> > > > > > > grrr! > > > > > > while ((drive->active >= DRIVE_MAXACTIVE) /* it has too much to do already, */ > > >>> (vinum_conf.active >= VINUM_MAXACTIVE)) /* or too many requests globally */ > > > - tsleep(&launch_requests, PRIBIO, "vinbuf", 0); /* wait for it to subside */ > > > + tsleep(&launch_requests, PRIBIO | PCATCH, "vinbuf", 0); /* wait for it to subside */ > > > drive->active++; > > > > > > please back this out, i've already fixed this twice, if you actually > > > get a signal you'll spin in the kernel. > > > > > > the correct solution must check the return from tsleep and actually > > > do something with an error return, for now we've agreed to ignore > > > signals until a safe return can be implemented. PCATCH must be > > > removed. > > > > OK, make a suggestion. > > Remove PCATCH. :) > > > > > The trouble is, you and Bernd keep asking for opposite things, and I > > was getting out of phase on the whole matter. In any case, another > > change I made (MAXREQUESTS = 30000) means that we'll never call tsleep > > here. > > The point is that PCATCH never works here, and that there shouldn't > be landminds in the code, if someone where to reduce MAXREQUESTS > to test something they'd have a good chance of getting hit. It never worked here too. But what do you want? Greg already removed it: bash-2.03$ cd /var/d7/src-2000-02-12/src/sys/dev/vinum bash-2.03$ grep PCATCH * vinumconfig.c: if ((error = tsleep(&vinum_conf, PRIBIO | PCATCH, "vincfg", 0)) != 0) I don't know if the same applies for config - but your point is corrected. > Bernd can have his system lockup and I won't really care. This > isn't like half the time PCATCH here will work and do something > benificial, <em>it will lock up every single time</em>. If Bernd > wants this to catch signals here, then he should supply diffs to > act properly when the event happens instead of fatally spinning > in the kernel wedging my boxes. No - it's different - You found the PCATCH issue and I removed it here and was happy that it worked and even removed it at another place in R5 locking where it had biten me earlier too. The problem was that I send Greg diffs which included the change for the PCATCH which was already commited and Greg accidantly revertet it. It was never my intention to put PCATCH back! Maybe PCATCH is good if implemented differently - but all I know is that the current code does not hang or panic the system. > I'm going to spend a couple minutes looking it over to see if I > can come up with a proper fix, but if no-one steps up with patches > to correctly handle the signal then the PCATCH flag should be removed. If you come up with a proper PCATCH implementation it may be fine, but there is really nothing left to remove. Maybe you want to take a lock at vinumconfig.c too... -- B.Walter COSMO-Project http://www.cosmo-project.de ticso@cicely.de Usergroup info@cosmo-project.de 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?20000301010400.A30259>