On Thursday, 3. May 2001 00:54, Martin Diehl wrote:
> On Wed, 2 May 2001, Oliver Neukum wrote:
> > as was recently discussed, sleep_on and derivates, which have a little
> > race problem are used 21 times in drivers/usb
>
> and 12 more in drivers/usb/serial ;-)
Oh boy.
> > The attached patch should fix one such place in printer.
>
> Don't want to be pedantic, but there are some more issues:
>
> - current state should be set to TASK_INTERRUPTIBLE for each loop
> traversing. schedule() always returns RUNNABLE, so we would only
> sleep once.
How could the condition be true, once we are woken up ?
But there is a bug, pending signals are not check for after schedule, but
before.
> - memory barriers required to make sure we are really on the wait_queue
> before testing whether to sleep - otherwise there would be a race in
> case the interrupt/wakeup is executed on other CPU
Right.
> - better use set_current_state() - it provides the memory barrier when
> placed between add_wait_queue() and sleep/nosleep decision
Right.
> - sleep/nosleep decision must be atomic. If this is enforced by a spinlock
> we may use __set_current_state() (which does not serialize).
What for ? There just must not be a false negative, which the order adding to
queue and checking will ensure.
Please explain.
Either you trust gcc to write status in one write or you don't. To ensure
what you want it would have to be atomic_t. A spinlock won't help.
> - not only lpusb_read() uses sleep_on(), similar for lpusb_write() but
> with timeout
>
> the updated patch below (vs. 2.4.4, -ac should be ok) addresses these
> issues.
>
> > It even compiles.
>
> and nothing appears to be broken - at least for my HL1250 (2 EP).
>
> > 20 more to go. People let's kill them all.
>
> Good idea anyway!
>
> Martin
This patch is better, but would you think about the spinlocks ?
Regards
Oliver
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
http://lists.sourceforge.net/lists/listinfo/linux-usb-devel
