Am Montag 03 Dezember 2001 13:33 schrieb Martin Diehl:
> 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.
In theory you could be checking urb->status before it becomes -EINPROGRESS.
Anyway it's a layering violation and should not be done.
> 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
I think schedule() takes care of that, but I am not sure.
Regards
Oliver
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
