[REGRESSION] critical target error, bisected

[REGRESSION] critical target error, bisected

[Chris Bainbridge]

Hello,

Machine is HP Aero 13 laptop. The following error appears in 6.11-rc3
when booted from USB drive:

[  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

This error appears on two different laptops with different USB drives.


Bisect result:

f874d7210d882cb1c58a8e3da66f61cdc63cd4b4 is the first bad commit
commit f874d7210d882cb1c58a8e3da66f61cdc63cd4b4
Author: Li Feng <fengli@smartx.com>
Date:   Thu Jul 18 16:07:22 2024 +0800

    scsi: sd: Keep the discard mode stable

    There is a scenario where a large number of discard commands are issued
    when the iscsi initiator connects to the target and then performs a session
    rescan operation. There is a time window, most of the commands are in UNMAP
    mode, and some discard commands become WRITE SAME with UNMAP.

    The discard mode has been negotiated during the SCSI probe. If the mode is
    temporarily changed from UNMAP to WRITE SAME with UNMAP, an I/O ERROR may
    occur because the target may not implement WRITE SAME with UNMAP. Keep the
    discard mode stable to fix this issue.

    Signed-off-by: Li Feng <fengli@smartx.com>
    Link: https://lore.kernel.org/r/20240718080751.313102-2-fengli@smartx.com
    Reviewed-by: Christoph Hellwig <hch@lst.de>
    Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
    Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

 drivers/scsi/sd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

#regzbot introduced: f874d7210d882cb1c58a8e3da66f61cdc63cd4b4

Re: [REGRESSION] critical target error, bisected

[Martin K. Petersen]

Chris,

> [  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
>
> This error appears on two different laptops with different USB drives.

Are these drives usb_storage or uas?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [REGRESSION] critical target error, bisected

[Martin K. Petersen]

Chris,

>> [  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
>>
>> This error appears on two different laptops with different USB drives.
>
> Are these drives usb_storage or uas?

Never mind, I see what's going on. Will fix...

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [REGRESSION] critical target error, bisected

[Martin K. Petersen]

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

I would appreciate if you could test the following patch.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

From dcbe0126551fedef94fd8334288e5b2bb6059475 Mon Sep 17 00:00:00 2001
From: "Martin K. Petersen" <martin.petersen@oracle.com>
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@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

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;
+
 	if (!sdkp->lbpvpd) {
 		/* LBP VPD page not provided */
 		if (sdkp->max_unmap_blocks)

Re: [REGRESSION] critical target error, bisected

[Chris Bainbridge]

On Fri, 16 Aug 2024 at 04:56, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
> I would appreciate if you could test the following patch.

Works for me.

Tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>

Re: [REGRESSION] critical target error, bisected

[Shinichiro Kawasaki]

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@oracle.com>
> 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@gmail.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> 
> 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@wdc.com>

> +
>  	if (!sdkp->lbpvpd) {
>  		/* LBP VPD page not provided */
>  		if (sdkp->max_unmap_blocks)

Re: [REGRESSION] critical target error, bisected

[hch]

On Fri, Aug 16, 2024 at 07:59:52AM +0000, Shinichiro Kawasaki wrote:
> 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!

Btw, zoned devices in all relevant standards can support discards,
despite it being rather pointless.  So we'll probably want to fix up
f2fs to handle that case correctly and not issue discards just to be on
the safe side.

Re: [REGRESSION] critical target error, bisected

[John Garry]

On 16/08/2024 04: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
> 
> I would appreciate if you could test the following patch.
> 
> Thanks!
> 

That solves the issue I was seeing on default scsi_debug, like this:

# mkfs.xfs -V
mkfs.xfs version 5.14.2
# mkfs.xfs /dev/sda
meta-data=/dev/sda               isize=512    agcount=4, agsize=22400 blks
          =                       sectsz=512   attr=2, projid32bit=1
          =                       crc=1        finobt=1, sparse=1, rmapbt=0
          =                       reflink=1    bigtime=0 inobtcount=0
data     =                       bsize=4096   blocks=89600, imaxpct=25
          =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0, ftype=1
log      =internal log           bsize=4096   blocks=1368, version=2
          =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
[   29.399767] sd 0:0:0:0: [sda] tag#166 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[   29.401463] sd 0:0:0:0: [sda] tag#166 Sense Key : Illegal Request 
[current]
[   29.403072] sd 0:0:0:0: [sda] tag#166 Add. Sense: Invalid field in cdb
[   29.404076] sd 0:0:0:0: [sda] tag#166 CDB: Write same(16) 93 08 00
00 00 00 00 00 00 00 00 00 ff ff 00 00
[   29.405344] critical target error, dev sda, sector 0 op
0x3:(DISCARD) flags 0x4800 phys_seg 1 prio class 0
[   29.408127] sd 0:0:0:0: [sda] tag#167 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[   29.410054] sd 0:0:0:0: [sda] tag#167 Sense Key : Illegal Request 
[current]
[   29.413442] sd 0:0:0:0: [sda] tag#167 Add. Sense: Invalid field in cdb
[   29.414552] sd 0:0:0:0: [sda] tag#167 CDB: Write same(16) 93 08 00
00 00 00 00 00 ff ff 00 00 ff ff 00 00
[   29.416241] critical target error, dev sda, sector 65535 op
0x3:(DISCARD) flags 0x4800 phys_seg 1 prio class 0
[   29.418022] sd 0:0:0:0: [sda] tag#168 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[   29.419604] sd 0:0:0:0: [sda] tag#168 Sense Key : Illegal Request 
[current]
[   29.420826] sd 0:0:0:0: [sda] tag#168 Add. Sense: Invalid field in cdb
[   29.421978] sd 0:0:0:0: [sda] tag#168 CDB: Write same(16) 93 08 00
00 00 00 00 01 ff fe 00 00 ff ff 00 00
[   29.423591] critical target error, dev sda, sector 131070 op
0x3:(DISCARD) flags 0x4800 phys_seg 1 prio class 0
[   29.425301] sd 0:0:0:0: [sda] tag#169 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[   29.426856] sd 0:0:0:0: [sda] tag#169 Sense Key : Illegal Request 
[current]
[   29.428020] sd 0:0:0:0: [sda] tag#169 Add. Sense: Invalid field in cdb
[   29.429329] sd 0:0:0:0: [sda] tag#169 CDB: Write same(16) 93 08 00
00 00 00 00 02 ff fd 00 00 ff ff 00 00
[   29.431314] critical target error, dev sda, sector 196605 op
0x3:(DISCARD) flags 0x4800 phys_seg 1 prio class 0
[   29.433681] sd 0:0:0:0: [sda] tag#170 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[   29.435182] sd 0:0:0:0: [sda] tag#170 Sense Key : Illegal Request 
[current]
[   29.436328] sd 0:0:0:0: [sda] tag#170 Add. Sense: Invalid field in cdb
[   29.437372] sd 0:0:0:0: [sda] tag#170 CDB: Write same(16) 93 08 00
00 00 00 00 03 ff fc 00 00 ff ff 00 00
[   29.438895] critical target error, dev sda, sector 262140 op
0x3:(DISCARD) flags 0x4800 phys_seg 1 prio class 0
[   29.440572] sd 0:0:0:0: [sda] tag#171 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[   29.442080] sd 0:0:0:0: [sda] tag#171 Sense Key : Illegal Request 
[current]
[   29.443277] sd 0:0:0:0: [sda] tag#171 Add. Sense: Invalid field in cdb
[   29.444353] sd 0:0:0:0: [sda] tag#171 CDB: Write same(16) 93 08 00
00 00 00 00 04 ff fb 00 00 ff ff 00 00
[   29.445919] critical target error, dev sda, sector 327675 op
0x3:(DISCARD) flags 0x4800 phys_seg 1 prio class 0
[   29.447607] sd 0:0:0:0: [sda] tag#172 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[   29.449145] sd 0:0:0:0: [sda] tag#172 Sense Key : Illegal Request 
[current]
[   29.450327] sd 0:0:0:0: [sda] tag#172 Add. Sense: Invalid field in cdb
[   29.451447] sd 0:0:0:0: [sda] tag#172 CDB: Write same(16) 93 08 00
00 00 00 00 05 ff fa 00 00 ff ff 00 00
[   29.452995] critical target error, dev sda, sector 393210 op
0x3:(DISCARD) flags 0x4800 phys_seg 1 prio class 0
[   29.454712] sd 0:0:0:0: [sda] tag#173 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[   29.456199] sd 0:0:0:0: [sda] tag#173 Sense Key : Illegal Request 
[current]
[   29.457409] sd 0:0:0:0: [sda] tag#173 Add. Sense: Invalid field in cdb
[   29.458486] sd 0:0:0:0: [sda] tag#173 CDB: Write same(16) 93 08 00
00 00 00 00 06 ff f9 00 00 ff ff 00 00
[   29.459836] critical target error, dev sda, sector 458745 op
0x3:(DISCARD) flags 0x4800 phys_seg 1 prio class 0
[   29.461266] sd 0:0:0:0: [sda] tag#174 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[   29.462608] sd 0:0:0:0: [sda] tag#174 Sense Key : Illegal Request 
[current]
[   29.463791] sd 0:0:0:0: [sda] tag#174 Add. Sense: Invalid field in cdb
[   29.464854] sd 0:0:0:0: [sda] tag#174 CDB: Write same(16) 93 08 00
00 00 00 00 07 ff f8 00 00 ff ff 00 00
[   29.466695] critical target error, dev sda, sector 524280 op
0x3:(DISCARD) flags 0x4800 phys_seg 1 prio class 0
[   29.468364] sd 0:0:0:0: [sda] tag#175 FAILED Result:
hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[   29.469774] sd 0:0:0:0: [sda] tag#175 Sense Key : Illegal Request 
[current]
[   29.470890] sd 0:0:0:0: [sda] tag#175 Add. Sense: Invalid field in cdb
[   29.471867] sd 0:0:0:0: [sda] tag#175 CDB: Write same(16) 93 08 00
00 00 00 00 08 ff f7 00 00 ff ff 00 00
[   29.473323] critical target error, dev sda, sector 589815 op
0x3:(DISCARD) flags 0x4800 phys_seg 1 prio class 0
#
#

Re: [REGRESSION] critical target error, bisected

[Martin K. Petersen]

Shinichiro,

> 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".

> I confirmed that the fix patch avoids my failure both with
> SD_LBP_DISABLE and SD_LBP_FULL.

Yes, you are right. It is more appropriate to print "full" in sysfs when
LBMPE is disabled. Printing aside, the two modes are identical. I
switched the return to SDP_LBP_FULL.

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH] scsi: sd: Do not attempt to configure discard unless LBPME is set

[Martin K. Petersen]

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 enabled.

Fixes: f874d7210d88 ("scsi: sd: Keep the discard mode stable")
Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
Tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>
Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Tested-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/sd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 699f4f9674d9..dad3991397cf 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_FULL;
+
 	if (!sdkp->lbpvpd) {
 		/* LBP VPD page not provided */
 		if (sdkp->max_unmap_blocks)
-- 
2.45.2

Re: [PATCH] scsi: sd: Do not attempt to configure discard unless LBPME is set

[Martin K. Petersen]

On Fri, 16 Aug 2024 20:53:10 -0400, Martin K. Petersen wrote:

> 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 enabled.
> 
> [...]

Applied to 6.11/scsi-fixes, thanks!

[1/1] scsi: sd: Do not attempt to configure discard unless LBPME is set
      https://git.kernel.org/mkp/scsi/c/cbaac68987b8

-- 
Martin K. Petersen	Oracle Linux Engineering