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

Reply via email to