| Message ID | 20210111130036.711898511@linuxfoundation.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
Hi! > From: Bart Van Assche <bvanassche@acm.org> > > [ Upstream commit cfefd9f8240a7b9fdd96fcd54cb029870b6d8d88 ] > > Disable runtime power management during domain validation. Since a later > patch removes RQF_PREEMPT, set RQF_PM for domain validation commands such > that these are executed in the quiesced SCSI device state. This and "05/77] scsi: ide: Do not set the RQF_PREEMPT flag for" do not fix anything AFAICT. They are in series with other patches in 5.10, so they may make sense there, but I don't think we need them in 4.19. Best regards, Pavel > index 69213842e63e0..efb9c3d902133 100644 > --- a/drivers/scsi/scsi_transport_spi.c > +++ b/drivers/scsi/scsi_transport_spi.c > @@ -130,12 +130,16 @@ static int spi_execute(struct scsi_device *sdev, const void *cmd, > sshdr = &sshdr_tmp; > > for(i = 0; i < DV_RETRIES; i++) { > + /* > + * The purpose of the RQF_PM flag below is to bypass the > + * SDEV_QUIESCE state. > + */ > result = scsi_execute(sdev, cmd, dir, buffer, bufflen, sense, > sshdr, DV_TIMEOUT, /* retries */ 1, > REQ_FAILFAST_DEV | > REQ_FAILFAST_TRANSPORT | > REQ_FAILFAST_DRIVER, > - 0, NULL); > + RQF_PM, NULL); > if (driver_byte(result) != DRIVER_SENSE || > sshdr->sense_key != UNIT_ATTENTION) > break; > @@ -1018,23 +1022,26 @@ spi_dv_device(struct scsi_device *sdev) > */ > lock_system_sleep(); > > + if (scsi_autopm_get_device(sdev)) > + goto unlock_system_sleep; > + > if (unlikely(spi_dv_in_progress(starget))) > - goto unlock; > + goto put_autopm; > > if (unlikely(scsi_device_get(sdev))) > - goto unlock; > + goto put_autopm; > > spi_dv_in_progress(starget) = 1; > > buffer = kzalloc(len, GFP_KERNEL); > > if (unlikely(!buffer)) > - goto out_put; > + goto put_sdev; > > /* 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 free_buffer; > > scsi_target_quiesce(starget); > > @@ -1054,12 +1061,16 @@ spi_dv_device(struct scsi_device *sdev) > > spi_initial_dv(starget) = 1; > > - out_free: > +free_buffer: > kfree(buffer); > - out_put: > + > +put_sdev: > spi_dv_in_progress(starget) = 0; > scsi_device_put(sdev); > -unlock: > +put_autopm: > + scsi_autopm_put_device(sdev); > + > +unlock_system_sleep: > unlock_system_sleep(); > } > EXPORT_SYMBOL(spi_dv_device); > -- > 2.27.0 > > -- http://www.livejournal.com/~pavelmachek
On 1/13/21 3:47 AM, Pavel Machek wrote: >> From: Bart Van Assche <bvanassche@acm.org> >> >> [ Upstream commit cfefd9f8240a7b9fdd96fcd54cb029870b6d8d88 ] >> >> Disable runtime power management during domain validation. Since a later >> patch removes RQF_PREEMPT, set RQF_PM for domain validation commands such >> that these are executed in the quiesced SCSI device state. > > This and "05/77] scsi: ide: Do not set the RQF_PREEMPT flag for" do > not fix anything AFAICT. They are in series with other patches in > 5.10, so they may make sense there, but I don't think we need them in > 4.19. Agreed. Please either backport the entire series of 8 patches or do not backport any patch from that series. Selecting a subset of the patches of that series is dangerous. As an example, applying patch 8/8 without applying the prior patches from that series would break SCSI domain validation. See also https://lore.kernel.org/linux-scsi/20201209052951.16136-1-bvanassche@acm.org/ Thanks, Bart.
diff --git a/drivers/scsi/scsi_transport_spi.c b/drivers/scsi/scsi_transport_spi.c index 69213842e63e0..efb9c3d902133 100644 --- a/drivers/scsi/scsi_transport_spi.c +++ b/drivers/scsi/scsi_transport_spi.c @@ -130,12 +130,16 @@ static int spi_execute(struct scsi_device *sdev, const void *cmd, sshdr = &sshdr_tmp; for(i = 0; i < DV_RETRIES; i++) { + /* + * The purpose of the RQF_PM flag below is to bypass the + * SDEV_QUIESCE state. + */ result = scsi_execute(sdev, cmd, dir, buffer, bufflen, sense, sshdr, DV_TIMEOUT, /* retries */ 1, REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER, - 0, NULL); + RQF_PM, NULL); if (driver_byte(result) != DRIVER_SENSE || sshdr->sense_key != UNIT_ATTENTION) break; @@ -1018,23 +1022,26 @@ spi_dv_device(struct scsi_device *sdev) */ lock_system_sleep(); + if (scsi_autopm_get_device(sdev)) + goto unlock_system_sleep; + if (unlikely(spi_dv_in_progress(starget))) - goto unlock; + goto put_autopm; if (unlikely(scsi_device_get(sdev))) - goto unlock; + goto put_autopm; spi_dv_in_progress(starget) = 1; buffer = kzalloc(len, GFP_KERNEL); if (unlikely(!buffer)) - goto out_put; + goto put_sdev; /* 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 free_buffer; scsi_target_quiesce(starget); @@ -1054,12 +1061,16 @@ spi_dv_device(struct scsi_device *sdev) spi_initial_dv(starget) = 1; - out_free: +free_buffer: kfree(buffer); - out_put: + +put_sdev: spi_dv_in_progress(starget) = 0; scsi_device_put(sdev); -unlock: +put_autopm: + scsi_autopm_put_device(sdev); + +unlock_system_sleep: unlock_system_sleep(); } EXPORT_SYMBOL(spi_dv_device);