On Wed, 22 Feb 2006 15:28:02 -0500 (EST), Alan Stern <[EMAIL PROTECTED]> wrote:
> On Wed, 22 Feb 2006, Pete Zaitcev wrote:
> > Frankly, I cannot fathom Alan's resistance to it when he actually suggested
> > the main idea for the patch.
>
> I'm not at all resistant to the idea. It's just that the patch you wrote
> doesn't apply to the current USB development tree because of other,
> large-scale changes to the driver. As far as I can see, it should work
> fine on earlier kernels, i.e., anything < 2.6.17-rc1.
I checked out your patches and it appears that they keep FSBR on as long as
there's an URB submitted. If that's the case, I am afraid there may be a
problem with that. I envision a scenario of a guy who has PCI bus hogged
by his UHCI executing FSBR whenever he has a USB serial device and something
like pilot-link, kermit, or getty opening the device. I remember someone
complaining that his I/O speed getting 5 times slower whenever he hooked
up his Pilot.
However, I tested the new driver at my laptops, and there was no such
effect on them (with Intel north- and southbridges). So, I think we best
proceed with what we have and fall back on my patch if a real-life problem
appears. I am attaching a version which works with the new UHCI, for
the archives.
> > So now it's up to you. If you get a performance
> > anomaly on stock upstream kernels and a loaded LAN, I can resubmit and
> > otherwise argue the point. But if not, bleah. I can bump the timeout
> > in RHEL-4 just as easily.
>
> That should be a quick, easy fix for the problems people have been seeing.
> Make the timeout something like 500 or 1000 ms.
I'll work with Chris on that.
-- Pete
diff -urp -X dontdiff linux-2.6.16-rc4-stern/drivers/usb/host/uhci-hcd.c
linux-2.6.16-rc4-wlan/drivers/usb/host/uhci-hcd.c
--- linux-2.6.16-rc4-stern/drivers/usb/host/uhci-hcd.c 2006-03-01
12:35:49.000000000 -0800
+++ linux-2.6.16-rc4-wlan/drivers/usb/host/uhci-hcd.c 2006-02-28
23:17:55.000000000 -0800
@@ -88,8 +88,7 @@ static void wakeup_rh(struct uhci_hcd *u
static void uhci_get_current_frame_number(struct uhci_hcd *uhci);
/* If a transfer is still active after this much time, turn off FSBR */
-#define IDLE_TIMEOUT msecs_to_jiffies(50)
-#define FSBR_DELAY msecs_to_jiffies(50)
+#define FSBR_TIMEOUT msecs_to_jiffies(50)
/* When we timeout an idle transfer for FSBR, we'll switch it over to */
/* depth first traversal. We'll do it in groups of this number of TDs */
@@ -490,8 +489,10 @@ static int uhci_start(struct usb_hcd *hc
hcd->uses_new_polling = 1;
- uhci->fsbr = 0;
- uhci->fsbrtimeout = 0;
+ uhci->fsbr_runs = 0;
+ init_timer(&uhci->fsbr_timer);
+ uhci->fsbr_timer.data = (unsigned long) uhci;
+ uhci->fsbr_timer.function = uhci_fsbr_timeout;
spin_lock_init(&uhci->lock);
@@ -673,6 +674,8 @@ static void uhci_stop(struct usb_hcd *hc
uhci_scan_schedule(uhci, NULL);
spin_unlock_irq(&uhci->lock);
+ del_timer_sync(&uhci->fsbr_timer);
+
release_uhci(uhci);
}
diff -urp -X dontdiff linux-2.6.16-rc4-stern/drivers/usb/host/uhci-hcd.h
linux-2.6.16-rc4-wlan/drivers/usb/host/uhci-hcd.h
--- linux-2.6.16-rc4-stern/drivers/usb/host/uhci-hcd.h 2006-03-01
12:35:49.000000000 -0800
+++ linux-2.6.16-rc4-wlan/drivers/usb/host/uhci-hcd.h 2006-02-28
23:16:17.000000000 -0800
@@ -396,8 +396,8 @@ struct uhci_hcd {
__le32 *frame;
void **frame_cpu; /* CPU's frame list */
- int fsbr; /* Full-speed bandwidth reclamation */
- unsigned long fsbrtimeout; /* FSBR delay */
+ int fsbr_runs; /* Full-speed bandwidth reclamation */
+ struct timer_list fsbr_timer;
enum uhci_rh_state rh_state;
unsigned long auto_stop_time; /* When to AUTO_STOP */
@@ -453,9 +453,10 @@ struct urb_priv {
struct urb *urb;
struct uhci_qh *qh; /* QH for this URB */
+ struct uhci_td *fsbr_td;
struct list_head td_list;
- unsigned fsbr : 1; /* URB turned on FSBR */
+ unsigned fsbr : 1; /* URB activates FSBR */
unsigned short_transfer : 1; /* URB got a short transfer, no
* need to rescan */
};
diff -urp -X dontdiff linux-2.6.16-rc4-stern/drivers/usb/host/uhci-hub.c
linux-2.6.16-rc4-wlan/drivers/usb/host/uhci-hub.c
--- linux-2.6.16-rc4-stern/drivers/usb/host/uhci-hub.c 2005-11-21
19:46:28.000000000 -0800
+++ linux-2.6.16-rc4-wlan/drivers/usb/host/uhci-hub.c 2006-02-28
23:14:05.000000000 -0800
@@ -152,7 +152,6 @@ static int uhci_hub_status_data(struct u
uhci_scan_schedule(uhci, NULL);
if (uhci->hc_inaccessible)
goto done;
- check_fsbr(uhci);
uhci_check_ports(uhci);
status = get_hub_status_data(uhci, buf);
diff -urp -X dontdiff linux-2.6.16-rc4-stern/drivers/usb/host/uhci-q.c
linux-2.6.16-rc4-wlan/drivers/usb/host/uhci-q.c
--- linux-2.6.16-rc4-stern/drivers/usb/host/uhci-q.c 2006-03-01
12:35:49.000000000 -0800
+++ linux-2.6.16-rc4-wlan/drivers/usb/host/uhci-q.c 2006-02-28
23:26:22.000000000 -0800
@@ -17,6 +17,9 @@
*/
static void uhci_free_pending_tds(struct uhci_hcd *uhci);
+static void uhci_fsbr_check_td(struct uhci_hcd *uhci, struct urb *urb);
+static void uhci_fsbr_start_urb(struct uhci_hcd *uhci, struct urb *urb);
+static void uhci_fsbr_activate(struct uhci_hcd *uhci);
/*
* Technically, updating td->status here is a race, but it's not really a
@@ -439,28 +442,6 @@ static void uhci_free_urb_priv(struct uh
kmem_cache_free(uhci_up_cachep, urbp);
}
-static void uhci_inc_fsbr(struct uhci_hcd *uhci, struct urb *urb)
-{
- struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
-
- if ((!(urb->transfer_flags & URB_NO_FSBR)) && !urbp->fsbr) {
- urbp->fsbr = 1;
- if (!uhci->fsbr++ && !uhci->fsbrtimeout)
- uhci->skel_term_qh->link =
cpu_to_le32(uhci->skel_fs_control_qh->dma_handle) | UHCI_PTR_QH;
- }
-}
-
-static void uhci_dec_fsbr(struct uhci_hcd *uhci, struct urb *urb)
-{
- struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
-
- if ((!(urb->transfer_flags & URB_NO_FSBR)) && urbp->fsbr) {
- urbp->fsbr = 0;
- if (!--uhci->fsbr)
- uhci->fsbrtimeout = jiffies + FSBR_DELAY;
- }
-}
-
/*
* Map status to standard result codes
*
@@ -497,6 +478,7 @@ static int uhci_map_status(int status, i
static int uhci_submit_control(struct uhci_hcd *uhci, struct urb *urb,
struct uhci_qh *qh)
{
+ struct urb_priv *urbp = urb->hcpriv;
struct uhci_td *td;
unsigned long destination, status;
int maxsze = le16_to_cpu(qh->hep->desc.wMaxPacketSize);
@@ -551,6 +533,8 @@ static int uhci_submit_control(struct uh
uhci_add_td_to_urb(urb, td);
uhci_fill_td(td, status, destination | uhci_explen(pktsze),
data);
+ if (urbp->fsbr_td == NULL)
+ urbp->fsbr_td = td;
plink = &td->link;
data += pktsze;
@@ -606,7 +590,7 @@ static int uhci_submit_control(struct uh
qh->skel = uhci->skel_ls_control_qh;
else {
qh->skel = uhci->skel_fs_control_qh;
- uhci_inc_fsbr(uhci, urb);
+ uhci_fsbr_start_urb(uhci, urb);
}
return 0;
@@ -750,6 +734,7 @@ err:
static int uhci_submit_common(struct uhci_hcd *uhci, struct urb *urb,
struct uhci_qh *qh)
{
+ struct urb_priv *urbp = urb->hcpriv;
struct uhci_td *td;
unsigned long destination, status;
int maxsze = le16_to_cpu(qh->hep->desc.wMaxPacketSize);
@@ -798,6 +783,8 @@ static int uhci_submit_common(struct uhc
destination | uhci_explen(pktsze) |
(toggle << TD_TOKEN_TOGGLE_SHIFT),
data);
+ if (urbp->fsbr_td == NULL)
+ urbp->fsbr_td = td;
plink = &td->link;
status |= TD_CTRL_ACTIVE;
@@ -954,7 +941,7 @@ static inline int uhci_submit_bulk(struc
qh->skel = uhci->skel_bulk_qh;
ret = uhci_submit_common(uhci, urb, qh);
if (ret == 0)
- uhci_inc_fsbr(uhci, urb);
+ uhci_fsbr_start_urb(uhci, urb);
return ret;
}
@@ -1221,7 +1208,6 @@ __acquires(uhci->lock)
qh->needs_fixup = 0;
}
- uhci_dec_fsbr(uhci, urb); /* Safe since it checks */
uhci_free_urb_priv(uhci, urbp);
switch (usb_pipetype(urb->pipe)) {
@@ -1275,11 +1261,14 @@ static void uhci_scan_qh(struct uhci_hcd
switch (usb_pipetype(urb->pipe)) {
case PIPE_CONTROL:
+ uhci_fsbr_check_td(uhci, urb);
status = uhci_result_control(uhci, urb);
break;
case PIPE_ISOCHRONOUS:
status = uhci_result_isochronous(uhci, urb);
break;
+ case PIPE_BULK:
+ uhci_fsbr_check_td(uhci, urb);
default: /* PIPE_BULK or PIPE_INTERRUPT */
status = uhci_result_common(uhci, urb);
break;
@@ -1348,6 +1337,62 @@ static void uhci_free_pending_tds(struct
}
/*
+ * FSBR management
+ */
+
+static void uhci_fsbr_check_td(struct uhci_hcd *uhci, struct urb *urb)
+{
+ struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
+ unsigned int status;
+
+ if (!urbp->fsbr)
+ return;
+ status = uhci_status_bits(td_status(urbp->fsbr_td));
+ if (status & TD_CTRL_ACTIVE)
+ return;
+
+ urbp->fsbr = 0; /* The chance to extend timeout is used up. */
+ uhci_fsbr_activate(uhci);
+}
+
+static void uhci_fsbr_start_urb(struct uhci_hcd *uhci, struct urb *urb)
+{
+ struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
+ struct uhci_td *td;
+
+ if ((td = urbp->fsbr_td) == NULL)
+ return;
+ td->status |= cpu_to_le32(TD_CTRL_IOC);
+ urbp->fsbr = 1;
+ uhci_fsbr_activate(uhci);
+}
+
+static void uhci_fsbr_activate(struct uhci_hcd *uhci)
+{
+ __le32 dma_addr;
+
+ if (!uhci->fsbr_runs) {
+ uhci->fsbr_runs = 1;
+ dma_addr = cpu_to_le32(uhci->skel_fs_control_qh->dma_handle);
+ uhci->skel_term_qh->link = dma_addr | UHCI_PTR_QH;
+ }
+ mod_timer(&uhci->fsbr_timer, jiffies + FSBR_TIMEOUT);
+}
+
+static void uhci_fsbr_timeout(unsigned long arg)
+{
+ struct uhci_hcd *uhci = (struct uhci_hcd *) arg;
+ unsigned long flags;
+
+ spin_lock_irqsave(&uhci->lock, flags);
+ if (uhci->fsbr_runs) {
+ uhci->fsbr_runs = 0;
+ uhci->skel_term_qh->link = UHCI_PTR_TERM;
+ }
+ spin_unlock_irqrestore(&uhci->lock, flags);
+}
+
+/*
* Process events in the schedule, but only in one thread at a time
*/
static void uhci_scan_schedule(struct uhci_hcd *uhci, struct pt_regs *regs)
@@ -1395,15 +1440,3 @@ static void uhci_scan_schedule(struct uh
else
uhci_set_next_interrupt(uhci);
}
-
-static void check_fsbr(struct uhci_hcd *uhci)
-{
- /* For now, don't scan URBs for FSBR timeouts.
- * Add it back in later... */
-
- /* Really disable FSBR */
- if (!uhci->fsbr && uhci->fsbrtimeout && time_after_eq(jiffies,
uhci->fsbrtimeout)) {
- uhci->fsbrtimeout = 0;
- uhci->skel_term_qh->link = UHCI_PTR_TERM;
- }
-}
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
