On Aug 15, 2024 / 23:56, Martin K. Petersen wrote:
>
> Chris,
>
> > [ 195.647081] sd 0:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
> > [ 195.647093] sd 0:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current]
> > [ 195.647096] sd 0:0:0:0: [sda] tag#0 Add. Sense: Invalid command operation code
> > [ 195.647099] sd 0:0:0:0: [sda] tag#0 CDB: Write same(16) 93 08 00 00 00 00 04 dd 42 f8 00 00 2d 48 00 00
> > [ 195.647101] critical target error, dev sda, sector 81609464 op 0x3:(DISCARD) flags 0x800 phys_seg 1 prio class 0
Hello Chris, Martin,
I also observed a different but similar failure symptom when I ran f2fs workload
using a zoned TCMU device. I found that the trigger is the commit f874d7210d88
("scsi: sd: Keep the discard mode stable") that Chris found. After the commit,
the device has unexpected non-zero values for the sysfs attributes
queue/discard_max_bytes and queue/discard_max_hw_bytes. I found that Martin's
fix avoids my failure symptom also. With the fix, the failure disappeared and
the sysfs attribute have the expected value 0. Thanks!
> From dcbe0126551fedef94fd8334288e5b2bb6059475 Mon Sep 17 00:00:00 2001
> From: "Martin K. Petersen" <martin.petersen@xxxxxxxxxx>
> Date: Tue, 13 Aug 2024 03:58:27 -0400
> Subject: [PATCH] scsi: sd: Do not attempt to configure discard unless LBPME is
> set
>
> Commit f874d7210d88 ("scsi: sd: Keep the discard mode stable")
> attempted to address an issue where one mode of discard operation got
> configured prior to the device completing full discovery.
> Unfortunately this change assumed discard was always enabled on the
> device.
>
> Do not attempt to configure discard unless LBPME is set.
>
> Fixes: f874d7210d88 ("scsi: sd: Keep the discard mode stable")
> Reported-by: Chris Bainbridge <chris.bainbridge@xxxxxxxxx>
> Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 699f4f9674d9..966fc717d235 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3308,6 +3308,9 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer)
>
> static unsigned int sd_discard_mode(struct scsi_disk *sdkp)
> {
> + if (!sdkp->lbpme)
> + return SD_LBP_DISABLE;
I guess SD_LBP_FULL would be more appropriate than SD_LBP_DISABLE, because the
comment in drivers/scsi/sd.h says that SD_LBP_DISABLE indicates that "Discard
disabled due to failed cmd".
$ git grep SD_LBP_DISABLE
drivers/scsi/sd.c: [SD_LBP_DISABLE] = "disabled",
drivers/scsi/sd.c: sdkp->provisioning_mode = SD_LBP_DISABLE;
drivers/scsi/sd.c: case SD_LBP_DISABLE:
drivers/scsi/sd.c: return SD_LBP_DISABLE;
drivers/scsi/sd.h: SD_LBP_DISABLE, /* Discard disabled due to failed cmd */
I confirmed that the fix patch avoids my failure both with SD_LBP_DISABLE
and SD_LBP_FULL.
Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> +
> if (!sdkp->lbpvpd) {
> /* LBP VPD page not provided */
> if (sdkp->max_unmap_blocks)
