Re: [PATCH] usb: gadget: f_midi: added spinlock on transmit function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Balbi,

On 09/03/16 10:33, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Ferreri Tonello <eu@xxxxxxxxxxxxxxxxx> writes:
>> [ text/plain ]
>> Hi Balbi,
>>
>> On 09/03/16 07:20, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> "Felipe F. Tonello" <eu@xxxxxxxxxxxxxxxxx> writes:
>>>> [ text/plain ]
>>>> Since f_midi_transmit is called by both ALSA and USB sub-systems, it can
>>>> potentially cause a race condition between both calls because f_midi_transmit
>>>> is not reentrant nor thread-safe. This is due to an implementation detail that
>>>> the transmit function looks for the next available usb request from the fifo
>>>> and only enqueues it if there is data to send, otherwise just re-uses it. So,
>>>> if both ALSA and USB frameworks calls this function at the same time,
>>>> kfifo_seek() will return the same usb_request, which will cause a race
>>>> condition.
>>>>
>>>> To solve this problem a syncronization mechanism is necessary. In this case it
>>>> is used a spinlock since f_midi_transmit is also called by usb_request->complete
>>>> callback in interrupt context.
>>>>
>>>> Cc: <stable@xxxxxxxxxxxxxxx> # v4.4+
>>>
>>> this should be v4.5+
>>>
>>> $ git describe e1e3d7ec5da3
>>> v4.4-rc5-59-ge1e3d7ec5da3
>>>
>>>> @@ -651,17 +653,22 @@ static void f_midi_transmit(struct f_midi *midi)
>>>>  {
>>>>  	struct usb_ep *ep = midi->in_ep;
>>>>  	int ret;
>>>> +	unsigned long flags;
>>>>  
>>>>  	/* We only care about USB requests if IN endpoint is enabled */
>>>>  	if (!ep || !ep->enabled)
>>>>  		goto drop_out;
>>>>  
>>>> +	spin_lock_irqsave(&midi->transmit_lock, flags);
>>>> +
>>>>  	do {
>>>>  		ret = f_midi_do_transmit(midi, ep);
>>>
>>> you wrote this commit on top of 'next' right ? I know that because of
>>> this call to f_midi_do_transmit() here which is introduced by commit
>>> aac887442d5e ("usb: gadget: f_midi: move some of f_midi_transmit to
>>> separate func") which is not in Linus' tree yet.
>>>
>>> This prevents me from taking this patch during current -rc.
>>
>> Yes, but then what should I do? Because if I patch on Linus' tree, then
>> the patch won't apply to your tree.
> 
> it should apply to my "fixes" branch where fixes for current -rc should
> go :-) Note that v4.5-rc7 doesn't have f_midi_do_transmit()

Right, but then you will have to fix the merge conflicts by hand. :(

-- 
Felipe

Attachment: 0x92698E6A.asc
Description: application/pgp-keys


[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]