Hi,
thanks for your suggestion about a spinlock.
Here it is again, after a redesign based on your suggestion.
I'll start testing this, when I'm back home.
Regards
Oliver
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], 2003-01-14 14:59:45+01:00, [EMAIL PROTECTED]
- explicit URB state
- improvement in unlinking semantics
drivers/usb/core/hcd.c | 41 ++++++++++++++++++++++++++++++++++++-----
drivers/usb/core/urb.c | 24 ++++++++++++++++++++++++
include/linux/usb.h | 7 +++++++
3 files changed, 67 insertions(+), 5 deletions(-)
diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
--- a/drivers/usb/core/hcd.c Tue Jan 14 15:07:14 2003
+++ b/drivers/usb/core/hcd.c Tue Jan 14 15:07:14 2003
@@ -1138,14 +1138,23 @@
goto done;
}
- /* Any status except -EINPROGRESS means something already started to
- * unlink this URB from the hardware. So there's no more work to do.
- *
- * FIXME use better explicit urb state
+ /*
+ * We decide based upon the internal_state what's to be done
*/
- if (urb->status != -EINPROGRESS) {
+ switch (urb->internal_state) {
+ case URB_SUBMITTED: /* normal case */
+ urb->internal_state = URB_CANCELED;
+ break;
+ default:
+ BUG();
+ case URB_IDLE:
+ case URB_CANCELED:
retval = -EBUSY;
goto done;
+ case URB_COMPLETING:
+ retval = -EBUSY;
+ urb->internal_state = URB_CANCELED;
+ goto err_out_waiting;
}
/* maybe set up to block until the urb's completion fires. the
@@ -1205,6 +1214,18 @@
if (retval && sys)
dev_dbg (sys, "hcd_unlink_urb %p fail %d\n", urb, retval);
return retval;
+
+err_out_waiting:
+ /* we are asked to unlink while the completing routine is already
+ running. We drop the locks and spin in case of synchronous unlink */
+ spin_unlock (&hcd_data_lock);
+ spin_unlock_irqrestore (&urb->lock, flags);
+ if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
+ spin_lock_irq(&urb->handler_lock);
+ spin_unlock_irq(&urb->handler_lock);
+ }
+
+ return retval;
}
/*-------------------------------------------------------------------------*/
@@ -1287,6 +1308,10 @@
void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb, struct pt_regs *regs)
{
urb_unlink (urb);
+ spin_lock(&urb->lock);
+ if (urb->internal_state == URB_SUBMITTED)
+ urb->internal_state = URB_COMPLETING;
+ spin_unlock(&urb->lock);
// NOTE: a generic device/urb monitoring hook would go here.
// hcd_monitor_hook(MONITOR_URB_FINISH, urb, dev)
@@ -1307,7 +1332,13 @@
}
/* pass ownership to the completion handler */
+ spin_lock(&urb->handler_lock);
urb->complete (urb, regs);
+ spin_unlock(&urb->handler_lock);
+ /* we are now idle in any case */
+ spin_lock(&urb->lock);
+ urb->internal_state = URB_IDLE;
+ spin_unlock(&urb->lock);
usb_put_urb (urb);
}
EXPORT_SYMBOL (usb_hcd_giveback_urb);
diff -Nru a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
--- a/drivers/usb/core/urb.c Tue Jan 14 15:07:14 2003
+++ b/drivers/usb/core/urb.c Tue Jan 14 15:07:14 2003
@@ -44,6 +44,7 @@
memset(urb, 0, sizeof(*urb));
urb->count = (atomic_t)ATOMIC_INIT(1);
spin_lock_init(&urb->lock);
+ spin_lock_init(&urb->handler_lock);
return urb;
}
@@ -192,6 +193,8 @@
struct usb_device *dev;
struct usb_operations *op;
int is_out;
+ int r;
+ unsigned long flags;
if (!urb || urb->hcpriv || !urb->complete)
return -EINVAL;
@@ -346,7 +349,28 @@
urb->interval = temp;
}
+ spin_lock_irqsave(urb->lock, flags);
+ switch (urb->internal_state) {
+ case URB_IDLE:
+ case URB_COMPLETING:
+ urb->internal_state = URB_SUBMITTED;
+ break;
+ case URB_CANCELED:
+ r = -EINTR;
+ goto err_out;
+ case URB_SUBMITTED:
+ default:
+ BUG();
+ r = -EBUSY;
+ goto err_out;
+ }
+ spin_unlock_irqrestore(urb->lock, flags);
+
return op->submit_urb (urb, mem_flags);
+
+err_out:
+ spin_unlock_irqrestore(urb->lock, flags);
+ return r;
}
/*-------------------------------------------------------------------*/
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Tue Jan 14 15:07:14 2003
+++ b/include/linux/usb.h Tue Jan 14 15:07:14 2003
@@ -728,6 +728,8 @@
{
spinlock_t lock; /* lock for the URB */
atomic_t count; /* reference count of the URB */
+ int internal_state; /* state in the URB's lifecycle */
+ spinlock_t handler_lock; /* lock during completion routine */
void *hcpriv; /* private data for host controller */
struct list_head urb_list; /* list pointer to all active urbs */
struct usb_device *dev; /* (in) pointer to associated device */
@@ -750,6 +752,11 @@
usb_complete_t complete; /* (in) completion routine */
struct usb_iso_packet_descriptor iso_frame_desc[0]; /* (in) ISO ONLY */
};
+
+#define URB_IDLE 0
+#define URB_SUBMITTED 1
+#define URB_COMPLETING 2
+#define URB_CANCELED 3
/* -------------------------------------------------------------------------- */
===================================================================
This BitKeeper patch contains the following changesets:
1.1271
## Wrapped with gzip_uu ##
begin 664 bkpatch13588
M'XL(`)09)#X``ZU7;6_:2!#^;/^*.57J)>D!NWX%HE1M7I1#3=,H"3I5JH06
M>P$+LZ9K.S0Z^M]O=LV;B:%-+E$DS.[.>.:9YYE9WD`WY;)M)''TP*7Y!OY.
MT@R_<I$(7A\E$QY'(O]13^00-V^3!#<;:KE16#3ZXUHF.4\;>6RY)IZY85DP
M`MQ*VP:MVZN5[''*V\;MQ67WZN.M:9Z<P-F(B2&_XQF<G)C]\8<PYW%]+!,V
M4J^;K[;G%B&44LLEMNM1=VXU;=N94X_T0WP,+;\UZ'LM<RCY\$-A'B23LKF-
M#AQ"7,="<\^G3?,<:)U:/@5B-PAM4`>HTW9;;<=]1VB;$"CR^U"%!+RSH4;,
M4_C_09^9`=2`_YC&41!ET+T]A31C&=?+T60JDP<^X2*#2$`N,()Q)(:0\@D3
M612DYB>P28M2\V:-IEE[YI]I$D;,][`%8"@5`%C8M-\($LD;N>S7`Y6713QB
M4]=QJ#>GI&6W,"\G)(%G^>Z@Y7I-O@^]/8YUF:CM^L2?>XY++8SJ*<:1".(\
MY`WM47FIC]9H.UAD,B>>X_CST++LL!FZ[L!S;(_N#6J'T\V(++]IJ8BFBL_;
M06TF-0K")5"4J`_7:<WMED_]N>VT6-]A#F/4L7G3>1Y0*\?KL#R%/7&TH"IR
MJ);62Q'<UM@^T+3:"/+#M^V%VNRRV/PV\7]';/[KBBT2F4S"/.!KV6U(;K49
M)\$8!HD$3'0:\RQ*!,@DSR+!E>P*,GR!FISI?Y31354!7J#&CF\3L$PCTK+/
MN!0L[ND0CPVC<51$JQI"-N*J8_R90AP->/`8Q!R.&J:13B.APN]E@`B%,9<]
M]?58&>NTPERJ-O(T,V7>\5T+7/.;^2;D`[6&K^AUSJ\N#%):NNN>?N[<WU^<
M&[2T?O;E\\W5Q7WG^M*PRAL?K\\NKO"\K=E:W08485^Q$3W#%T57R!^7-%'I
M2"--6HN\C+.6\[JDS=,G=/T$18O<XF!UAB^AH>,!+<BDZ=.+1)0=O$5WM?>;
MM#H\-CNTY2P8:QB&/#:-7*314/`0^2:&AC&(V3#%<[;3!.J7G,KO*7O@!]JM
M6OD+]&'T:J2S2-T<BKVR$@[A7],(6+IF9WOC^YJ"N&I4F,-)F<+X,J,O.1L?
M;WI9\%7YD&A1N^A<W]^JH\,D2X!+V4/5;%JL_*$)$I_E<::,3[N7!RJ?A9?3
M[MW7"B\_%[#D8@F,Y&F&%:S"YIO"L@4./BP\M)]C;DB>Y1)E?UPM13UHE!1?
M<];]4HNE\48L?'#IG.+<*:YK3?^)%KW?NJQY4'-?68MLT84?F(Q8/RZ&QX2-
M.:2YY-B9609L@)13)Q]%,)*)2/)T<8<#D2P[,T0IR%P([,>HZ&*6_TK1&J<7
M*/I<71J0,QW\U'IM')D&',$_'$(>1"&'/C(YA'R:%,-E2S0SS`J'#;*VCQ:(
MM/:HND1'?S:?(=FU5`"GDDCDA,6@=]4$VZ/9I2HW)5NAM=V]8:5J%703X=C5
M-U`C#QC4IF1_+ZQ-8?=F+,+).E1-TB(^8,->27:YUU:5@!FR"JG#TC&6`!TL
MJ#(;13'7U5B.:YS<RW&-Y&$Q@A`^HJ0+%M5U.64RU39*]GA&A*":@[HTZ&23
M014KEW>'10^!@[?(M%[(,K9L]#M:#"S&PE:3B09P\$=!A4PRD0YP8NA->*LQ
M^WCW]?JLU[V^ZEQ_.M3T*`^&ZF%C;`>QX]A/!'K5YW0I=0V:JFNNW[,1^3+D
MRB)O#8S#_618\:@,6?EE'8J_W4HC=M=TQ8L)WL@J/&WGO*:12&80X9XJ.1./
F:VGMRGQW-DI!^_)8_<0/1AS9ED].!K8W"/$>9OX'C@'';U\0````
`
end
-------------------------------------------------------
This SF.NET email is sponsored by: FREE SSL Guide from Thawte
are you planning your Web Server Security? Click here to get a FREE
Thawte SSL guide and find the answers to all your SSL security issues.
http://ads.sourceforge.net/cgi-bin/redirect.pl?thaw0026en
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
