All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.