Hi Balbi, On 07/03/16 07:32, Felipe Balbi wrote: > > Hi, > > (please break your lines at 80-characters, have a look at > Documentation/email-clients.txt if needed) > > Felipe Ferreri Tonello <eu@xxxxxxxxxxxxxxxxx> writes: >> [ text/plain ] >> 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. > > did you measure ? Is the extra speed really necessary ? How did you > benchmark this ? Yes I did measure and it was not that significant. This is not about speed. There was a bug in that approach that I already explained on that patch, which was approved and applied BTW. Any way, this spinlock should've been there since that patch but I couldn't really trigger this problem without a stress test. So, this patch fixes a bug in the current implementation. Felipe
Attachment:
0x92698E6A.asc
Description: application/pgp-keys
