Am Donnerstag, 20. Dezember 2007 18:54:49 schrieb Greg KH:
 
> > From a more general angle, perhaps what we provide with usb_kill_urb()
> > is not ideally suited to combat this problem. What about a construction like
> > this:
> 
> That's nice, but it will not solve the issue with devices like the
> io-edgeport, which sends a new control message to the device from what I
> remember.  Or it will not work for any device that creates new urbs for
> every message, a model drivers are moving more toward over time...

OK, here's the version without a race against reset.

The basic idea is still to use anchor to block submission. So you the
disconnect() method can use the anchor to render harmless any code
paths using the anchor before submitting.

Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]>

        Regards
                Oliver

----

--- linux-2.6.24-rc7/drivers/usb/usb-skeleton.c 2008-01-16 10:31:12.000000000 
+0100
+++ linux-2.6.24-poison/drivers/usb/usb-skeleton.c      2008-01-16 
12:28:57.000000000 +0100
@@ -282,14 +282,6 @@ static ssize_t skel_write(struct file *f
                goto error;
        }
 
-       /* this lock makes sure we don't submit URBs to gone devices */
-       mutex_lock(&dev->io_mutex);
-       if (!dev->interface) {          /* disconnect() was called */
-               mutex_unlock(&dev->io_mutex);
-               retval = -ENODEV;
-               goto error;
-       }
-
        /* initialize the urb properly */
        usb_fill_bulk_urb(urb, dev->udev,
                          usb_sndbulkpipe(dev->udev, 
dev->bulk_out_endpointAddr),
@@ -297,6 +289,8 @@ static ssize_t skel_write(struct file *f
        urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
        usb_anchor_urb(urb, &dev->submitted);
 
+       /* no IO while a device is being reset */
+       mutex_lock(&dev->io_mutex);
        /* send the data out the bulk port */
        retval = usb_submit_urb(urb, GFP_KERNEL);
        mutex_unlock(&dev->io_mutex);
@@ -431,12 +425,12 @@ static void skel_disconnect(struct usb_i
        /* give back our minor */
        usb_deregister_dev(interface, &skel_class);
 
-       /* prevent more I/O from starting */
+       /* prevent more I/O from starting in the read path*/
        mutex_lock(&dev->io_mutex);
        dev->interface = NULL;
        mutex_unlock(&dev->io_mutex);
 
-       usb_kill_anchored_urbs(&dev->submitted);
+       usb_poison_anchored_urbs(&dev->submitted);
 
        /* decrement our usage count */
        kref_put(&dev->kref, skel_delete);
--- linux-2.6.24-rc7/drivers/usb/core/urb.c     2008-01-16 10:31:12.000000000 
+0100
+++ linux-2.6.24-poison/drivers/usb/core/urb.c  2008-01-16 09:41:13.000000000 
+0100
@@ -10,6 +10,8 @@
 
 #define to_urb(d) container_of(d, struct urb, kref)
 
+static DEFINE_MUTEX(usb_reject_mutex);
+
 static void urb_destroy(struct kref *kref)
 {
        struct urb *urb = to_urb(kref);
@@ -38,6 +40,7 @@ void usb_init_urb(struct urb *urb)
 {
        if (urb) {
                memset(urb, 0, sizeof(*urb));
+               atomic_set(&urb->reject, 0);
                kref_init(&urb->kref);
                INIT_LIST_HEAD(&urb->anchor_list);
        }
@@ -121,6 +124,9 @@ void usb_anchor_urb(struct urb *urb, str
 
        spin_lock_irqsave(&anchor->lock, flags);
        usb_get_urb(urb);
+       if (unlikely(anchor->weighed))
+               atomic_inc(&urb->reject);
+
        list_add_tail(&urb->anchor_list, &anchor->urb_list);
        urb->anchor = anchor;
        spin_unlock_irqrestore(&anchor->lock, flags);
@@ -537,21 +543,52 @@ int usb_unlink_urb(struct urb *urb)
  */
 void usb_kill_urb(struct urb *urb)
 {
-       static DEFINE_MUTEX(reject_mutex);
-
        might_sleep();
        if (!(urb && urb->dev && urb->ep))
                return;
-       mutex_lock(&reject_mutex);
-       ++urb->reject;
-       mutex_unlock(&reject_mutex);
+       mutex_lock(&usb_reject_mutex);
+       atomic_inc(&urb->reject);
+       mutex_unlock(&usb_reject_mutex);
 
        usb_hcd_unlink_urb(urb, -ENOENT);
        wait_event(usb_kill_urb_queue, atomic_read(&urb->use_count) == 0);
 
-       mutex_lock(&reject_mutex);
-       --urb->reject;
-       mutex_unlock(&reject_mutex);
+       mutex_lock(&usb_reject_mutex);
+       atomic_dec(&urb->reject);
+       mutex_unlock(&usb_reject_mutex);
+}
+
+/**
+ * usb_poison_urb - cancel an URB, wait for it to finish, prevent its use
+ * @urb: pointer to URB describing a previously submitted request,
+ *     may be NULL
+ *
+ * This routine cancels an in-progress request.  It is guaranteed that
+ * upon return all completion handlers will have finished and the URB
+ * cannot be reused.  These features make
+ * this an ideal way to stop I/O in a disconnect() callback or close()
+ * function.  If the request has not already finished or been unlinked
+ * the completion handler will see urb->status == -ENOENT.
+ *
+ * After the routine has been run, attempts to resubmit the URB will fail
+ * with error -EPERM.  Thus even if a driver tries to resubmit, the device
+ * won't be bothered
+ *
+ * This routine may not be used in an interrupt context (such as a bottom
+ * half or a completion handler), or when holding a spinlock, or in other
+ * situations where the caller can't schedule().
+ */
+void usb_poison_urb(struct urb *urb)
+{
+       might_sleep();
+       if (!(urb && urb->dev && urb->ep))
+               return;
+       mutex_lock(&usb_reject_mutex);
+       atomic_inc(&urb->reject);
+       mutex_unlock(&usb_reject_mutex);
+
+       usb_hcd_unlink_urb(urb, -ENOENT);
+       wait_event(usb_kill_urb_queue, atomic_read(&urb->use_count) == 0);
 }
 
 /**
@@ -581,6 +618,32 @@ void usb_kill_anchored_urbs(struct usb_a
 EXPORT_SYMBOL_GPL(usb_kill_anchored_urbs);
 
 /**
+ * usb_poison_anchored_urbs - block transfer requests en masse
+ * @anchor: anchor the requests are bound to
+ *
+ * this allows all outstanding URBs to be killed starting
+ * from the back of the queue and blocks their submisson
+ */
+void usb_poison_anchored_urbs(struct usb_anchor *anchor)
+{
+       struct urb *victim;
+
+       spin_lock_irq(&anchor->lock);
+       while (!list_empty(&anchor->urb_list)) {
+               victim = list_entry(anchor->urb_list.prev, struct urb, 
anchor_list);
+               /* we must make sure the URB isn't freed before we kill it*/
+               usb_get_urb(victim);
+               spin_unlock_irq(&anchor->lock);
+               /* this will unanchor the URB */
+               usb_poison_urb(victim);
+               usb_put_urb(victim);
+               spin_lock_irq(&anchor->lock);
+       }
+       spin_unlock_irq(&anchor->lock);
+}
+EXPORT_SYMBOL_GPL(usb_poison_anchored_urbs);
+
+/**
  * usb_wait_anchor_empty_timeout - wait for an anchor to be unused
  * @anchor: the anchor you want to become unused
  * @timeout: how long you are willing to wait in milliseconds
@@ -596,6 +659,20 @@ int usb_wait_anchor_empty_timeout(struct
 }
 EXPORT_SYMBOL_GPL(usb_wait_anchor_empty_timeout);
 
+void usb_weigh_anchor(struct usb_anchor *anchor)
+{
+       unsigned long flags;
+
+       /* Nothing new */
+       spin_lock_irqsave(&anchor->lock, flags);
+       anchor->weighed = 1;
+       spin_unlock_irqrestore(&anchor->lock, flags);
+
+       /* poison anything already in */
+       usb_poison_anchored_urbs(anchor);
+}
+EXPORT_SYMBOL_GPL(usb_weigh_anchor);
+
 EXPORT_SYMBOL(usb_init_urb);
 EXPORT_SYMBOL(usb_alloc_urb);
 EXPORT_SYMBOL(usb_free_urb);
@@ -603,3 +680,4 @@ EXPORT_SYMBOL(usb_get_urb);
 EXPORT_SYMBOL(usb_submit_urb);
 EXPORT_SYMBOL(usb_unlink_urb);
 EXPORT_SYMBOL(usb_kill_urb);
+EXPORT_SYMBOL(usb_poison_urb);
\ Kein Zeilenumbruch am Dateiende.
--- linux-2.6.24-rc7/include/linux/usb.h        2008-01-16 10:31:30.000000000 
+0100
+++ linux-2.6.24-poison/include/linux/usb.h     2008-01-16 09:29:36.000000000 
+0100
@@ -1064,6 +1064,7 @@ struct usb_anchor {
        struct list_head urb_list;
        wait_queue_head_t wait;
        spinlock_t lock;
+       unsigned weighed:1;
 };
 
 static inline void init_usb_anchor(struct usb_anchor *anchor)
@@ -1248,7 +1249,7 @@ struct urb
        struct kref kref;               /* reference count of the URB */
        void *hcpriv;                   /* private data for host controller */
        atomic_t use_count;             /* concurrent submissions counter */
-       u8 reject;                      /* submissions will fail */
+       atomic_t reject;                /* submissions will fail */
        int unlinked;                   /* unlink error code */
 
        /* public: documented fields in the urb that can be used by drivers */
@@ -1389,11 +1390,14 @@ extern struct urb *usb_get_urb(struct ur
 extern int usb_submit_urb(struct urb *urb, gfp_t mem_flags);
 extern int usb_unlink_urb(struct urb *urb);
 extern void usb_kill_urb(struct urb *urb);
+extern void usb_poison_urb(struct urb *urb);
 extern void usb_kill_anchored_urbs(struct usb_anchor *anchor);
 extern void usb_anchor_urb(struct urb *urb, struct usb_anchor *anchor);
 extern void usb_unanchor_urb(struct urb *urb);
 extern int usb_wait_anchor_empty_timeout(struct usb_anchor *anchor,
                                         unsigned int timeout);
+extern void usb_weigh_anchor(struct usb_anchor *anchor);
+extern void usb_poison_anchored_urbs(struct usb_anchor *anchor);
 
 /**
  * usb_urb_dir_in - check if an URB describes an IN transfer
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to