Re: [RFC PATCH] scsi: support packing multi-segment in UNMAP command

[Chao Yu <chao@kernel.org>]

Ping,

Any comments?

Thanks,

On 2022/5/21 19:32, Chao Yu wrote:
> As SPEC 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 in-batch UNMAP.
> 
> This patch tries to set max_discard_segments config according to block
> limits of device, and supports in-batch UNMAP.
> 
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>   drivers/scsi/sd.c | 30 +++++++++++++++++++-----------
>   drivers/scsi/sd.h |  1 +
>   2 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index dc6e55761fd1..be070457c78d 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -790,6 +790,7 @@ 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, sdkp->max_block_desc_count);
>   	sdkp->provisioning_mode = mode;
>   
>   	switch (mode) {
> @@ -837,10 +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 bio *bio;
>   	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, i = 0;
>   	char *buf;
>   
>   	rq->special_vec.bv_page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
> @@ -853,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[8 + 16 * i]);
> +		put_unaligned_be32(nr_blocks, &buf[8 + 16 * i + 8]);
> +		i++;
> +	}
>   
>   	cmd->allowed = sdkp->max_retries;
>   	cmd->transfersize = data_len;
> @@ -2859,7 +2867,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
>   	sdkp->opt_xfer_blocks = get_unaligned_be32(&buffer[12]);
>   
>   	if (buffer[3] == 0x3c) {
> -		unsigned int lba_count, desc_count;
> +		unsigned int lba_count;
>   
>   		sdkp->max_ws_blocks = (u32)get_unaligned_be64(&buffer[36]);
>   
> @@ -2867,9 +2875,9 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
>   			goto out;
>   
>   		lba_count = get_unaligned_be32(&buffer[20]);
> -		desc_count = get_unaligned_be32(&buffer[24]);
> +		sdkp->max_block_desc_count = get_unaligned_be32(&buffer[24]);
>   
> -		if (lba_count && desc_count)
> +		if (lba_count && sdkp->max_block_desc_count)
>   			sdkp->max_unmap_blocks = lba_count;
>   
>   		sdkp->unmap_granularity = get_unaligned_be32(&buffer[28]);
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 0a33a4b68ffb..e396bcaf76ca 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -99,6 +99,7 @@ struct scsi_disk {
>   	u32		opt_xfer_blocks;
>   	u32		max_ws_blocks;
>   	u32		max_unmap_blocks;
> +	u32		max_block_desc_count;
>   	u32		unmap_granularity;
>   	u32		unmap_alignment;
>   	u32		index;