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)