On Wed, Mar 14, 2012 at 2:33 PM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote: > On Wed, 2012-03-14 at 17:11 -0400, Alan Stern wrote: >> I see. It's a nasty situation; I guess the best way to describe it is >> a conflict between the requirements of the PM and SCSI subsystems: >> >> The PM core runs suspend and resume operations in async >> threads, and these threads need to acquire the device lock; >> >> The sd driver needs to insure that async probing is finished >> when starting its remove routine, and it is called with the >> device lock held. >> >> The best solution might be to use a workqueue for sd's async probing >> instead of the "async" threads. Then the work routine could be >> cancelled without doing async_synchronize_full(). > > Hmm... I was wondering why we needed a kernel global sync. If this is > just for probe work what about just doing something like the following? > Keep the sync operation local to probe-work and not entangle with the > resume-work? I'll give this a shot when I get back to my test system. > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index bd17cf8..ec69e35 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -2629,6 +2629,8 @@ static void sd_probe_async(void *data, async_cookie_t cookie) > put_device(&sdkp->dev); > } > > +static LIST_HEAD(sd_probe_domain); > + > /** > * sd_probe - called during driver initialization and whenever a > * new scsi device is attached to the system. It is called once > @@ -2717,7 +2719,7 @@ static int sd_probe(struct device *dev) > dev_set_drvdata(dev, sdkp); > > get_device(&sdkp->dev); /* prevent release before async_schedule */ > - async_schedule(sd_probe_async, sdkp); > + async_schedule_domain(sd_probe_async, sdkp, &sd_probe_domain); > > return 0; > > @@ -2751,7 +2753,7 @@ static int sd_remove(struct device *dev) > sdkp = dev_get_drvdata(dev); > scsi_autopm_get_device(sdkp->device); > > - async_synchronize_full(); > + async_synchronize_full_domain(&sd_probe_domain); > blk_queue_prep_rq(sdkp->device->request_queue, scsi_prep_fn); > blk_queue_unprep_rq(sdkp->device->request_queue, NULL); > device_del(&sdkp->dev); This works fine for me for resolving the deadlock, but I found I also needed the following to fix a spurious: scsi 6:0:1:0: Illegal state transition deleted->running ...in the resume path. @@ -2348,10 +2349,11 @@ EXPORT_SYMBOL(scsi_device_quiesce); * * Must be called with user context, may sleep. */ -void -scsi_device_resume(struct scsi_device *sdev) +void scsi_device_resume(struct scsi_device *sdev) { - if(scsi_device_set_state(sdev, SDEV_RUNNING)) + /* check if the device was deleted during suspend */ + if (sdev->sdev_state == SDEV_DEL || + scsi_device_set_state(sdev, SDEV_RUNNING)) return; scsi_run_queue(sdev->request_queue); } Unless someone can point out something I'm missing I'll go ahead and roll this into it's own patch and rebase/drop the hack out of the libsas resume code. -- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html