Date: Mon, 7 Aug 2000 11:26:12 -0700 From: Alfred Perlstein <bright@wintelcom.net> To: Poul-Henning Kamp <phk@critter.freebsd.dk> Cc: Matt Dillon <dillon@earth.backplane.com>, Mike Smith <msmith@FreeBSD.ORG>, Stephen McKay <mckay@thehub.com.au>, freebsd-current@FreeBSD.ORG, dillon@FreeBSD.ORG Subject: Re: Ugly, slow shutdown Message-ID: <20000807112612.V4854@fw.wintelcom.net> In-Reply-To: <10026.965667788@critter.freebsd.dk>; from phk@critter.freebsd.dk on Mon, Aug 07, 2000 at 07:03:08PM %2B0200 References: <200008071647.JAA36775@earth.backplane.com> <10026.965667788@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
* Poul-Henning Kamp <phk@critter.freebsd.dk> [000807 10:03] wrote:
> In message <200008071647.JAA36775@earth.backplane.com>, Matt Dillon writes:
> >:> * Stephen McKay <mckay@thehub.com.au> [000805 08:49] wrote:
> >:> >
> >:> > Patch 2 is smaller and possibly controversial. Normally bufdaemon and
> >:> > syncer are sleeping when they are told to suspend. This delays shutdown
> >:> > by a few boring seconds. With this patch, it is zippier. I expect people
> >:> > to complain about this shortcut, but every sleeping process should expect
> >:> > to be woken for no reason at all. Basic kernel premise.
> >:>
> >:> You better bet it's controversial, this isn't "Basic kernel premise"
> >:
> >:Actually, that depends. It is definitely poor programming practice to
> >:not check the condition for which you slept on wakeup.
> >:
> >:> *boom* *crash* *ow* :)
> >:
> >:Doctor: So don't do that.
> >
> > I gotta agree. This is very bad programming practice. There are many,
> > many places in the kernel where tsleep() is called with a 0 delay and
> > assumed not to return until something meaningful happens. For example,
> > for handling NFS retries, some of the locking code (I think), and I
> > could probably find many others.
>
> Then this code should be changed to do the right thing, which is
> to *always* check the condition being slept on before proceeding.
Can you give a reason why we'll have to now start coding defensively
because our arguments to tsleep() are just "advisory" now?
I'm not really sure why for a single reader/writer situation we have
to have hysterics for a stray wakeup, it bloats code and is not needed
in all places.
I can also imagine some fun infinite loops like so:
monitor issues wakeup
producer wakes and terminates or goes away
consumer spins checking on availability
Also, one must now do this?
timeo = currenttime + 2;
while (timeo != currenttime)
tsleep(timeo);
?
-Alfred
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000807112612.V4854>