Am Freitag, 30. April 2004 16:12 schrieb Alan Stern:
> On Fri, 30 Apr 2004, Oliver Neukum wrote:
> > Very, very tempting. Elegant and simple. But I have a problem with it.
> > It's entirely legal for a driver to call usb_get_urb() twice. And then we
> > have a problem. So I don't think that we should use the refcount.
>
> Yes, it is legal for a driver to do that. I suspect that very few of them
> do, because there's not often a reason for it. In fact, a recursive grep
> of the kernel source shows that usb_get_urb() is mentioned only within
> include/linux/usb.h, core/hcd.c, core/urb.c, host/ehci-sched.c, and
> host/hc_simple.c. No drivers call it. Anyway, we can always document at
> the start of the routine that it shouldn't be used if the URB's reference
> count has been monkeyed with.
But that's the direction we are moving in, isn't it?
> On the other hand, there are some very good reasons for using the refcount
> in that test. Whatever we test, it needs to be something that is changed
> in giveback_urb after the completion handler returns. The only field for
> which that is currently true is the refcount. Remember, at the end of
> giveback_urb usbcore no longer owns the URB; the driver does. Hence there
> isn't much that giveback_urb can afford to change.
But giveback_urb doesn't need to change anything. Calling the handler
itself it knows when it has run.
> Also, whatever we use, it has to take into account the possibility of
> resubmission (and even the theoretical possibility of nested invocations
> of giveback_urb!). The only thing that will handle this properly is a
> refcount of some sort. Yes, we could introduce another one that would
> track just the active submissions -- but why go to all that trouble (and
> introduce yet another field into the already-bloated struct urb) when
> we have a ready-made reference count sitting right there?
I don't think we need a refcount. Could you have a look at this code:
You can import this changeset into BK by piping this whole message to:
'| bk receive [path to repository]' or apply the patch as usual.
===================================================================
[EMAIL PROTECTED], 2004-04-30 17:27:46+02:00, [EMAIL PROTECTED]
- add an API that allows to terminate all activity associated with
an URB
drivers/usb/core/hcd.c | 7 +++++-
drivers/usb/core/hub.c | 24 +++++++++++-----------
drivers/usb/core/urb.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/usb.h | 9 ++++++++
4 files changed, 79 insertions(+), 14 deletions(-)
diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
--- a/drivers/usb/core/hcd.c Fri Apr 30 17:49:43 2004
+++ b/drivers/usb/core/hcd.c Fri Apr 30 17:49:43 2004
@@ -1478,6 +1478,7 @@
void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb, struct pt_regs *regs)
{
urb_unlink (urb);
+ urb->phase = URB_PHASE_DONE; /* to detect resubmission in handler */
// NOTE: a generic device/urb monitoring hook would go here.
// hcd_monitor_hook(MONITOR_URB_FINISH, urb, dev)
@@ -1492,7 +1493,7 @@
DMA_TO_DEVICE);
if (urb->transfer_buffer_length != 0
&& !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP))
- dma_unmap_single (hcd->self.controller,
+ dma_unmap_single (hcd->self.controller,
urb->transfer_dma,
urb->transfer_buffer_length,
usb_pipein (urb->pipe)
@@ -1502,6 +1503,10 @@
/* pass ownership to the completion handler */
urb->complete (urb, regs);
+ if (urb->phase != URB_PHASE_DOING) { /* no resubmission */
+ urb->phase = URB_PHASE_IDLE;
+ }
+ wake_up(&urb->handler_queue); /* for usb_kill_urb() */
usb_put_urb (urb);
}
EXPORT_SYMBOL (usb_hcd_giveback_urb);
diff -Nru a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
--- a/drivers/usb/core/hub.c Fri Apr 30 17:49:43 2004
+++ b/drivers/usb/core/hub.c Fri Apr 30 17:49:43 2004
@@ -233,11 +233,11 @@
int status;
spin_lock(&hub_event_lock);
- hub->urb_active = 0;
- if (hub->urb_complete) { /* disconnect or rmmod */
- complete(hub->urb_complete);
- goto done;
- }
+ //hub->urb_active = 0;
+ //if (hub->urb_complete) { /* disconnect or rmmod */
+ // complete(hub->urb_complete);
+ // goto done;
+ //}
switch (urb->status) {
case -ENOENT: /* synchronous unlink */
@@ -252,7 +252,7 @@
goto resubmit;
hub->error = urb->status;
/* FALL THROUGH */
-
+
/* let khubd handle things */
case 0: /* we got data: port status changed */
break;
@@ -271,8 +271,8 @@
/* ENODEV means we raced disconnect() */
&& status != -ENODEV)
dev_err (&hub->intf->dev, "resubmit --> %d\n", urb->status);
- if (status == 0)
- hub->urb_active = 1;
+ //if (status == 0)
+ // hub->urb_active = 1;
done:
spin_unlock(&hub_event_lock);
}
@@ -588,7 +588,7 @@
message = "couldn't submit status urb";
goto fail;
}
- hub->urb_active = 1;
+ //hub->urb_active = 1;
/* Wake up khubd */
wake_up(&khubd_wait);
@@ -640,9 +640,9 @@
flush_scheduled_work ();
if (hub->urb) {
- usb_unlink_urb(hub->urb);
- if (hub->urb_active)
- wait_for_completion(&urb_complete);
+ usb_kill_urb(hub->urb);
+ //if (hub->urb_active)
+ // wait_for_completion(&urb_complete);
usb_free_urb(hub->urb);
hub->urb = NULL;
}
diff -Nru a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
--- a/drivers/usb/core/urb.c Fri Apr 30 17:49:43 2004
+++ b/drivers/usb/core/urb.c Fri Apr 30 17:49:43 2004
@@ -33,6 +33,7 @@
memset(urb, 0, sizeof(*urb));
urb->count = (atomic_t)ATOMIC_INIT(1);
spin_lock_init(&urb->lock);
+ init_waitqueue_head(&urb->handler_queue);
}
}
@@ -384,6 +385,7 @@
urb->interval = temp;
}
+ urb->phase = URB_PHASE_DOING;
return op->submit_urb (urb, mem_flags);
}
@@ -454,10 +456,59 @@
return -ENODEV;
}
+/**
+ * usb_kill_urb cancel all data transfers designated by an URB
+ * @urb: pointer to urb describing a previously submitted request
+ *
+ * it will cancel/abort all data transfers and make sure that all
+ * completion handlers have run
+ *
+ */
+
+int usb_kill_urb(struct urb *urb)
+{
+ int retval;
+
+ spin_lock_irq(&urb->lock);
+ if (urb->phase == URB_PHASE_IDLE) {
+ retval = -EINVAL;
+ spin_unlock_irq(&urb->lock);
+ goto done;
+ } else {
+ spin_unlock_irq(&urb->lock);
+ }
+
+rekill:
+ retval = usb_unlink_urb(urb);
+ if (retval < 0) { /* failed - find out whether URB has been given back */
+ spin_lock_irq(&urb->lock);
+ switch (urb->phase) {
+ case URB_PHASE_IDLE: /* done */
+ spin_unlock_irq(&urb->lock);
+ retval = 0;
+ break;
+ case URB_PHASE_DOING: /* handler has resubmitted */
+ spin_unlock_irq(&urb->lock);
+ goto rekill;
+ case URB_PHASE_DONE: /* will still be given back */
+ spin_unlock_irq(&urb->lock);
+ wait_event(urb->handler_queue, urb->phase !=
URB_PHASE_IDLE);
+ retval = 0;
+ break;
+ default:
+ BUG();
+ }
+ }
+
+done:
+ return retval;
+}
+
+
EXPORT_SYMBOL(usb_init_urb);
EXPORT_SYMBOL(usb_alloc_urb);
EXPORT_SYMBOL(usb_free_urb);
EXPORT_SYMBOL(usb_get_urb);
EXPORT_SYMBOL(usb_submit_urb);
EXPORT_SYMBOL(usb_unlink_urb);
-
+EXPORT_SYMBOL(usb_kill_urb);
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Fri Apr 30 17:49:43 2004
+++ b/include/linux/usb.h Fri Apr 30 17:49:43 2004
@@ -551,6 +551,13 @@
#define URB_ZERO_PACKET 0x0040 /* Finish bulk OUTs with short packet
*/
#define URB_NO_INTERRUPT 0x0080 /* HINT: no non-error interrupt needed */
+/*
+ * urb->phase
+ */
+#define URB_PHASE_IDLE 0
+#define URB_PHASE_DOING 1
+#define URB_PHASE_DONE 2
+
struct usb_iso_packet_descriptor {
unsigned int offset;
unsigned int length; /* expected length */
@@ -736,6 +743,8 @@
void *hcpriv; /* private data for host controller */
struct list_head urb_list; /* list pointer to all active urbs */
int bandwidth; /* bandwidth for INT/ISO request */
+ wait_queue_head_t handler_queue;/* waiting for completion handlers */
+ int phase; /* tell apart phases of the URB */
/* public, documented fields in the urb that can be used by drivers */
struct usb_device *dev; /* (in) pointer to associated device */
===================================================================
This BitKeeper patch contains the following changesets:
1.1626
## Wrapped with gzip_uu ##
begin 664 bkpatch6746
M'XL(`)=UDD```[58?V_;-A#]6_P4+`H,25I;)/7+<I8@;1.TQKHT2-MA0SL8
MM$3;A&7)%:ED1=WOOB-E.W:C.*G1)($5B^33W>.]NZ.>XH]*E%VGR.25*-%3
M_*90&KZ*O,A%>UQ,12;[EMAIL PROTECTED](H8-`UM]TBRR1S/Y1"*'<BLXR%"&9<
M<)V,,4"IKD/;WNJ._CH37>?R[/7'MR\N$3HZPJ_&/!^)]T+CHR.DB_**9ZDZ
MX7J<%7E;ESQ74Z%Y.RFF\]74.2.$P6]`(X\$X9R&Q(_F"4TIY3X5*6%^)_31
MN!J-3\`&J;DR`)^6\/]N0OG$9S&CU&.=.8MCGZ%33-LT9"[EMAIL PROTECTED]'L$TZK*H
MZX?/"[EMAIL PROTECTED]>3IKXP<]\W"+H)?ZUSKQ""6YAGJ:8Y_C%10_K,=>89UEQK>!1
M6(MR*G.NA;F'>:+EE=1?,5>J2"3<3O&UU&,`P0;@X^5+]`?V:(>$Z.)F#U#K
M)W\0([EMAIL PROTECTED]'([EMAIL PROTECTED]:#>5H:?I1;J8&;[EMAIL PROTECTED],*<>
MZ9`Y#[CP:!Q[29*2)$ZWD;L%&'[EMAIL PROTECTED]/81S;W212%8-;V+9!YDE6I<"VZ06R/
MUS<#`F%."?/8W$O!NBCQDH@&G0X9;K7P#M1U\T(2!=Z]YJV[.JY6]%GC?$*C
MN1_Y83`7PT',F$\&7/!D^)/LK7#7S6-!['7`O)F1[`-L2]([EMAIL PROTECTED]'XP#P(/
MB..=-(R$GW0Z/H2V^$G;EKCKMGDLILPFCN9(,%GD5P8C2OF53%N#DQE/!B++
MVKG0GT:E&/V[-10]"!S(*D`!I3$A-J$PMIE.:)>[EMAIL PROTECTED]<1\`O;W3>;N
[EMAIL PROTECTED]&KMO,.M\MK^@<@O[B![AW31\P),D>/(7.K^-9?Z2R4JT1\+GN[]!J"M
M8_`CS439MP/[A["B$YHE=G`VYDK@(Y._^A=O7KP_ZY^^ZYV_AEE^$&(_1N[!
M`<+X8,,IG/`\$9E-C"G7'%OFAN`/3H62H]RFQ\'796([EMAIL PROTECTED]/"IE#8C7Y
MU>#`[*24`YF/,,>S4ES)HE+95ZRJP51J`U(*,%MI`V%AI(:T"X^M+7#YH"AU
MDQW@,Y[RB0"H4JQ2NX6`S9UE0LLBQPMJ%/QS)7!9Y<OGN/"!P-0-M_>4+JM$
M6\L/X&,??4..F50*#8%TB#XC1\UDWL^*9-*7Y9?%!IBOP+LCAWAOG?1UUGNG
M;\_V,>`Y-1AL2>NL=_[7B[>PL$:M\CMPG5$!?*80Z?#E.Q89@'^[=]5W,+<4
MQK<NNGFH\1>6R'QB/39>+BQ?3/D=$[`3NP=XR&4&.]3"0PED%Q7LS%CH,>PN
MN`6,*CP0(L<C"/0<[EMAIL PROTECTED],;0Z6QER%-14Z&K6:*I)<1+#V"9=76.$\;K&O9>E
M-6I)_7U0"CXY;("W&K#XBPBQ[I3B)BX?^DB[-37-S0\ZK_VP0:VT^1R(6YP]
MX$%&^GT!R_3>;=4_QVMQ]^16W-W'3BJ&O,ITU]Y^^?'UGEWQO8XALP-U!%5E
MOI*"&?F,[EMAIL PROTECTED]/GR_?O=U;5Q;@[EMAIL PROTECTED],!?KEE?-G
MRM!:[EMAIL PROTECTED]@1<'[EMAIL PROTECTED]/[EMAIL PROTECTED](Q23856B1Z&;E*F:0G\X84
M:)I7VPG<5Z<L&;O4*>IWR-:R<WYV:)1PM]5++8(D3JD?F[K76UQ-D$XY"&3*
M9WT%]203>`],;1TKD0V!R5R7<+`2Y7-8$A`?^[?2\9-;17"1Y_)BTQ*KR#N\
M,'*RN17T.!']:M9<@VWZ+,K-ZK)OD)ME4"T:L<?H;Q&?S*:KWF[M2+>UMZ4Q
MM&$!I=#;,B^T.HAWU`$U[1A[)"54L$&YN/ZQ)ZL[\GMCO=JM)SL%2G"`>D`R
M7!S7`+6.X<E]>XP4BQSKNB8$5V,+49J*YYC*)A6$;6Z$`)%23J=%775<UUG.
M;%AK83<:[EMAIL PROTECTED](RBWS,X'L4P&5AB-)<5\JT)63?HMRVFAZBTR"F
M!JJ^-/IF9H6^ASW4"_T`+B"7]4!?KK!5Y`<2:I!].U#7--!)_R9?63VMNVL$
MTW!"O%\M.Q]6D2D3)Y.RX&/[$J2N&G>>4CV/,D8C&LY]SX>@LZ]"2+";5N+'
M>Q.RM6:8=MKLALU:RZX9=%0?O'_040,5NQ0,[EMAIL PROTECTED],',J>/5;[EMAIL PROTECTED]/H2.1
M^8\-H4,:!FPF=VCCR/F9PT`-O<CK&!G8<+LY/O4UWDC;[EMAIL PROTECTED]&)H:#I'
M&(&:$X$U]1"JDBEHPKP_FO%R<5OA8@@G$FN*3?G+=WEPR$XFJIH>#7@<$<Z'
*Z'^:WR0C1A0`````
`
end
-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g.
Take an Oracle 10g class now, and we'll give you the exam FREE.
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
