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

[Felipe Ferreri Tonello]

Hi Balbi, 

On March 4, 2016 7:20:10 AM GMT+00:00, Felipe Balbi <balbi@xxxxxxxxxx> wrote:
>
>Hi,
>
>"Felipe F. Tonello" <eu@xxxxxxxxxxxxxxxxx> writes:
>> [ text/plain ]
>> Since f_midi_transmit is called by both ALSA and USB frameworks, it
>can
>> potentially cause a race condition between both calls. This is bad
>because the
>> way f_midi_transmit is implemented can't handle concurrent calls.
>This is due
>> to the fact that the usb request fifo looks for the next element and
>only if
>> it has data to process it enqueues the request, otherwise re-uses it.
>If both
>> (ALSA and USB) frameworks calls this function at the same time, the
>> 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.
>>
>> On benchmarks realized by me, spinlocks were more efficient then
>scheduling
>> the f_midi_transmit tasklet in process context and using a mutex to
>> synchronize. Also it performs better then previous implementation
>that
>> allocated a usb_request for every new transmit made.
>
>behaves better in what way ? Also, previous implementation would not
>suffer from this concurrency problem, right ?

The spin lock is faster than allocating usb requests all the time, even if the udc uses da for it. 

That's true it wasn't necessary to put a spin lock in the gadget driver because the udc driver does it when allocating a new request. 

Felipe 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.