On 2020-08-28 08:37, Alan Stern wrote: > On Thu, Aug 27, 2020 at 08:27:49PM -0700, Bart Van Assche wrote: >> On 2020-08-27 13:33, Alan Stern wrote: >>> It may not need to be that complicated. what about something like this? > >> I think this patch will break SCSI domain validation. The SCSI domain >> validation code calls scsi_device_quiesce() and that function in turn calls >> blk_set_pm_only(). The SCSI domain validation code submits SCSI commands with >> the BLK_MQ_REQ_PREEMPT flag. Since the above code postpones such requests >> while blk_set_pm_only() is in effect, I think the above patch will cause the >> SCSI domain validation code to deadlock. > > Yes, you're right. > > There may be an even simpler solution: Ensure that SCSI domain > validation is mutually exclusive with runtime PM. It's already mutually > exclusive with system PM, so this makes sense. > > What do you think of the patch below? > > Alan Stern > > > Index: usb-devel/drivers/scsi/scsi_transport_spi.c > =================================================================== > --- usb-devel.orig/drivers/scsi/scsi_transport_spi.c > +++ usb-devel/drivers/scsi/scsi_transport_spi.c > @@ -1001,7 +1001,7 @@ spi_dv_device(struct scsi_device *sdev) > * Because this function and the power management code both call > * scsi_device_quiesce(), it is not safe to perform domain validation > * while suspend or resume is in progress. Hence the > - * lock/unlock_system_sleep() calls. > + * lock/unlock_system_sleep() and scsi_autopm_get/put_device() calls. > */ > lock_system_sleep(); > > @@ -1018,10 +1018,13 @@ spi_dv_device(struct scsi_device *sdev) > if (unlikely(!buffer)) > goto out_put; > > + if (scsi_autopm_get_device(sdev)) > + goto out_free; > + > /* We need to verify that the actual device will quiesce; the > * later target quiesce is just a nice to have */ > if (unlikely(scsi_device_quiesce(sdev))) > - goto out_free; > + goto out_autopm_put; > > scsi_target_quiesce(starget); > > @@ -1041,6 +1044,8 @@ spi_dv_device(struct scsi_device *sdev) > > spi_initial_dv(starget) = 1; > > + out_autopm_put: > + scsi_autopm_put_device(sdev); > out_free: > kfree(buffer); > out_put: Hi Alan, I think this is only a part of the solution. scsi_target_quiesce() invokes scsi_device_quiesce() and that function in turn calls blk_set_pm_only(). So I think that the above is not sufficient to fix the deadlock mentioned in my previous email. Maybe it is possible to fix this by creating a new request queue and by submitting the DV SCSI commands to that new request queue. There may be better solutions. Bart.
