mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Felipe F. Tonello" <eu@felipetonello.com>
To: linux-usb@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Felipe Balbi <balbi@kernel.org>,
	Michal Nazarewicz <mina86@mina86.com>,
	Clemens Ladisch <clemens@ladisch.de>
Subject: [PATCH 2/5] usb: gadget: f_midi: added spinlock on transmit function
Date: Wed,  2 Mar 2016 19:40:37 +0000	[thread overview]
Message-ID: <1456947640-20673-3-git-send-email-eu@felipetonello.com> (raw)
In-Reply-To: <1456947640-20673-1-git-send-email-eu@felipetonello.com>

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.

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 drivers/usb/gadget/function/f_midi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 3cdb0741f3f8..8475e3dc82d4 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/device.h>
 #include <linux/kfifo.h>
+#include <linux/spinlock.h>
 
 #include <sound/core.h>
 #include <sound/initval.h>
@@ -95,6 +96,7 @@ struct f_midi {
 	unsigned int buflen, qlen;
 	/* This fifo is used as a buffer ring for pre-allocated IN usb_requests */
 	DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
+	spinlock_t transmit_lock;
 	unsigned int in_last_port;
 
 	struct gmidi_in_port	in_ports_array[/* in_ports */];
@@ -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);
 		if (ret < 0)
 			goto drop_out;
 	} while (ret);
 
+	spin_unlock_irqrestore(&midi->transmit_lock, flags);
+
 	return;
 
 drop_out:
@@ -1255,6 +1262,8 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
 	if (status)
 		goto setup_fail;
 
+	spin_lock_init(&midi->transmit_lock);
+
 	++opts->refcnt;
 	mutex_unlock(&opts->lock);
 
-- 
2.7.2

  parent reply	other threads:[~2016-03-02 19:38 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-02 19:40 [PATCH 0/5] MIDI USB Gadget improvements Felipe F. Tonello
2016-03-02 19:40 ` [PATCH 1/5] usb: gadget: f_midi: refactor state machine Felipe F. Tonello
2016-03-02 21:09   ` Clemens Ladisch
2016-03-03  8:57     ` Felipe Ferreri Tonello
2016-03-03 11:38       ` Clemens Ladisch
2016-03-03 16:30         ` Felipe Ferreri Tonello
2016-03-04  8:07           ` Clemens Ladisch
2016-03-04 18:39             ` Felipe Ferreri Tonello
2016-03-04 18:43               ` Clemens Ladisch
2016-03-02 19:40 ` Felipe F. Tonello [this message]
2016-03-04  7:20   ` [PATCH 2/5] usb: gadget: f_midi: added spinlock on transmit function Felipe Balbi
2016-03-04 18:49     ` Felipe Ferreri Tonello
2016-03-07  7:32       ` Felipe Balbi
2016-03-07  9:28         ` Felipe Ferreri Tonello
2016-03-08  7:37           ` Felipe Balbi
2016-03-08 13:46             ` Felipe Ferreri Tonello
2016-03-08 14:01               ` Felipe Balbi
2016-03-08 15:40                 ` Felipe Ferreri Tonello
2016-03-09  7:22                   ` Felipe Balbi
2016-03-02 19:40 ` [PATCH 3/5] usb: gadget: gmidi: remove bus powered requirement on bmAttributes Felipe F. Tonello
2016-03-04  7:16   ` Felipe Balbi
2016-03-04 18:46     ` Felipe Ferreri Tonello
2016-03-07  7:34       ` Felipe Balbi
2016-03-07  9:40         ` Felipe Ferreri Tonello
2016-03-07 10:59           ` Felipe Balbi
2016-03-07 11:13             ` Felipe Ferreri Tonello
2016-03-08  7:43               ` Felipe Balbi
2016-03-08 10:14                 ` Krzysztof Opasiak
2016-03-08 10:34                   ` Felipe Balbi
2016-03-08 13:54                 ` Felipe Ferreri Tonello
2016-03-08 14:04                   ` Felipe Balbi
2016-03-08 14:15                   ` Krzysztof Opasiak
2016-03-08 14:20                     ` Felipe Balbi
2016-03-08 15:24                       ` Felipe Ferreri Tonello
2016-03-02 19:40 ` [PATCH 4/5] usb: gadget: f_midi: cleanups and typos fixes Felipe F. Tonello
2016-03-04  7:13   ` Felipe Balbi
2016-03-04 19:17   ` Michal Nazarewicz
2016-03-04 20:17     ` Felipe Ferreri Tonello
2016-03-05 16:28       ` Michal Nazarewicz
2016-03-05 19:39         ` Greg KH
2016-03-05 23:53           ` Felipe Ferreri Tonello
2016-03-06  3:02             ` Greg KH
2016-03-05 23:57         ` Felipe Ferreri Tonello
2016-03-07  7:35           ` Felipe Balbi
2016-03-07  9:32             ` Felipe Ferreri Tonello
2016-03-08  7:44               ` Felipe Balbi
2016-03-02 19:40 ` [PATCH 5/5] usb: gadget: f_midi: updated copyright Felipe F. Tonello
2016-03-04  7:13   ` Felipe Balbi
2016-03-04 18:41     ` Felipe Ferreri Tonello
2016-03-07  7:36       ` Felipe Balbi
2016-03-07  9:23         ` Felipe Ferreri Tonello
2016-03-04  7:11 ` [PATCH 0/5] MIDI USB Gadget improvements Felipe Balbi
2016-03-04 18:43   ` Felipe Ferreri Tonello

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1456947640-20673-3-git-send-email-eu@felipetonello.com \
    --to=eu@felipetonello.com \
    --cc=balbi@kernel.org \
    --cc=clemens@ladisch.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mina86@mina86.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®