On 21/06/18 13:52, Lars-Peter Clausen wrote:
> 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.
drivers/usb/udc/gadget/core.c has the only documentation, but context is not
mentioned there.
Felipe, what do you suggest?
>
>>
>>>>
>>>> return value;
>>>> }
>>>> --
>>>> 2.17.1
>>>>
>>
>
--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
