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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <D065B08D-83C3-45F5-840C-C7752A5CCEDD@felipetonello.com>
Date:	Fri, 04 Mar 2016 18:49:36 +0000
From:	Felipe Ferreri Tonello <eu@...ipetonello.com>
To:	Felipe Balbi <balbi@...nel.org>, linux-usb@...r.kernel.org
CC:	linux-kernel@...r.kernel.org,
	Michal Nazarewicz <mina86@...a86.com>,
	Clemens Ladisch <clemens@...isch.de>
Subject: Re: [PATCH 2/5] usb: gadget: f_midi: added spinlock on transmit function

Hi Balbi, 

On March 4, 2016 7:20:10 AM GMT+00:00, Felipe Balbi <balbi@...nel.org> wrote:
>
>Hi,
>
>"Felipe F. Tonello" <eu@...ipetonello.com> 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.