On Sun, 23 Jun 2002, Jonathan Lemon wrote:
> In article
><local.mail.freebsd-hackers/[EMAIL PROTECTED]> 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);
> ...
> }
I just added debug code in the TAILQ code that sets the forward pointor
to -1. Since Matt had this it's possible that this is what hit him?
I do this to stop people accessingthings that they shouldn't be counting
on..
>
> --
> Jonathan
>
> To Unsubscribe: send mail to [EMAIL PROTECTED]
> with "unsubscribe freebsd-hackers" in the body of the message
>
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
