Good point. I tested your suggested patch fix and it works well in my test use case. BTW, I've got test code in my just tested version of your patch to read the kref of the sdev_gendev between the calls to scsi_run_queue and put_device to increase the likelihood that the use case has actually been tested. > -----Original Message----- > From: James Bottomley [mailto:James.Bottomley@xxxxxxxxxxxx] > Sent: Tuesday, November 08, 2005 12:02 PM > To: goggin, edward > Cc: 'Andrew Morton'; Masanari Iida; > linux-kernel@xxxxxxxxxxxxxxx; > linux-usb-devel@xxxxxxxxxxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx > Subject: RE: oops with USB Storage on 2.6.14 > > On Tue, 2005-11-08 at 11:24 -0500, goggin, edward wrote: > > ! struct scsi_device *sdev = cmd->device; > > ! struct request_queue *q = sdev->request_queue; > > ! > > ! // need to hold a reference on the device before we let > go of the > > cmd > > ! if (scsi_device_get(sdev)) { > > ! scsi_put_command(cmd); > > ! return; // maybe sdev_state == > SDEV_CANCEL, SDEV_DEL > > ! } > > > > scsi_put_command(cmd); > > scsi_run_queue(q); > > + > > + // ok to remove device now > > + scsi_device_put(sdev); > > This is the right idea, I think, but not necessarily the right fix. > scsi_device_get() will fail if the device is going offline, > but we would > still need to run the queues. > > try this sequence instead: > > get_device(&sdev->sdev_gendev); > scsi_put_command(cmd); > scsi_run_queue(q); > put_device(&sdev->sdev_gendev); > > James > > - : send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
