ChangeSet 1.1057, 2003/03/27 14:46:26-08:00, arjanv@redhat.com [PATCH] usb storage horkage fix usb-storage and usb-ohci fixes from me and Pete * Make detect() not sleep with spinlocks helt * Make queuecommand() not sleep with spinlocks helt and/or from irq context by transforming the semaphore into a spinlock * Fix PCI posting mess in usb-ohci * Fix usb-storage abort handling; the completion didn't actually happen due to a design bug so on any timeout you get really stuck drivers/usb/host/usb-ohci.c | 22 +++++++++++++++++++--- drivers/usb/storage/scsiglue.c | 23 +++++++++++++++++------ drivers/usb/storage/usb.c | 10 ++++++---- drivers/usb/storage/usb.h | 6 +++++- 4 files changed, 47 insertions(+), 14 deletions(-) diff -Nru a/drivers/usb/host/usb-ohci.c b/drivers/usb/host/usb-ohci.c --- a/drivers/usb/host/usb-ohci.c Thu Mar 27 16:01:21 2003 +++ b/drivers/usb/host/usb-ohci.c Thu Mar 27 16:01:21 2003 @@ -550,7 +550,7 @@ int i, size = 0; unsigned long flags; int bustime = 0; - int mem_flags = ALLOC_FLAGS; + int mem_flags = GFP_ATOMIC; if (!urb->dev || !urb->dev->bus) return -ENODEV; @@ -708,6 +708,7 @@ /* drive timeouts by SF (messy, but works) */ writel (OHCI_INTR_SF, &ohci->regs->intrenable); + (void)readl (&ohci->regs->intrdisable); /* PCI posting flush */ } #endif @@ -782,8 +783,10 @@ /* wait until all TDs are deleted */ set_current_state(TASK_UNINTERRUPTIBLE); - while (timeout && (urb->status == USB_ST_URB_PENDING)) + while (timeout && (urb->status == USB_ST_URB_PENDING)) { timeout = schedule_timeout (timeout); + set_current_state(TASK_UNINTERRUPTIBLE); + } set_current_state(TASK_RUNNING); remove_wait_queue (&unlink_wakeup, &wait); if (urb->status == USB_ST_URB_PENDING) { @@ -1284,6 +1287,7 @@ /* enable SOF interrupt */ writel (OHCI_INTR_SF, &ohci->regs->intrstatus); writel (OHCI_INTR_SF, &ohci->regs->intrenable); + (void)readl (&ohci->regs->intrdisable); /* PCI posting flush */ } } @@ -1404,6 +1408,7 @@ if (!ohci->sleeping) { wmb(); writel (OHCI_BLF, &ohci->regs->cmdstatus); /* start bulk list */ + (void)readl (&ohci->regs->intrdisable); /* PCI posting flush */ } break; @@ -1432,6 +1437,7 @@ if (!ohci->sleeping) { wmb(); writel (OHCI_CLF, &ohci->regs->cmdstatus); /* start Control list */ + (void)readl (&ohci->regs->intrdisable); /* PCI posting flush */ } break; @@ -2239,6 +2245,8 @@ writel (RH_HS_LPSC, &ohci->regs->roothub.status); #endif /* OHCI_USE_NPS */ + (void)readl (&ohci->regs->intrdisable); /* PCI posting flush */ + // POTPGT delay is bits 24-31, in 2 ms units. mdelay ((roothub_a (ohci) >> 23) & 0x1fe); @@ -2340,24 +2348,30 @@ if (ints & OHCI_INTR_WDH) { writel (OHCI_INTR_WDH, ®s->intrdisable); + (void)readl (®s->intrdisable); /* PCI posting flush */ dl_done_list (ohci, dl_reverse_done_list (ohci)); writel (OHCI_INTR_WDH, ®s->intrenable); + (void)readl (®s->intrdisable); /* PCI posting flush */ } if (ints & OHCI_INTR_SO) { dbg("USB Schedule overrun"); writel (OHCI_INTR_SO, ®s->intrenable); + (void)readl (®s->intrdisable); /* PCI posting flush */ } // FIXME: this assumes SOF (1/ms) interrupts don't get lost... if (ints & OHCI_INTR_SF) { unsigned int frame = le16_to_cpu (ohci->hcca->frame_no) & 1; writel (OHCI_INTR_SF, ®s->intrdisable); + (void)readl (®s->intrdisable); /* PCI posting flush */ if (ohci->ed_rm_list[!frame] != NULL) { dl_del_list (ohci, !frame); } - if (ohci->ed_rm_list[frame] != NULL) + if (ohci->ed_rm_list[frame] != NULL) { writel (OHCI_INTR_SF, ®s->intrenable); + (void)readl (®s->intrdisable); /* PCI posting flush */ + } } if (!list_empty (&ohci->timeout_list)) { @@ -2371,6 +2385,7 @@ writel (ints, ®s->intrstatus); writel (OHCI_INTR_MIE, ®s->intrenable); + (void)readl (®s->intrdisable); /* PCI posting flush */ } /*-------------------------------------------------------------------------*/ @@ -2521,6 +2536,7 @@ /* FIXME this is a second HC reset; why?? */ writel (ohci->hc_control = OHCI_USB_RESET, &ohci->regs->control); + (void)readl (&ohci->regs->intrdisable); /* PCI posting flush */ wait_ms (10); usb_register_bus (ohci->bus); diff -Nru a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c --- a/drivers/usb/storage/scsiglue.c Thu Mar 27 16:01:21 2003 +++ b/drivers/usb/storage/scsiglue.c Thu Mar 27 16:01:21 2003 @@ -75,15 +75,15 @@ { struct us_data *us; char local_name[32]; - + /* Note: this function gets called with io_request_lock spinlock helt! */ /* This is not nice at all, but how else are we to get the * data here? */ us = (struct us_data *)sht->proc_dir; /* set up the name of our subdirectory under /proc/scsi/ */ sprintf(local_name, "usb-storage-%d", us->host_number); - sht->proc_name = kmalloc (strlen(local_name) + 1, GFP_KERNEL); - if (!sht->proc_name) + sht->proc_name = kmalloc (strlen(local_name) + 1, GFP_ATOMIC); + if (!sht->proc_name) return 0; strcpy(sht->proc_name, local_name); @@ -145,12 +145,13 @@ static int queuecommand( Scsi_Cmnd *srb , void (*done)(Scsi_Cmnd *)) { struct us_data *us = (struct us_data *)srb->host->hostdata[0]; + unsigned long flags; US_DEBUGP("queuecommand() called\n"); srb->host_scribble = (unsigned char *)us; /* get exclusive access to the structures we want */ - down(&(us->queue_exclusion)); + spin_lock_irqsave(&(us->queue_exclusion), flags); /* enqueue the command */ us->queue_srb = srb; @@ -158,7 +159,7 @@ us->action = US_ACT_COMMAND; /* release the lock on the structure */ - up(&(us->queue_exclusion)); + spin_unlock_irqrestore(&(us->queue_exclusion), flags); /* wake up the process task */ up(&(us->sema)); @@ -191,11 +192,15 @@ /* if we have an urb pending, let's wake the control thread up */ if (!us->current_done.done) { + atomic_inc(&us->abortcnt); + spin_unlock_irq(&io_request_lock); /* cancel the URB -- this will automatically wake the thread */ usb_unlink_urb(us->current_urb); /* wait for us to be done */ wait_for_completion(&(us->notify)); + spin_lock_irq(&io_request_lock); + atomic_dec(&us->abortcnt); return SUCCESS; } @@ -231,6 +236,8 @@ return SUCCESS; } + spin_unlock_irq(&io_request_lock); + /* release the IRQ, if we have one */ down(&(us->irq_urb_sem)); if (us->irq_urb) { @@ -241,8 +248,10 @@ up(&(us->irq_urb_sem)); /* attempt to reset the port */ - if (usb_reset_device(us->pusb_dev) < 0) + if (usb_reset_device(us->pusb_dev) < 0) { + spin_lock_irq(&io_request_lock); return FAILED; + } /* FIXME: This needs to lock out driver probing while it's working * or we can have race conditions */ @@ -282,6 +291,8 @@ US_DEBUGP("usb_submit_urb() returns %d\n", result); up(&(us->irq_urb_sem)); } + + spin_lock_irq(&io_request_lock); US_DEBUGP("bus_reset() complete\n"); return SUCCESS; diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c --- a/drivers/usb/storage/usb.c Thu Mar 27 16:01:21 2003 +++ b/drivers/usb/storage/usb.c Thu Mar 27 16:01:21 2003 @@ -343,6 +343,7 @@ set_current_state(TASK_INTERRUPTIBLE); for(;;) { + unsigned long flags; US_DEBUGP("*** thread sleeping.\n"); if(down_interruptible(&us->sema)) break; @@ -350,7 +351,7 @@ US_DEBUGP("*** thread awakened.\n"); /* lock access to the queue element */ - down(&(us->queue_exclusion)); + spin_lock_irqsave(&(us->queue_exclusion), flags); /* take the command off the queue */ action = us->action; @@ -358,7 +359,7 @@ us->srb = us->queue_srb; /* release the queue lock as fast as possible */ - up(&(us->queue_exclusion)); + spin_unlock_irqrestore(&(us->queue_exclusion), flags); switch (action) { case US_ACT_COMMAND: @@ -469,7 +470,8 @@ us->srb->result); set_current_state(TASK_INTERRUPTIBLE); us->srb->scsi_done(us->srb); - } else { + }; + if (atomic_read(&us->abortcnt) != 0) { US_DEBUGP("scsi command aborted\n"); set_current_state(TASK_INTERRUPTIBLE); complete(&(us->notify)); @@ -773,7 +775,7 @@ /* Initialize the mutexes only when the struct is new */ init_completion(&(ss->notify)); init_MUTEX_LOCKED(&(ss->ip_waitq)); - init_MUTEX(&(ss->queue_exclusion)); + spin_lock_init(&(ss->queue_exclusion)); init_MUTEX(&(ss->irq_urb_sem)); init_MUTEX(&(ss->current_urb_sem)); init_MUTEX(&(ss->dev_semaphore)); diff -Nru a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h --- a/drivers/usb/storage/usb.h Thu Mar 27 16:01:21 2003 +++ b/drivers/usb/storage/usb.h Thu Mar 27 16:01:21 2003 @@ -48,6 +48,8 @@ #include #include #include +#include +#include #include "scsi.h" #include "hosts.h" @@ -147,6 +149,8 @@ int host_number; /* to find us */ int host_no; /* allocated by scsi */ Scsi_Cmnd *srb; /* current srb */ + atomic_t abortcnt; /* must complete(¬ify) */ + /* thread information */ Scsi_Cmnd *queue_srb; /* the single queue slot */ @@ -173,7 +177,7 @@ /* mutual exclusion structures */ struct completion notify; /* thread begin/end */ - struct semaphore queue_exclusion; /* to protect data structs */ + spinlock_t queue_exclusion; /* to protect data structs */ struct us_unusual_dev *unusual_dev; /* If unusual device */ void *extra; /* Any extra data */ extra_data_destructor extra_destructor;/* extra data destructor */