linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] scsi: support packing multi-segment in UNMAP command
@ 2022-06-16  1:36 Chao Yu
  2022-06-16  6:09 ` Christoph Hellwig
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Chao Yu @ 2022-06-16  1:36 UTC (permalink / raw)
  To: jejb, martin.petersen; +Cc: linux-scsi, linux-kernel, chao, hch, bvanassche

As SCSI SBC4 specification section 5.30.2 describes that it can
support unmapping one or more LBA range in single UNMAP command.

However, previously we only pack one LBA range in UNMAP command
by default no matter device gives the block limits that says it
can support unmapping multiple LBA ranges with a single UNMAP
command.

This patch sets max_discard_segments config according to block
limits of device, and supports unmapping multiple LBA ranges with
a single UNMAP command.

Signed-off-by: Chao Yu <chao@kernel.org>
---
v4:
- clean up commit message.
- fix to avoid truncating .max_unmap_block_desc_count during type cast.
- add comments and clean up codes.

 drivers/scsi/sd.c | 35 ++++++++++++++++++++++++-----------
 drivers/scsi/sd.h |  1 +
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 895b56c8f25e..114f61c3ccd3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -790,6 +790,8 @@ static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode)
 	q->limits.discard_granularity =
 		max(sdkp->physical_block_size,
 		    sdkp->unmap_granularity * logical_block_size);
+	blk_queue_max_discard_segments(q, min_t(u32, U16_MAX,
+				sdkp->max_unmap_block_desc_count));
 	sdkp->provisioning_mode = mode;
 
 	switch (mode) {
@@ -836,9 +838,10 @@ static blk_status_t sd_setup_unmap_cmnd(struct scsi_cmnd *cmd)
 	struct scsi_device *sdp = cmd->device;
 	struct request *rq = scsi_cmd_to_rq(cmd);
 	struct scsi_disk *sdkp = scsi_disk(rq->q->disk);
-	u64 lba = sectors_to_logical(sdp, blk_rq_pos(rq));
-	u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
-	unsigned int data_len = 24;
+	unsigned short segments = blk_rq_nr_discard_segments(rq);
+	unsigned int data_len = 8 + 16 * segments;
+	unsigned int descriptor_offset = 8;
+	struct bio *bio;
 	char *buf;
 
 	rq->special_vec.bv_page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
@@ -851,13 +854,20 @@ static blk_status_t sd_setup_unmap_cmnd(struct scsi_cmnd *cmd)
 
 	cmd->cmd_len = 10;
 	cmd->cmnd[0] = UNMAP;
-	cmd->cmnd[8] = 24;
+	cmd->cmnd[8] = data_len;
 
 	buf = bvec_virt(&rq->special_vec);
-	put_unaligned_be16(6 + 16, &buf[0]);
-	put_unaligned_be16(16, &buf[2]);
-	put_unaligned_be64(lba, &buf[8]);
-	put_unaligned_be32(nr_blocks, &buf[16]);
+	put_unaligned_be16(6 + 16 * segments, &buf[0]);
+	put_unaligned_be16(16 * segments, &buf[2]);
+
+	__rq_for_each_bio(bio, rq) {
+		u64 lba = sectors_to_logical(sdp, bio->bi_iter.bi_sector);
+		u32 nr_blocks = sectors_to_logical(sdp, bio_sectors(bio));
+
+		put_unaligned_be64(lba, &buf[descriptor_offset]);
+		put_unaligned_be32(nr_blocks, &buf[descriptor_offset + 8]);
+		descriptor_offset += 16;
+	}
 
 	cmd->allowed = sdkp->max_retries;
 	cmd->transfersize = data_len;
@@ -2862,7 +2872,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
 	sdkp->opt_xfer_blocks = get_unaligned_be32(&vpd->data[12]);
 
 	if (vpd->len >= 64) {
-		unsigned int lba_count, desc_count;
+		unsigned int lba_count;
 
 		sdkp->max_ws_blocks = (u32)get_unaligned_be64(&vpd->data[36]);
 
@@ -2870,9 +2880,12 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
 			goto out;
 
 		lba_count = get_unaligned_be32(&vpd->data[20]);
-		desc_count = get_unaligned_be32(&vpd->data[24]);
 
-		if (lba_count && desc_count)
+		/* Extract the MAXIMUM UNMAP BLOCK DESCRIPTOR COUNT. */
+		sdkp->max_unmap_block_desc_count =
+					get_unaligned_be32(&vpd->data[24]);
+
+		if (lba_count && sdkp->max_unmap_block_desc_count)
 			sdkp->max_unmap_blocks = lba_count;
 
 		sdkp->unmap_granularity = get_unaligned_be32(&vpd->data[28]);
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 5eea762f84d1..e7c51d23395b 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -119,6 +119,7 @@ struct scsi_disk {
 	u32		opt_xfer_blocks;
 	u32		max_ws_blocks;
 	u32		max_unmap_blocks;
+	u32		max_unmap_block_desc_count;
 	u32		unmap_granularity;
 	u32		unmap_alignment;
 	u32		index;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] scsi: support packing multi-segment in UNMAP command
  2022-06-16  1:36 [PATCH v4] scsi: support packing multi-segment in UNMAP command Chao Yu
@ 2022-06-16  6:09 ` Christoph Hellwig
  2022-06-17  2:01 ` Martin K. Petersen
  2022-06-17  3:50 ` Bart Van Assche
  2 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2022-06-16  6:09 UTC (permalink / raw)
  To: Chao Yu; +Cc: jejb, martin.petersen, linux-scsi, linux-kernel, hch, bvanassche

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] scsi: support packing multi-segment in UNMAP command
  2022-06-16  1:36 [PATCH v4] scsi: support packing multi-segment in UNMAP command Chao Yu
  2022-06-16  6:09 ` Christoph Hellwig
@ 2022-06-17  2:01 ` Martin K. Petersen
  2022-08-11 19:34   ` Bart Van Assche
  2022-06-17  3:50 ` Bart Van Assche
  2 siblings, 1 reply; 7+ messages in thread
