On Sun, 2 Dec 2001, Pete Zaitcev wrote:
> > - the ioctl blocks (no, not sleep_on) on the completion (i.e. until
> > urb->status becomes != EINPROGRESS).
>
> BTW, This is broken. The ioctl must block until a callback
> is triggered. It is safe to examing urb->status only after
> the callback awakens the requesting thread or inside the callback.
Hi Pete,
thanks for pointing this out - yes, I've seen Oliver's sleep_on fixes
did address urb->status issues as well. However, I haven't done the
changes in my codebase yet - mostly because I'm somewhat lost trying
to understand what the problem really is.
Maybe the description above was too short - the real code is this:
add_wait_queue(&wqh,&wait);
for(;;) {
set_current_state(TASK_INTERRUPTIBLE);
if (urb->status != -EINPROGRESS)
break;
if (signal_pending(current))
break;
schedule();
if (drv->disconnect_pending)
break;
}
remove_wait_queue(&wqh,&wait);
set_current_state(TASK_RUNNING);
wake_up_interruptible(&wqh) is called from urb->complete.
Sure, urb->status is modified from interrupt context. But the read for
comparison should be atomic (although not explicitly). Furthermore
urb->status is the translated result, i.e. not part of the TD which might
require pci-sync to re-establish coherency for the bus mapping. With the
right application of the waitqueue I don't see things going wrong on SMP.
The only thing I'm wondering is whether urb->status would need to be
qualified volatile to prevent the compiler from bad assumptions. But I've
never seen a problem pointing into that direction. And I don't see what
makes urb->status all different from drv->can_go_on_flag - both are
modified from interrupt context (I'm not talking about the new hcd
approach where the completion handler is scheduled as a tasklet, IIRC).
What have I missed? Would you (or somebody else) mind to explain?
TIA
Martin
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
