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