On 06/21/2018 10:29 AM, Roger Quadros wrote:
[...]
>>> static int ffs_aio_cancel(struct kiocb *kiocb)
>>> {
>>> struct ffs_io_data *io_data = kiocb->private;
>>> - struct ffs_epfile *epfile = kiocb->ki_filp->private_data;
>>> + struct ffs_data *ffs = io_data->ffs;
>>> int value;
>>>
>>> ENTER();
>>>
>>> - spin_lock_irq(&epfile->ffs->eps_lock);
>>> -
>>> - if (likely(io_data && io_data->ep && io_data->req))
>>> - value = usb_ep_dequeue(io_data->ep, io_data->req);
>>> - else
>>> + if (likely(io_data && io_data->ep && io_data->req)) {
>>> + INIT_WORK(&io_data->cancellation_work, ffs_aio_cancel_worker);
>>> + queue_work(ffs->io_completion_wq, &io_data->cancellation_work);
>>> + value = -EINPROGRESS;
>>> + } else {
>>> value = -EINVAL;
>>> -
>>> - spin_unlock_irq(&epfile->ffs->eps_lock);
>>> + }
>
> Can we avoid the spin_lock() and the work-queue and call usb_ep_dequeue() directly from here?
>> What is the purpose of the spin_lock()?
I agree that the lock doesn't seem to be necessary. But I believe the whole
thing is already running in non-sleeping context, even before the spinlock
is taken. So this wouldn't help much.
Even the io_cancel() syscall takes a spinlock before invoking the cancel
function. So this issue is not exclusive to program termination.
Are there any documented guidelines on which context usb_ep_dequeue() should
be able to be called in? The sleep in the dwc3 driver seems to be a recent
addition.
>
>>>
>>> return value;
>>> }
>>> --
>>> 2.17.1
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
