Date: Tue, 29 Feb 2000 19:28:54 +1030 From: Greg Lehey <grog@lemis.com> To: Alfred Perlstein <bright@wintelcom.net> Cc: 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: <20000229192854.E16629@freebie.lemis.com> 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 Tuesday, 29 February 2000 at 1:17:37 -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. :) And Bernd? I must confess, I got your two reports confused. Both of you said that it had to be that way round or the system would lock up, and I wasn't able to reproduce it here. >> 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. OK, we can remove it again. > Bernd can have his system lockup and I won't really care. But I would. > This isn't like half the time PCATCH here will work and do something > benificial, <em>it will lock up every single time</em>. No, that's not correct. I do test these things before I commit them, and currently I'm running a Vinum root quite happily with this code. First you need to have a signal. > 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. The real issue is that we need to find out what Bernd's problem is. > 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. Yes, that makes sense. I'll do it first thing tomorrow morning if you don't send me patches in the meantime. Greg -- Finger grog@lemis.com for PGP public key See complete headers for address and phone numbers 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?20000229192854.E16629>