On Mon, Aug 20, 2001, Pete Zaitcev <zaitcev@redhat.com> wrote:
> A prolifiration of subtly different versions of basic primitives
> is not an answer either. But wait, here's a better fix. The
> root of the evil is that the waiting thread accesses urb->status
> before a callback happened, which is unsafe.
>
> BTW, I took a liberty to clean the thing up a bit. It looked as
> if the author of that fragment was not sure of what he was doing,
> and the style was quite dirty. I think a number of wrongs for
> such a small fragment was astonishing.
>
> - "status" was an errno in the begining, then 5 lines down
> it's bool (== timeout), then it turns into system style once again.
> - Wasted pointer to wait head
> - Unused void *stuff.
> - typedef without _t and unprefixed type name in global header.
> - Urban legend of test for waitqueue_active() before wakeup.
> This is one half wrong because so many people do it,
> anyone has an idea why? Half a point deducted.
> - Confused and redundant checking for -EINPROGRESS
> (even if it was the cause of oops)
>
> And the last one ...
> - THE GOD DAMN RACE THAT OOPS - that's 10 hacker points down!
> It only goes to show that replacing interruptible_sleep_on
> with add_wait_queue/schedule/remove_wait_queue does not make
> any racy code correct automatically.
>
> Cheesh, I am surprised _anything_ in Linux kernel works.
So am I. To be honest, there's a bunch of things in the USB code I'm not
entirely happy.
I didn't see many of them until recently, but I guess with experience,
comes wisdom. I had intended to fix many of them in 2.5 when it finally
forks.
I like your patch, but since we have the new completion stuff now, we
should probably use that. I'll make the mod and send off the patch to
Linus when I get back from this business trip.
Thanks.
JE
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Thu Aug 23 2001 - 21:00:39 EST