[PATCH v3 0/2] scsi discard fix

[PATCH v3 0/2] scsi discard fix

[Li Feng]

Hi Jens,

These two patches have been reviewed but have not been merged into
linux-next. Can they be merged into 6.11?

Thanks,

v3:
- rebased to the latest linux-next branch;
- put the separate patch2 in this patchset;
- add reviewed-by tag.

v2:
- rewrite the patch.

Haoqian He (1):
  scsi: sd: remove some redundant initialization code

Li Feng (1):
  scsi: sd: Keep the discard mode stable

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

-- 
2.45.2

[PATCH v3 1/2] scsi: sd: Keep the discard mode stable

[Li Feng]

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, IO 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>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/sd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8bb3a3611851..c180427e2c98 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2711,8 +2711,6 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
 
 		if (buffer[14] & 0x40) /* LBPRZ */
 			sdkp->lbprz = 1;
-
-		sd_config_discard(sdkp, lim, SD_LBP_WS16);
 	}
 
 	sdkp->capacity = lba + 1;
@@ -3365,8 +3363,6 @@ static void sd_read_block_limits(struct scsi_disk *sdkp,
 			sdkp->unmap_alignment =
 				get_unaligned_be32(&vpd->data[32]) & ~(1 << 31);
 
-		sd_config_discard(sdkp, lim, sd_discard_mode(sdkp));
-
 config_atomic:
 		sdkp->max_atomic = get_unaligned_be32(&vpd->data[44]);
 		sdkp->atomic_alignment = get_unaligned_be32(&vpd->data[48]);
@@ -3756,6 +3752,8 @@ static int sd_revalidate_disk(struct gendisk *disk)
 			sd_read_cpr(sdkp);
 		}
 
+		sd_config_discard(sdkp, &lim, sd_discard_mode(sdkp));
+
 		sd_print_capacity(sdkp, old_capacity);
 
 		sd_read_write_protect_flag(sdkp, buffer);
-- 
2.45.2

[PATCH v3 2/2] scsi: sd: remove some redundant initialization code

[Li Feng]

From: Haoqian He <haoqian.he@smartx.com>

Since the memory allocated by kzalloc for sdkp has been
initialized to 0, the code that initializes some sdkp
fields to 0 is no longer needed.

Signed-off-by: Haoqian He <haoqian.he@smartx.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Li Feng <fengli@smartx.com>
---
 drivers/scsi/sd.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index c180427e2c98..3921b8fd71d1 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3953,7 +3953,6 @@ static int sd_probe(struct device *dev)
 	sdkp->disk = gd;
 	sdkp->index = index;
 	sdkp->max_retries = SD_MAX_RETRIES;
-	atomic_set(&sdkp->openers, 0);
 	atomic_set(&sdkp->device->ioerr_cnt, 0);
 
 	if (!sdp->request_queue->rq_timeout) {
@@ -3986,13 +3985,7 @@ static int sd_probe(struct device *dev)
 
 	/* defaults, until the device tells us otherwise */
 	sdp->sector_size = 512;
-	sdkp->capacity = 0;
 	sdkp->media_present = 1;
-	sdkp->write_prot = 0;
-	sdkp->cache_override = 0;
-	sdkp->WCE = 0;
-	sdkp->RCD = 0;
-	sdkp->ATO = 0;
 	sdkp->first_scan = 1;
 	sdkp->max_medium_access_timeouts = SD_MAX_MEDIUM_TIMEOUTS;
 
-- 
2.45.2

Re: [PATCH v3 0/2] scsi discard fix

[Jens Axboe]

On 7/18/24 2:07 AM, Li Feng wrote:
> Hi Jens,
> 
> These two patches have been reviewed but have not been merged into
> linux-next. Can they be merged into 6.11?

They can, but is there some dependency that means they should go
through the block tree? Would seem more logical that Martin picked
them up.

-- 
Jens Axboe

Re: [PATCH v3 0/2] scsi discard fix

[Martin K. Petersen]

Jens,

> They can, but is there some dependency that means they should go
> through the block tree? Would seem more logical that Martin picked
> them up.

I'll get to them. Busy with a couple of hardware-related regressions
right now.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v3 0/2] scsi discard fix

[Jens Axboe]

On 7/22/24 5:55 PM, Martin K. Petersen wrote:
> 
> Jens,
> 
>> They can, but is there some dependency that means they should go
>> through the block tree? Would seem more logical that Martin picked
>> them up.
> 
> I'll get to them. Busy with a couple of hardware-related regressions
> right now.

All good, just wanted to ensure they weren't waiting on me, as the
cover letter seemed to indicate.

Thanks!

-- 
Jens Axboe

Re: [PATCH v3 0/2] scsi discard fix

[Li Feng]

Hi Jens and Martin,

Thank you. 
I was worried that you were too busy and forgot about these two patches.

> 2024年7月23日 08:37，Jens Axboe <axboe@kernel.dk> 写道：
> 
> On 7/22/24 5:55 PM, Martin K. Petersen wrote:
>> 
>> Jens,
>> 
>>> They can, but is there some dependency that means they should go
>>> through the block tree? Would seem more logical that Martin picked
>>> them up.
>> 
>> I'll get to them. Busy with a couple of hardware-related regressions
>> right now.
> 
> All good, just wanted to ensure they weren't waiting on me, as the
> cover letter seemed to indicate.
> 
> Thanks!
> 
> -- 
> Jens Axboe
> 
> 

Re: [PATCH v3 0/2] scsi discard fix

[Martin K. Petersen]

On Thu, 18 Jul 2024 16:07:21 +0800, Li Feng wrote:

> These two patches have been reviewed but have not been merged into
> linux-next. Can they be merged into 6.11?
> 
> Thanks,
> 
> v3:
> - rebased to the latest linux-next branch;
> - put the separate patch2 in this patchset;
> - add reviewed-by tag.
> 
> [...]

Applied to 6.11/scsi-fixes, thanks!

[1/2] scsi: sd: Keep the discard mode stable
      https://git.kernel.org/mkp/scsi/c/f874d7210d88

-- 
Martin K. Petersen	Oracle Linux Engineering