Re: [PATCH v9 11/19] scsi: sd: Translate data lifetime information

[Bart Van Assche <bvanassche@acm.org>]

On 6/11/24 2:21 PM, Christian Heusel wrote:
> On 24/06/11 11:57PM, Andy Shevchenko wrote:
>> Tue, Jan 30, 2024 at 01:48:37PM -0800, Bart Van Assche kirjoitti:
>>> Recently T10 standardized SBC constrained streams. This mechanism allows
>>> to pass data lifetime information to SCSI devices in the group number
>>> field. Add support for translating write hint information into a
>>> permanent stream number in the sd driver. Use WRITE(10) instead of
>>> WRITE(6) if data lifetime information is present because the WRITE(6)
>>> command does not have a GROUP NUMBER field.
>>
>> This patch broke very badly my connected Garmin FR35 sport watch. The boot time
>> increased by 1 minute along with broken access to USB mass storage.
>>
>> On the reboot it takes ages as well.
>>
>> Revert of this and one little dependency (unrelated by functional means) helps.
> 
> We have tested that the revert fixes the issue on top of v6.10-rc3.
> 
> Also adding the regressions list in CC and making regzbot aware of this
> issue.
> 
>> Details are here: https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/issues/60
>>
>> P.S. Big thanks to Arch Linux team to help with bisection!
> 
> If this is fixed adding in a "Reported-by" or "Bisected-by" (depending
> on what this subsystem uses) for me would be appreciated :)

Thank you Christian for having gone through the painful process of
bisecting this issue.

Is the Garmin FR35 Flash device perhaps connected to a USB bus? If so,
this is the second report of a USB storage device that resets if it
receives a query for the IO Advice Hints Grouping mode page. Does the
patch below help?

Thanks,

Bart.


diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3a43e2209751..fcf3d7730466 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -63,6 +63,7 @@
  #include <scsi/scsi_cmnd.h>
  #include <scsi/scsi_dbg.h>
  #include <scsi/scsi_device.h>
+#include <scsi/scsi_devinfo.h>
  #include <scsi/scsi_driver.h>
  #include <scsi/scsi_eh.h>
  #include <scsi/scsi_host.h>
@@ -3117,6 +3118,9 @@ static void sd_read_io_hints(struct scsi_disk *sdkp, unsigned char *buffer)
  	struct scsi_mode_data data;
  	int res;

+	if (sdp->sdev_bflags & BLIST_SKIP_IO_HINTS)
+		return;
+
  	res = scsi_mode_sense(sdp, /*dbd=*/0x8, /*modepage=*/0x0a,
  			      /*subpage=*/0x05, buffer, SD_BUF_SIZE, SD_TIMEOUT,
  			      sdkp->max_retries, &data, &sshdr);
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index b31464740f6c..9a7185c68872 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -79,6 +79,8 @@ static int slave_alloc (struct scsi_device *sdev)
  	if (us->protocol == USB_PR_BULK && us->max_lun > 0)
  		sdev->sdev_bflags |= BLIST_FORCELUN;

+	sdev->sdev_bflags |= BLIST_SKIP_IO_HINTS;
+
  	return 0;
  }

diff --git a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
index 6b548dc2c496..fa8721e49dec 100644
--- a/include/scsi/scsi_devinfo.h
+++ b/include/scsi/scsi_devinfo.h
@@ -69,8 +69,10 @@
  #define BLIST_RETRY_ITF		((__force blist_flags_t)(1ULL << 32))
  /* Always retry ABORTED_COMMAND with ASC 0xc1 */
  #define BLIST_RETRY_ASC_C1	((__force blist_flags_t)(1ULL << 33))
+/* Do not read the I/O hints mode page */
+#define BLIST_SKIP_IO_HINTS	((__force blist_flags_t)(1ULL << 34))

-#define __BLIST_LAST_USED BLIST_RETRY_ASC_C1
+#define __BLIST_LAST_USED BLIST_SKIP_IO_HINTS

  #define __BLIST_HIGH_UNUSED (~(__BLIST_LAST_USED | \
  			       (__force blist_flags_t) \