public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Romit Dasgupta <romlinux@gmail.com>
To: greg@kroah.com, stern@rowland.harvard.edu,
	linux-usb@vger.kernel.org, dbrownell@users.sourceforge.net
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH] Moving spinlock to struct usb_hcd
Date: Sat, 26 Jan 2008 12:00:41 +0530	[thread overview]
Message-ID: <1201329041.5807.20.camel@maxwell> (raw)

Hi,
   This is an attempt to move the hcd_urb_list_lock to struct usb_hcd.
The lock is taken on functions that try to add/delete/use urb against a
given hcd. I have not seen any association of an urb with multiple hcds.
Hence I thought this can be moved within usb_hcd. This should help
reduce contention to usb during high load where i/o is happening  to
multiple hcds. I am also trying to see if hcd_root_hub_lock can also be
moved to usb_hcd. Any comments on this?  I have done some testing with
this patch and it seems to be holding fine. If this looks ok I will
submit the lock statistics before and after the change.

Thanks,
-Romit


---
 drivers/usb/core/hcd.c |   24 +++++++++++-------------
 drivers/usb/core/hcd.h |    1 +
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index d5ed3fa..6eb0f45 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -98,9 +98,6 @@ EXPORT_SYMBOL_GPL (usb_bus_list_lock);
 /* used for controlling access to virtual root hubs */
 static DEFINE_SPINLOCK(hcd_root_hub_lock);
 
-/* used when updating an endpoint's URB list */
-static DEFINE_SPINLOCK(hcd_urb_list_lock);
-
 /* wait queue for synchronous unlinks */
 DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue);
 
@@ -1000,7 +997,7 @@ int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd,
struct urb *urb)
 {
 	int		rc = 0;
 
-	spin_lock(&hcd_urb_list_lock);
+	spin_lock(&hcd->hcd_urb_list_lock);
 
 	/* Check that the URB isn't being killed */
 	if (unlikely(urb->reject)) {
@@ -1033,7 +1030,7 @@ int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd,
struct urb *urb)
 		goto done;
 	}
  done:
-	spin_unlock(&hcd_urb_list_lock);
+	spin_unlock(&hcd->hcd_urb_list_lock);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(usb_hcd_link_urb_to_ep);
@@ -1106,9 +1103,9 @@ EXPORT_SYMBOL_GPL(usb_hcd_check_unlink_urb);
 void usb_hcd_unlink_urb_from_ep(struct usb_hcd *hcd, struct urb *urb)
 {
 	/* clear all state linking urb to this dev (and hcd) */
-	spin_lock(&hcd_urb_list_lock);
+	spin_lock(&hcd->hcd_urb_list_lock);
 	list_del_init(&urb->urb_list);
-	spin_unlock(&hcd_urb_list_lock);
+	spin_unlock(&hcd->hcd_urb_list_lock);
 }
 EXPORT_SYMBOL_GPL(usb_hcd_unlink_urb_from_ep);
 
@@ -1311,7 +1308,7 @@ void usb_hcd_flush_endpoint(struct usb_device
*udev,
 	hcd = bus_to_hcd(udev->bus);
 
 	/* No more submits can occur */
-	spin_lock_irq(&hcd_urb_list_lock);
+	spin_lock_irq(&hcd->hcd_urb_list_lock);
 rescan:
 	list_for_each_entry (urb, &ep->urb_list, urb_list) {
 		int	is_in;
@@ -1320,7 +1317,7 @@ rescan:
 			continue;
 		usb_get_urb (urb);
 		is_in = usb_urb_dir_in(urb);
-		spin_unlock(&hcd_urb_list_lock);
+		spin_unlock(&hcd->hcd_urb_list_lock);
 
 		/* kick hcd */
 		unlink1(hcd, urb, -ESHUTDOWN);
@@ -1345,14 +1342,14 @@ rescan:
 		usb_put_urb (urb);
 
 		/* list contents may have changed */
-		spin_lock(&hcd_urb_list_lock);
+		spin_lock(&hcd->hcd_urb_list_lock);
 		goto rescan;
 	}
-	spin_unlock_irq(&hcd_urb_list_lock);
+	spin_unlock_irq(&hcd->hcd_urb_list_lock);
 
 	/* Wait until the endpoint queue is completely empty */
 	while (!list_empty (&ep->urb_list)) {
-		spin_lock_irq(&hcd_urb_list_lock);
+		spin_lock_irq(&hcd->hcd_urb_list_lock);
 
 		/* The list may have changed while we acquired the spinlock */
 		urb = NULL;
@@ -1361,7 +1358,7 @@ rescan:
 					urb_list);
 			usb_get_urb (urb);
 		}
-		spin_unlock_irq(&hcd_urb_list_lock);
+		spin_unlock_irq(&hcd->hcd_urb_list_lock);
 
 		if (urb) {
 			usb_kill_urb (urb);
@@ -1618,6 +1615,7 @@ struct usb_hcd *usb_create_hcd (const struct
hc_driver *driver,
 		dev_dbg (dev, "hcd alloc failed\n");
 		return NULL;
 	}
+	spin_lock_init(&hcd->hcd_urb_list_lock);
 	dev_set_drvdata(dev, hcd);
 	kref_init(&hcd->kref);
 
diff --git a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h
index 98e2419..e23ff45 100644
--- a/drivers/usb/core/hcd.h
+++ b/drivers/usb/core/hcd.h
@@ -128,6 +128,7 @@ struct usb_hcd {
 	 * input size of periodic table to an interrupt scheduler. 
 	 * (ohci 32, uhci 1024, ehci 256/512/1024).
 	 */
+	spinlock_t hcd_urb_list_lock;
 
 	/* The HC driver's private data is stored at the end of
 	 * this structure.
-- 
1.4.4.2



             reply	other threads:[~2008-01-26  6:31 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-26  6:30 Romit Dasgupta [this message]
2008-01-26  8:21 ` [PATCH] Moving spinlock to struct usb_hcd David Brownell
2008-01-26 15:36   ` Romit Dasgupta
2008-01-28  4:20     ` Romit Dasgupta
2008-01-28  4:24       ` Greg KH

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=1201329041.5807.20.camel@maxwell \
    --to=romlinux@gmail.com \
    --cc=dbrownell@users.sourceforge.net \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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