Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752236AbdFAQct (ORCPT ); Thu, 1 Jun 2017 12:32:49 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:33281 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751949AbdFAPoo (ORCPT ); Thu, 1 Jun 2017 11:44:44 -0400 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "Felipe Balbi" , "Krzysztof Opasiak" , "David Lechner" , "Krzysztof Opasiak" Date: Thu, 01 Jun 2017 16:43:16 +0100 Message-ID: X-Mailer: LinuxStableQueue (scripts by bwh) Subject: [PATCH 3.16 069/212] usb: gadget: f_hid: Use spinlock instead of mutex In-Reply-To: X-SA-Exim-Connect-IP: 82.70.136.246 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6514 Lines: 213 3.16.44-rc1 review patch. If anyone has any objections, please let me know. ------------------ From: Krzysztof Opasiak commit 33e4c1a9987a1fc3b42c3b534100b5b006d55c61 upstream. As IN request has to be allocated in set_alt() and released in disable() we cannot use mutex to protect it as we cannot sleep in those funcitons. Let's replace this mutex with a spinlock. Tested-by: David Lechner Signed-off-by: Krzysztof Opasiak Signed-off-by: Felipe Balbi [bwh: Backported to 3.16: adjust filename] Signed-off-by: Ben Hutchings --- drivers/usb/gadget/f_hid.c | 57 ++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 23 deletions(-) --- a/drivers/usb/gadget/f_hid.c +++ b/drivers/usb/gadget/f_hid.c @@ -44,12 +44,12 @@ struct f_hidg { /* recv report */ struct list_head completed_out_req; - spinlock_t spinlock; + spinlock_t read_spinlock; wait_queue_head_t read_queue; unsigned int qlen; /* send report */ - struct mutex lock; + spinlock_t write_spinlock; bool write_pending; wait_queue_head_t write_queue; struct usb_request *req; @@ -178,20 +178,20 @@ static ssize_t f_hidg_read(struct file * if (!access_ok(VERIFY_WRITE, buffer, count)) return -EFAULT; - spin_lock_irqsave(&hidg->spinlock, flags); + spin_lock_irqsave(&hidg->read_spinlock, flags); #define READ_COND (!list_empty(&hidg->completed_out_req)) /* wait for at least one buffer to complete */ while (!READ_COND) { - spin_unlock_irqrestore(&hidg->spinlock, flags); + spin_unlock_irqrestore(&hidg->read_spinlock, flags); if (file->f_flags & O_NONBLOCK) return -EAGAIN; if (wait_event_interruptible(hidg->read_queue, READ_COND)) return -ERESTARTSYS; - spin_lock_irqsave(&hidg->spinlock, flags); + spin_lock_irqsave(&hidg->read_spinlock, flags); } /* pick the first one */ @@ -206,7 +206,7 @@ static ssize_t f_hidg_read(struct file * req = list->req; count = min_t(unsigned int, count, req->actual - list->pos); - spin_unlock_irqrestore(&hidg->spinlock, flags); + spin_unlock_irqrestore(&hidg->read_spinlock, flags); /* copy to user outside spinlock */ count -= copy_to_user(buffer, req->buf + list->pos, count); @@ -228,9 +228,9 @@ static ssize_t f_hidg_read(struct file * return ret; } } else { - spin_lock_irqsave(&hidg->spinlock, flags); + spin_lock_irqsave(&hidg->read_spinlock, flags); list_add(&list->list, &hidg->completed_out_req); - spin_unlock_irqrestore(&hidg->spinlock, flags); + spin_unlock_irqrestore(&hidg->read_spinlock, flags); wake_up(&hidg->read_queue); } @@ -241,13 +241,16 @@ static ssize_t f_hidg_read(struct file * static void f_hidg_req_complete(struct usb_ep *ep, struct usb_request *req) { struct f_hidg *hidg = (struct f_hidg *)ep->driver_data; + unsigned long flags; if (req->status != 0) { ERROR(hidg->func.config->cdev, "End Point Request ERROR: %d\n", req->status); } + spin_lock_irqsave(&hidg->write_spinlock, flags); hidg->write_pending = 0; + spin_unlock_irqrestore(&hidg->write_spinlock, flags); wake_up(&hidg->write_queue); } @@ -255,18 +258,19 @@ static ssize_t f_hidg_write(struct file size_t count, loff_t *offp) { struct f_hidg *hidg = file->private_data; + unsigned long flags; ssize_t status = -ENOMEM; if (!access_ok(VERIFY_READ, buffer, count)) return -EFAULT; - mutex_lock(&hidg->lock); + spin_lock_irqsave(&hidg->write_spinlock, flags); #define WRITE_COND (!hidg->write_pending) /* write queue */ while (!WRITE_COND) { - mutex_unlock(&hidg->lock); + spin_unlock_irqrestore(&hidg->write_spinlock, flags); if (file->f_flags & O_NONBLOCK) return -EAGAIN; @@ -274,17 +278,20 @@ static ssize_t f_hidg_write(struct file hidg->write_queue, WRITE_COND)) return -ERESTARTSYS; - mutex_lock(&hidg->lock); + spin_lock_irqsave(&hidg->write_spinlock, flags); } + hidg->write_pending = 1; count = min_t(unsigned, count, hidg->report_length); + + spin_unlock_irqrestore(&hidg->write_spinlock, flags); status = copy_from_user(hidg->req->buf, buffer, count); if (status != 0) { ERROR(hidg->func.config->cdev, "copy_from_user error\n"); - mutex_unlock(&hidg->lock); - return -EINVAL; + status = -EINVAL; + goto release_write_pending; } hidg->req->status = 0; @@ -292,19 +299,23 @@ static ssize_t f_hidg_write(struct file hidg->req->length = count; hidg->req->complete = f_hidg_req_complete; hidg->req->context = hidg; - hidg->write_pending = 1; status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC); if (status < 0) { ERROR(hidg->func.config->cdev, "usb_ep_queue error on int endpoint %zd\n", status); - hidg->write_pending = 0; - wake_up(&hidg->write_queue); + goto release_write_pending; } else { status = count; } - mutex_unlock(&hidg->lock); + return status; +release_write_pending: + spin_lock_irqsave(&hidg->write_spinlock, flags); + hidg->write_pending = 0; + spin_unlock_irqrestore(&hidg->write_spinlock, flags); + + wake_up(&hidg->write_queue); return status; } @@ -371,9 +382,9 @@ static void hidg_set_report_complete(str req_list->req = req; - spin_lock_irqsave(&hidg->spinlock, flags); + spin_lock_irqsave(&hidg->read_spinlock, flags); list_add_tail(&req_list->list, &hidg->completed_out_req); - spin_unlock_irqrestore(&hidg->spinlock, flags); + spin_unlock_irqrestore(&hidg->read_spinlock, flags); wake_up(&hidg->read_queue); break; @@ -491,13 +502,13 @@ static void hidg_disable(struct usb_func usb_ep_disable(hidg->out_ep); hidg->out_ep->driver_data = NULL; - spin_lock_irqsave(&hidg->spinlock, flags); + spin_lock_irqsave(&hidg->read_spinlock, flags); list_for_each_entry_safe(list, next, &hidg->completed_out_req, list) { free_ep_req(hidg->out_ep, list->req); list_del(&list->list); kfree(list); } - spin_unlock_irqrestore(&hidg->spinlock, flags); + spin_unlock_irqrestore(&hidg->read_spinlock, flags); } static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt) @@ -642,8 +653,8 @@ static int __init hidg_bind(struct usb_c if (status) goto fail; - mutex_init(&hidg->lock); - spin_lock_init(&hidg->spinlock); + spin_lock_init(&hidg->write_spinlock); + spin_lock_init(&hidg->read_spinlock); init_waitqueue_head(&hidg->write_queue); init_waitqueue_head(&hidg->read_queue); INIT_LIST_HEAD(&hidg->completed_out_req);