linux-kernel - Re: [REGRESSION] critical target error, bisected

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <whnt3t5fqae5pujt6fkg5xu6mqxc2x5llbmq6q2lclfuuafbqx@tzj3dzgb7ury>
Date: Fri, 16 Aug 2024 07:59:52 +0000
From: Shinichiro Kawasaki <shinichiro.kawasaki@....com>
To: "Martin K. Petersen" <martin.petersen@...cle.com>
CC: Chris Bainbridge <chris.bainbridge@...il.com>, "fengli@...rtx.com"
	<fengli@...rtx.com>, hch <hch@....de>, "axboe@...nel.dk" <axboe@...nel.dk>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [REGRESSION] critical target error, bisected

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

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