From: Martin K. Petersen @ 2022-06-17  2:01 UTC (permalink / raw)
  To: Chao Yu; +Cc: jejb, martin.petersen, linux-scsi, linux-kernel, hch, bvanassche


Chao,

> As SCSI SBC4 specification section 5.30.2 describes that it can
> support unmapping one or more LBA range in single UNMAP command.
>
> However, previously we only pack one LBA range in UNMAP command by
> default no matter device gives the block limits that says it can
> support unmapping multiple LBA ranges with a single UNMAP command.
>
> This patch sets max_discard_segments config according to block limits
> of device, and supports unmapping multiple LBA ranges with a single
> UNMAP command.

This looks OK to me. Will test with a variety of targets.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] scsi: support packing multi-segment in UNMAP command
  2022-06-16  1:36 [PATCH v4] scsi: support packing multi-segment in UNMAP command Chao Yu
  2022-06-16  6:09 ` Christoph Hellwig
  2022-06-17  2:01 ` Martin K. Petersen
@ 2022-06-17  3:50 ` Bart Van Assche
  2 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2022-06-17  3:50 UTC (permalink / raw)
  To: Chao Yu, jejb, martin.petersen; +Cc: linux-scsi, linux-kernel, hch

On 6/15/22 18:36, Chao Yu wrote:
> As SCSI SBC4 specification section 5.30.2 describes that it can
> support unmapping one or more LBA range in single UNMAP command.
> 
> However, previously we only pack one LBA range in UNMAP command
> by default no matter device gives the block limits that says it
> can support unmapping multiple LBA ranges with a single UNMAP
> command.
> 
> This patch sets max_discard_segments config according to block
> limits of device, and supports unmapping multiple LBA ranges with
> a single UNMAP command.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] scsi: support packing multi-segment in UNMAP command
  2022-06-17  2:01 ` Martin K. Petersen
@ 2022-08-11 19:34   ` Bart Van Assche
  2022-08-12  1:53     ` Martin K. Petersen
  0 siblings, 1 reply; 7+ messages in thread
From: Bart Van Assche @ 2022-08-11 19:34 UTC (permalink / raw)
  To: Martin K. Petersen, Chao Yu; +Cc: jejb, linux-scsi, linux-kernel, hch

On 6/16/22 19:01, Martin K. Petersen wrote:
>> As SCSI SBC4 specification section 5.30.2 describes that it can
>> support unmapping one or more LBA range in single UNMAP command.
>>
>> However, previously we only pack one LBA range in UNMAP command by
>> default no matter device gives the block limits that says it can
>> support unmapping multiple LBA ranges with a single UNMAP command.
>>
>> This patch sets max_discard_segments config according to block limits
>> of device, and supports unmapping multiple LBA ranges with a single
>> UNMAP command.
> 
> This looks OK to me. Will test with a variety of targets.

Hi Martin,

Have you already had the chance to test this patch? We would like to use 
this functionality in Android.

Thanks,

Bart.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] scsi: support packing multi-segment in UNMAP command
  2022-08-11 19:34   ` Bart Van Assche
@ 2022-08-12  1:53     ` Martin K. Petersen
  2022-08-12  5:59       ` Chao Yu
  0 siblings, 1 reply; 7+ messages in thread
From: Martin K. Petersen @ 2022-08-12  1:53 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K. Petersen, Chao Yu, jejb, linux-scsi, linux-kernel, hch


Bart,

> Have you already had the chance to test this patch? We would like to
> use this functionality in Android.

https://git.kernel.org/pub/scm/linux/kernel/git/mkp/linux.git/log/?h=5.20/discovery

Had to drop the series from 5.20/6.0 due to a couple of reported
regressions. Will try again for 6.1.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] scsi: support packing multi-segment in UNMAP command
  2022-08-12  1:53     ` Martin K. Petersen
@ 2022-08-12  5:59       ` Chao Yu
  0 siblings, 0 replies; 7+ messages in thread
From: Chao Yu @ 2022-08-12  5:59 UTC (permalink / raw)
  To: Martin K. Petersen, Bart Van Assche; +Cc: jejb, linux-scsi, linux-kernel, hch

Hi Martin,

On 2022/8/12 9:53, Martin K. Petersen wrote:
> 
> Bart,
> 
>> Have you already had the chance to test this patch? We would like to
>> use this functionality in Android.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/mkp/linux.git/log/?h=5.20/discovery
> 
> Had to drop the series from 5.20/6.0 due to a couple of reported
> regressions. Will try again for 6.1.

Could you please provider details of regression reports, let me check them
as well.

Thanks,

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-08-12  6:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-16  1:36 [PATCH v4] scsi: support packing multi-segment in UNMAP command Chao Yu
2022-06-16  6:09 ` Christoph Hellwig
2022-06-17  2:01 ` Martin K. Petersen
2022-08-11 19:34   ` Bart Van Assche
2022-08-12  1:53     ` Martin K. Petersen
2022-08-12  5:59       ` Chao Yu
2022-06-17  3:50 ` Bart Van Assche

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).