Date: Sun, 23 Jun 2002 16:58:09 -0500 (CDT) From: Jonathan Lemon <jlemon@flugsvamp.com> To: dillon@apollo.backplane.com, hackers@freebsd.org Subject: Re: Bug in wakeup() (stable and current) ? Message-ID: <200206232158.g5NLw9c49030@prism.flugsvamp.com> In-Reply-To: <local.mail.freebsd-hackers/200206232032.g5NKWVZW063483@apollo.backplane.com> References: <local.mail.freebsd-hackers/200206232014.g5NKE5x3058562@apollo.backplane.com> <local.mail.freebsd-hackers/20020623201933.GM53232@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In article <local.mail.freebsd-hackers/200206232032.g5NKWVZW063483@apollo.backplane.com> you write:
>:I'm pretty sure you only need to 'goto restart' if you call into
>:maybe_resched() as someone else may have manipulated the queues.
>:
>:The 'restart' label is only in there for restarting in case one of
>:the functions called may change the lists, if we restart _every_
>:time we'll traverse the same procs where p->p_wchan != ident over
>:and over needlessly.
>:
>:-Alfred
>
> Look at the code carefully. It's *removing* the element from the list,
> the conditionally restarting rather then removing the element from the
> list and unconditionally restarting. The only reason it works at all
> is because sys/queue.h does not clear out the pointers in the node
> that was just removed. The code is just plain wrong, though, because
> the queue mechanisms make no such (documented) guarentee.
Looks like the original damage happened in r1.21, where the temporary
variable (used to hold the next item on the list) was replaced by a
dereference through the pointer of the item that was just removed.
The code works simply because it relies TAILQ_REMOVE() not changing
the tqe_next pointer. I suppose that this should either be documented,
or the loop changed back to use a temp variable:
for (td = TAILQ_FIRST(qp); td != NULL; td = tdq) {
tdq = TAILQ_NEXT(td, td_slpq);
...
}
--
Jonathan
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200206232158.g5NLw9c49030>