[v1 0/2] enable sector size > PAGE_SIZE for scsi

[v1 0/2] enable sector size > PAGE_SIZE for scsi

[sw.prabhu6]

From: Swarna Prabhu <sw.prabhu6@gmail.com>

Hi All,

This is non RFC v1 series sent based on the feedback received on RFC
v2 [1] and RFC v1 [2]. This patchset enables sector sizes > PAGE_SIZE for
sd driver and scsi_debug driver since block layer can support block
size > PAGE_SIZE. There was one issue with write_same16 and write_same10
command, which is fixed as a part of the series.

Motivation:
 - While enabling LBS on ZoneFS, zonefs-tools tests were being skipped
   for conventional zones when tested on nvme block device emulated using
   QEMU. Hence there was a need to enable scsi with higher sector sizes
   to run zonefs tests for conventional zones as well.

Changes since RFC v2:
 - Replaced open coded check for enabling sdebug_sector_size > PAGE_SIZE
   with blk_validate_block_size() in scsi_debug.
 - Replaced open coded check for enabling sector_size > PAGE_SIZE
   with blk_validate_block_size() in sd driver.
 - Replaced 'struct request *rq' argument in  'sd_set_special_bvec()' to
   'struct scsi_cmnd *cmnd' and used that to get SCSI device pointer
   'struct scsi_device *sdp'.
 - Slightly modified the commit title for scsi sd driver fix patch.
 - Added "Cc: stable@vger.kernel.org" tag to scsi sd driver fix patch.

Changes since RFC v1:
 - Re organized the patch series into one patch for scsi_debug driver
   and one patch for sd driver.
 - Updated commit title and description to accommodate the above
   re organization on commit titled - scsi: sd: fix write_same(16/10)
   to enable sector size > PAGE_SIZE.
 - Updated commit title and description to reflect the re organization
   on commit titled - scsi: scsi_debug: enable sdebug_sector_size
   > PAGE_SIZE.

Thanks to Bart for feedback on the RFC v1 and v2 series.

Testing:
 -Test suite: xfs and generic from fstest + QEMU emulated block
    device(scsi and nvme)
  - fstest Config for patched xfs 16k block size [xfs_reflink_16k_scsi]
    TEST_DEV=/dev/sda
    SCRATCH_DEV_POOL="/dev/sdb"
    MKFS_OPTIONS='-f -m reflink=1,rmapbt=1, -i sparse=1, -b size=16384,
    -s size=16384'
  - Generic test results
    Baseline: 6.18.0-rc7 kernel + nvme 16k logical block size
    Patched: 6.18 kernel + scsi 16k logical block size
    5 failures seen on generic tests and the same seen on baseline
    No regressions introduced by the patch.
  - XFS tests results
    Baseline: 6.18.0-rc7 kernel + nvme16k logical block size
    Patched: 6.18 kernel + sci 16k logical block size
    30 failures seen on patched and baseline
    No regressions introduced by the patch
  - Blktests results
    scis and block layer tests with 16k and 32k logical block size.
    config used:
    TEST_DEVS=(/dev/sda)
    EXCLUDE=(block/010 block/011) # these didn't run on baseline(nvme 16k)
    All tests passed.

Link to RFC v2: https://lore.kernel.org/all/20251203230546.1275683-2-sw.prabhu6@gmail.com/ [1]
Link to RFC v1: https://lore.kernel.org/all/20251202021522.188419-1-sw.prabhu6@gmail.com/ [2]

Swarna Prabhu (2):
  scsi: sd: fix write_same(16/10) to enable sector size > PAGE_SIZE
  scsi: scsi_debug: enable sdebug_sector_size > PAGE_SIZE

 drivers/scsi/scsi_debug.c |  8 +-------
 drivers/scsi/sd.c         | 27 +++++++++++++++++----------
 2 files changed, 18 insertions(+), 17 deletions(-)

-- 
2.51.0

[PATCH 1/2] scsi: sd: fix write_same(16/10) to enable sector size > PAGE_SIZE

[sw.prabhu6]

From: Swarna Prabhu <sw.prabhu6@gmail.com>

The WRITE SAME(16) and WRITE SAME(10) scsi commands uses
a page from a dedicated mempool('sd_page_pool') for its
payload. This pool was initialized to allocate single
pages, which was sufficient as long as the device sector
size did not exceed the PAGE_SIZE.

Given that block layer now supports block size upto
64K ie beyond PAGE_SIZE, adapt sd_set_special_bvec()
to accommodate that.

With the above fix, enable sector sizes > PAGE_SIZE in
scsi sd driver.

Cc: stable@vger.kernel.org
Signed-off-by: Swarna Prabhu <s.prabhu@samsung.com>
Co-developed-by: Pankaj Raghav <p.raghav@samsung.com>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
Note: We are allocating pages of order aligned to 
BLK_MAX_BLOCK_SIZE for the mempool page allocator
'sd_page_pool' all the time. This is because we only
know that a bigger sector size device is attached at
sd_probe and it might be too late to reallocate mempool
with order >0.

 drivers/scsi/sd.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 0252d3f6bed1..17b5c1589eb2 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -892,14 +892,24 @@ static void sd_config_discard(struct scsi_disk *sdkp, struct queue_limits *lim,
 		(logical_block_size >> SECTOR_SHIFT);
 }
 
-static void *sd_set_special_bvec(struct request *rq, unsigned int data_len)
+static void *sd_set_special_bvec(struct scsi_cmnd *cmd, unsigned int data_len)
 {
 	struct page *page;
+	struct request *rq = scsi_cmd_to_rq(cmd);
+	struct scsi_device *sdp = cmd->device;
+	unsigned sector_size = sdp->sector_size;
+	unsigned int nr_pages = DIV_ROUND_UP(sector_size, PAGE_SIZE);
+	int n = 0;
 
 	page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
 	if (!page)
 		return NULL;
-	clear_highpage(page);
+
+	do {
+		clear_highpage(page + n);
+		n++;
+	} while (n < nr_pages);
+
 	bvec_set_page(&rq->special_vec, page, data_len, 0);
 	rq->rq_flags |= RQF_SPECIAL_PAYLOAD;
 	return bvec_virt(&rq->special_vec);
@@ -915,7 +925,7 @@ static blk_status_t sd_setup_unmap_cmnd(struct scsi_cmnd *cmd)
 	unsigned int data_len = 24;
 	char *buf;
 
-	buf = sd_set_special_bvec(rq, data_len);
+	buf = sd_set_special_bvec(cmd, data_len);
 	if (!buf)
 		return BLK_STS_RESOURCE;
 
@@ -1004,7 +1014,7 @@ static blk_status_t sd_setup_write_same16_cmnd(struct scsi_cmnd *cmd,
 	u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
 	u32 data_len = sdp->sector_size;
 
-	if (!sd_set_special_bvec(rq, data_len))
+	if (!sd_set_special_bvec(cmd, data_len))
 		return BLK_STS_RESOURCE;
 
 	cmd->cmd_len = 16;
@@ -1031,7 +1041,7 @@ static blk_status_t sd_setup_write_same10_cmnd(struct scsi_cmnd *cmd,
 	u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
 	u32 data_len = sdp->sector_size;
 
-	if (!sd_set_special_bvec(rq, data_len))
+	if (!sd_set_special_bvec(cmd, data_len))
 		return BLK_STS_RESOURCE;
 
 	cmd->cmd_len = 10;
@@ -2880,10 +2890,7 @@ sd_read_capacity(struct scsi_disk *sdkp, struct queue_limits *lim,
 			  "assuming 512.\n");
 	}
 
-	if (sector_size != 512 &&
-	    sector_size != 1024 &&
-	    sector_size != 2048 &&
-	    sector_size != 4096) {
+	if (blk_validate_block_size(sector_size)) {
 		sd_printk(KERN_NOTICE, sdkp, "Unsupported sector size %d.\n",
 			  sector_size);
 		/*
@@ -4368,7 +4375,7 @@ static int __init init_sd(void)
 	if (err)
 		goto err_out;
 
-	sd_page_pool = mempool_create_page_pool(SD_MEMPOOL_SIZE, 0);
+	sd_page_pool = mempool_create_page_pool(SD_MEMPOOL_SIZE, get_order(BLK_MAX_BLOCK_SIZE));
 	if (!sd_page_pool) {
 		printk(KERN_ERR "sd: can't init discard page pool\n");
 		err = -ENOMEM;
-- 
2.51.0

[PATCH 2/2] scsi: scsi_debug: enable sdebug_sector_size > PAGE_SIZE

[sw.prabhu6]

From: Swarna Prabhu <sw.prabhu6@gmail.com>

Now that block layer can support block size > PAGE_SIZE
and the issue with WRITE_SAME(16) and WRITE_SAME(10) are
fixed for sector sizes > PAGE_SIZE, enable sdebug_sector_size
> PAGE_SIZE in scsi_debug.

Signed-off-by: Swarna Prabhu <s.prabhu@samsung.com>
---
 drivers/scsi/scsi_debug.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index b2ab97be5db3..2348976b3f70 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -8459,13 +8459,7 @@ static int __init scsi_debug_init(void)
 	} else if (sdebug_ndelay > 0)
 		sdebug_jdelay = JDELAY_OVERRIDDEN;
 
-	switch (sdebug_sector_size) {
-	case  512:
-	case 1024:
-	case 2048:
-	case 4096:
-		break;
-	default:
+	if (blk_validate_block_size(sdebug_sector_size)) {
 		pr_err("invalid sector_size %d\n", sdebug_sector_size);
 		return -EINVAL;
 	}
-- 
2.51.0

Re: [PATCH 1/2] scsi: sd: fix write_same(16/10) to enable sector size > PAGE_SIZE

[Damien Le Moal]

On 2025/12/09 17:41, sw.prabhu6@gmail.com wrote:
> From: Swarna Prabhu <sw.prabhu6@gmail.com>
> 
> The WRITE SAME(16) and WRITE SAME(10) scsi commands uses
> a page from a dedicated mempool('sd_page_pool') for its
> payload. This pool was initialized to allocate single
> pages, which was sufficient as long as the device sector
> size did not exceed the PAGE_SIZE.
> 
> Given that block layer now supports block size upto
> 64K ie beyond PAGE_SIZE, adapt sd_set_special_bvec()
> to accommodate that.
> 
> With the above fix, enable sector sizes > PAGE_SIZE in
> scsi sd driver.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Swarna Prabhu <s.prabhu@samsung.com>
> Co-developed-by: Pankaj Raghav <p.raghav@samsung.com>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
> Note: We are allocating pages of order aligned to 
> BLK_MAX_BLOCK_SIZE for the mempool page allocator
> 'sd_page_pool' all the time. This is because we only
> know that a bigger sector size device is attached at
> sd_probe and it might be too late to reallocate mempool
> with order >0.

That is a lot heavier on the memory for the vast majority of devices which are
512B or 4K block size... It may be better to have the special "large block"
mempool attached to the scsi disk struct and keep the default single page
mempool for all other regular devices.

> 
>  drivers/scsi/sd.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 0252d3f6bed1..17b5c1589eb2 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -892,14 +892,24 @@ static void sd_config_discard(struct scsi_disk *sdkp, struct queue_limits *lim,
>  		(logical_block_size >> SECTOR_SHIFT);
>  }
>  
> -static void *sd_set_special_bvec(struct request *rq, unsigned int data_len)
> +static void *sd_set_special_bvec(struct scsi_cmnd *cmd, unsigned int data_len)
>  {
>  	struct page *page;
> +	struct request *rq = scsi_cmd_to_rq(cmd);
> +	struct scsi_device *sdp = cmd->device;
> +	unsigned sector_size = sdp->sector_size;
> +	unsigned int nr_pages = DIV_ROUND_UP(sector_size, PAGE_SIZE);
> +	int n = 0;
>  
>  	page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
>  	if (!page)
>  		return NULL;
> -	clear_highpage(page);
> +
> +	do {
> +		clear_highpage(page + n);
> +		n++;
> +	} while (n < nr_pages);
> +
>  	bvec_set_page(&rq->special_vec, page, data_len, 0);
>  	rq->rq_flags |= RQF_SPECIAL_PAYLOAD;
>  	return bvec_virt(&rq->special_vec);
> @@ -915,7 +925,7 @@ static blk_status_t sd_setup_unmap_cmnd(struct scsi_cmnd *cmd)
>  	unsigned int data_len = 24;
>  	char *buf;
>  
> -	buf = sd_set_special_bvec(rq, data_len);
> +	buf = sd_set_special_bvec(cmd, data_len);
>  	if (!buf)
>  		return BLK_STS_RESOURCE;
>  
> @@ -1004,7 +1014,7 @@ static blk_status_t sd_setup_write_same16_cmnd(struct scsi_cmnd *cmd,
>  	u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
>  	u32 data_len = sdp->sector_size;
>  
> -	if (!sd_set_special_bvec(rq, data_len))
> +	if (!sd_set_special_bvec(cmd, data_len))
>  		return BLK_STS_RESOURCE;
>  
>  	cmd->cmd_len = 16;
> @@ -1031,7 +1041,7 @@ static blk_status_t sd_setup_write_same10_cmnd(struct scsi_cmnd *cmd,
>  	u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
>  	u32 data_len = sdp->sector_size;
>  
> -	if (!sd_set_special_bvec(rq, data_len))
> +	if (!sd_set_special_bvec(cmd, data_len))
>  		return BLK_STS_RESOURCE;
>  
>  	cmd->cmd_len = 10;
> @@ -2880,10 +2890,7 @@ sd_read_capacity(struct scsi_disk *sdkp, struct queue_limits *lim,
>  			  "assuming 512.\n");
>  	}
>  
> -	if (sector_size != 512 &&
> -	    sector_size != 1024 &&
> -	    sector_size != 2048 &&
> -	    sector_size != 4096) {
> +	if (blk_validate_block_size(sector_size)) {
>  		sd_printk(KERN_NOTICE, sdkp, "Unsupported sector size %d.\n",
>  			  sector_size);
>  		/*
> @@ -4368,7 +4375,7 @@ static int __init init_sd(void)
>  	if (err)
>  		goto err_out;
>  
> -	sd_page_pool = mempool_create_page_pool(SD_MEMPOOL_SIZE, 0);
> +	sd_page_pool = mempool_create_page_pool(SD_MEMPOOL_SIZE, get_order(BLK_MAX_BLOCK_SIZE));
>  	if (!sd_page_pool) {
>  		printk(KERN_ERR "sd: can't init discard page pool\n");
>  		err = -ENOMEM;


-- 
Damien Le Moal
Western Digital Research

Re: [v1 0/2] enable sector size > PAGE_SIZE for scsi

[Damien Le Moal]

On 2025/12/09 17:41, sw.prabhu6@gmail.com wrote:
> From: Swarna Prabhu <sw.prabhu6@gmail.com>
> 
> Hi All,
> 
> This is non RFC v1 series sent based on the feedback received on RFC
> v2 [1] and RFC v1 [2]. This patchset enables sector sizes > PAGE_SIZE for
> sd driver and scsi_debug driver since block layer can support block
> size > PAGE_SIZE. There was one issue with write_same16 and write_same10
> command, which is fixed as a part of the series.
> 
> Motivation:
>  - While enabling LBS on ZoneFS, zonefs-tools tests were being skipped
>    for conventional zones when tested on nvme block device emulated using
>    QEMU. Hence there was a need to enable scsi with higher sector sizes
>    to run zonefs tests for conventional zones as well.

This is super confusing: there are no conventional zones with NVMe. And why
would a problem with NVMe require scsi patches ?

> Changes since RFC v2:
>  - Replaced open coded check for enabling sdebug_sector_size > PAGE_SIZE
>    with blk_validate_block_size() in scsi_debug.
>  - Replaced open coded check for enabling sector_size > PAGE_SIZE
>    with blk_validate_block_size() in sd driver.
>  - Replaced 'struct request *rq' argument in  'sd_set_special_bvec()' to
>    'struct scsi_cmnd *cmnd' and used that to get SCSI device pointer
>    'struct scsi_device *sdp'.
>  - Slightly modified the commit title for scsi sd driver fix patch.
>  - Added "Cc: stable@vger.kernel.org" tag to scsi sd driver fix patch.
> 
> Changes since RFC v1:
>  - Re organized the patch series into one patch for scsi_debug driver
>    and one patch for sd driver.
>  - Updated commit title and description to accommodate the above
>    re organization on commit titled - scsi: sd: fix write_same(16/10)
>    to enable sector size > PAGE_SIZE.
>  - Updated commit title and description to reflect the re organization
>    on commit titled - scsi: scsi_debug: enable sdebug_sector_size
>    > PAGE_SIZE.
> 
> Thanks to Bart for feedback on the RFC v1 and v2 series.
> 
> Testing:
>  -Test suite: xfs and generic from fstest + QEMU emulated block
>     device(scsi and nvme)
>   - fstest Config for patched xfs 16k block size [xfs_reflink_16k_scsi]
>     TEST_DEV=/dev/sda
>     SCRATCH_DEV_POOL="/dev/sdb"
>     MKFS_OPTIONS='-f -m reflink=1,rmapbt=1, -i sparse=1, -b size=16384,
>     -s size=16384'
>   - Generic test results
>     Baseline: 6.18.0-rc7 kernel + nvme 16k logical block size
>     Patched: 6.18 kernel + scsi 16k logical block size
>     5 failures seen on generic tests and the same seen on baseline
>     No regressions introduced by the patch.
>   - XFS tests results
>     Baseline: 6.18.0-rc7 kernel + nvme16k logical block size
>     Patched: 6.18 kernel + sci 16k logical block size
>     30 failures seen on patched and baseline
>     No regressions introduced by the patch
>   - Blktests results
>     scis and block layer tests with 16k and 32k logical block size.
>     config used:
>     TEST_DEVS=(/dev/sda)
>     EXCLUDE=(block/010 block/011) # these didn't run on baseline(nvme 16k)
>     All tests passed.
> 
> Link to RFC v2: https://lore.kernel.org/all/20251203230546.1275683-2-sw.prabhu6@gmail.com/ [1]
> Link to RFC v1: https://lore.kernel.org/all/20251202021522.188419-1-sw.prabhu6@gmail.com/ [2]
> 
> Swarna Prabhu (2):
>   scsi: sd: fix write_same(16/10) to enable sector size > PAGE_SIZE
>   scsi: scsi_debug: enable sdebug_sector_size > PAGE_SIZE
> 
>  drivers/scsi/scsi_debug.c |  8 +-------
>  drivers/scsi/sd.c         | 27 +++++++++++++++++----------
>  2 files changed, 18 insertions(+), 17 deletions(-)
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/2] scsi: sd: fix write_same(16/10) to enable sector size > PAGE_SIZE

[Pankaj Raghav]

On 12/10/25 07:22, Damien Le Moal wrote:
> On 2025/12/09 17:41, sw.prabhu6@gmail.com wrote:
>> From: Swarna Prabhu <sw.prabhu6@gmail.com>
>>
>> The WRITE SAME(16) and WRITE SAME(10) scsi commands uses
>> a page from a dedicated mempool('sd_page_pool') for its
>> payload. This pool was initialized to allocate single
>> pages, which was sufficient as long as the device sector
>> size did not exceed the PAGE_SIZE.
>>
>> Given that block layer now supports block size upto
>> 64K ie beyond PAGE_SIZE, adapt sd_set_special_bvec()
>> to accommodate that.
>>
>> With the above fix, enable sector sizes > PAGE_SIZE in
>> scsi sd driver.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Swarna Prabhu <s.prabhu@samsung.com>
>> Co-developed-by: Pankaj Raghav <p.raghav@samsung.com>
>> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
>> ---
>> Note: We are allocating pages of order aligned to 
>> BLK_MAX_BLOCK_SIZE for the mempool page allocator
>> 'sd_page_pool' all the time. This is because we only
>> know that a bigger sector size device is attached at
>> sd_probe and it might be too late to reallocate mempool
>> with order >0.
> 
> That is a lot heavier on the memory for the vast majority of devices which are
> 512B or 4K block size... It may be better to have the special "large block"
> mempool attached to the scsi disk struct and keep the default single page
> mempool for all other regular devices.
> 

We had the same feeling as well and we mentioned it in the 1st RFC.

But when will you initialize the mempool for the large block devices? I don't think it
makes sense to unconditionally initialize it in init_sd.
Do we do it during the sd_probe() when we first encounter a large block device? That way
we may not waste any memory if no large block devices are attached.

--
Pankaj

Re: [v1 0/2] enable sector size > PAGE_SIZE for scsi

[Swarna Prabhu]

On Tue, Dec 09, 2025 at 05:56:05PM -0800, Damien Le Moal wrote:
> On 2025/12/09 17:41, sw.prabhu6@gmail.com wrote:
> > From: Swarna Prabhu <sw.prabhu6@gmail.com>
> > 
> > Hi All,
> > 
> > This is non RFC v1 series sent based on the feedback received on RFC
> > v2 [1] and RFC v1 [2]. This patchset enables sector sizes > PAGE_SIZE for
> > sd driver and scsi_debug driver since block layer can support block
> > size > PAGE_SIZE. There was one issue with write_same16 and write_same10
> > command, which is fixed as a part of the series.
> > 
> > Motivation:
> >  - While enabling LBS on ZoneFS, zonefs-tools tests were being skipped
> >    for conventional zones when tested on nvme block device emulated using
> >    QEMU. Hence there was a need to enable scsi with higher sector sizes
> >    to run zonefs tests for conventional zones as well.
> 
> This is super confusing: there are no conventional zones with NVMe. And why
> would a problem with NVMe require scsi patches ?
> 
Agree with you. NVME Zoned Namespace require sequential writes.

Our initial goal was to enable LBS on Zonefs. Running zonefs tests
on a scsi device covered both conventional and sequential zone based
tests. So we had to enable higher sector sizes on scsi device and while
doing so fix the issue seen with WRITE SAME commands with higher sector
sizes.

Thanks
Swarna

Re: [v1 0/2] enable sector size > PAGE_SIZE for scsi

[Damien Le Moal]

On 2025/12/12 9:18, Swarna Prabhu wrote:
> On Tue, Dec 09, 2025 at 05:56:05PM -0800, Damien Le Moal wrote:
>> On 2025/12/09 17:41, sw.prabhu6@gmail.com wrote:
>>> From: Swarna Prabhu <sw.prabhu6@gmail.com>
>>>
>>> Hi All,
>>>
>>> This is non RFC v1 series sent based on the feedback received on RFC
>>> v2 [1] and RFC v1 [2]. This patchset enables sector sizes > PAGE_SIZE for
>>> sd driver and scsi_debug driver since block layer can support block
>>> size > PAGE_SIZE. There was one issue with write_same16 and write_same10
>>> command, which is fixed as a part of the series.
>>>
>>> Motivation:
>>>  - While enabling LBS on ZoneFS, zonefs-tools tests were being skipped
>>>    for conventional zones when tested on nvme block device emulated using
>>>    QEMU. Hence there was a need to enable scsi with higher sector sizes
>>>    to run zonefs tests for conventional zones as well.
>>
>> This is super confusing: there are no conventional zones with NVMe. And why
>> would a problem with NVMe require scsi patches ?
>>
> Agree with you. NVME Zoned Namespace require sequential writes.
> 
> Our initial goal was to enable LBS on Zonefs. Running zonefs tests
> on a scsi device covered both conventional and sequential zone based
> tests. So we had to enable higher sector sizes on scsi device and while
> doing so fix the issue seen with WRITE SAME commands with higher sector
> sizes.

OK. Understood. But since this is in the end a scsi fix that is not directly
related to zonefs (zonefs does not use write same), I think you can drop any
mention of zonefs to avoid any confusion.


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/2] scsi: sd: fix write_same(16/10) to enable sector size > PAGE_SIZE

[Damien Le Moal]

On 2025/12/12 8:53, Pankaj Raghav wrote:
>>> Cc: stable@vger.kernel.org Signed-off-by: Swarna Prabhu
>>> <s.prabhu@samsung.com> Co-developed-by: Pankaj Raghav
>>> <p.raghav@samsung.com> Signed-off-by: Pankaj Raghav
>>> <p.raghav@samsung.com> --- Note: We are allocating pages of order
>>> aligned to BLK_MAX_BLOCK_SIZE for the mempool page allocator 
>>> 'sd_page_pool' all the time. This is because we only know that a bigger
>>> sector size device is attached at sd_probe and it might be too late to
>>> reallocate mempool with order >0.
>> 
>> That is a lot heavier on the memory for the vast majority of devices which
>> are 512B or 4K block size... It may be better to have the special "large
>> block" mempool attached to the scsi disk struct and keep the default
>> single page mempool for all other regular devices.
>> 
> 
> We had the same feeling as well and we mentioned it in the 1st RFC.
> 
> But when will you initialize the mempool for the large block devices? I
> don't think it makes sense to unconditionally initialize it in init_sd. Do
> we do it during the sd_probe() when we first encounter a large block device?
> That way we may not waste any memory if no large block devices are attached.

That sounds reasonable to me. Any system that has a device with a large sector
size will get this mempool initialized when the first such device is scanned,
and systems with regular disks (the vast majority of cases for scsi) will not.
You may want to be careful with that initialization in sd_probe() though: scsi
device scan is asynchronous and done in parallel for multiple devices, so you
will need some atomicity for checking the mempool existence and initializing it
if needed.

-- 
Damien Le Moal
Western Digital Research

[PATCH 1/2] scsi: sd: fix write_same(16/10) to enable sector size > PAGE_SIZE

[sw.prabhu6]

From: Swarna Prabhu <sw.prabhu6@gmail.com>

The WRITE SAME(16) and WRITE SAME(10) scsi commands uses
a page from a dedicated mempool('sd_page_pool') for its
payload. This pool was initialized to allocate single
pages, which was sufficient as long as the device sector
size did not exceed the PAGE_SIZE.

Given that block layer now supports block size upto
64K ie beyond PAGE_SIZE, initialize large page pool in
'sd_probe()' if a higher sector device is attached ensuring
atomicity. Adapt 'sd_set_special_bvec()' to use large page
pool when a higher sector size device is attached.

With the above fix, enable sector sizes > PAGE_SIZE in
scsi sd driver.

Cc: stable@vger.kernel.org
Signed-off-by: Swarna Prabhu <s.prabhu@samsung.com>
Co-developed-by: Pankaj Raghav <p.raghav@samsung.com>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 drivers/scsi/sd.c | 79 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 67 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index f50b92e63201..0e0c5dd1c668 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -112,8 +112,11 @@ static void sd_shutdown(struct device *);
 static void scsi_disk_release(struct device *cdev);
 
 static DEFINE_IDA(sd_index_ida);
+static DEFINE_MUTEX(sd_mutex_lock);
 
 static mempool_t *sd_page_pool;
+static mempool_t *sd_large_page_pool;
+static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
 static struct lock_class_key sd_bio_compl_lkclass;
 
 static const char *sd_cache_types[] = {
@@ -922,14 +925,27 @@ static void sd_config_discard(struct scsi_disk *sdkp, struct queue_limits *lim,
 		(logical_block_size >> SECTOR_SHIFT);
 }
 
-static void *sd_set_special_bvec(struct request *rq, unsigned int data_len)
+static void *sd_set_special_bvec(struct scsi_cmnd *cmd, unsigned int data_len)
 {
 	struct page *page;
+	struct request *rq = scsi_cmd_to_rq(cmd);
+	struct scsi_device *sdp = cmd->device;
+	unsigned sector_size = sdp->sector_size;
+	unsigned int nr_pages = DIV_ROUND_UP(sector_size, PAGE_SIZE);
+	int n = 0;
 
-	page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
+	if (sector_size > PAGE_SIZE)
+		page = mempool_alloc(sd_large_page_pool, GFP_ATOMIC);
+	else
+		page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
 	if (!page)
 		return NULL;
-	clear_highpage(page);
+
+	do {
+		clear_highpage(page + n);
+		n++;
+	} while (n < nr_pages);
+
 	bvec_set_page(&rq->special_vec, page, data_len, 0);
 	rq->rq_flags |= RQF_SPECIAL_PAYLOAD;
 	return bvec_virt(&rq->special_vec);
@@ -945,7 +961,7 @@ static blk_status_t sd_setup_unmap_cmnd(struct scsi_cmnd *cmd)
 	unsigned int data_len = 24;
 	char *buf;
 
-	buf = sd_set_special_bvec(rq, data_len);
+	buf = sd_set_special_bvec(cmd, data_len);
 	if (!buf)
 		return BLK_STS_RESOURCE;
 
@@ -1034,7 +1050,7 @@ static blk_status_t sd_setup_write_same16_cmnd(struct scsi_cmnd *cmd,
 	u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
 	u32 data_len = sdp->sector_size;
 
-	if (!sd_set_special_bvec(rq, data_len))
+	if (!sd_set_special_bvec(cmd, data_len))
 		return BLK_STS_RESOURCE;
 
 	cmd->cmd_len = 16;
@@ -1061,7 +1077,7 @@ static blk_status_t sd_setup_write_same10_cmnd(struct scsi_cmnd *cmd,
 	u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
 	u32 data_len = sdp->sector_size;
 
-	if (!sd_set_special_bvec(rq, data_len))
+	if (!sd_set_special_bvec(cmd, data_len))
 		return BLK_STS_RESOURCE;
 
 	cmd->cmd_len = 10;
@@ -1507,9 +1523,15 @@ static blk_status_t sd_init_command(struct scsi_cmnd *cmd)
 static void sd_uninit_command(struct scsi_cmnd *SCpnt)
 {
 	struct request *rq = scsi_cmd_to_rq(SCpnt);
+	struct scsi_device *sdp = SCpnt->device;
+	unsigned sector_size = sdp->sector_size;
 
-	if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
-		mempool_free(rq->special_vec.bv_page, sd_page_pool);
+	if (rq->rq_flags & RQF_SPECIAL_PAYLOAD) {
+		if (sector_size > PAGE_SIZE)
+			mempool_free(rq->special_vec.bv_page, sd_large_page_pool);
+		else
+			mempool_free(rq->special_vec.bv_page, sd_page_pool);
+	}
 }
 
 static bool sd_need_revalidate(struct gendisk *disk, struct scsi_disk *sdkp)
@@ -2920,10 +2942,7 @@ sd_read_capacity(struct scsi_disk *sdkp, struct queue_limits *lim,
 			  "assuming 512.\n");
 	}
 
-	if (sector_size != 512 &&
-	    sector_size != 1024 &&
-	    sector_size != 2048 &&
-	    sector_size != 4096) {
+	if (blk_validate_block_size(sector_size)) {
 		sd_printk(KERN_NOTICE, sdkp, "Unsupported sector size %d.\n",
 			  sector_size);
 		/*
@@ -4044,6 +4063,21 @@ static int sd_probe(struct device *dev)
 	sdkp->max_medium_access_timeouts = SD_MAX_MEDIUM_TIMEOUTS;
 
 	sd_revalidate_disk(gd);
+	if (sdp->sector_size > PAGE_SIZE) {
+		mutex_lock(&sd_mutex_lock);
+		if (!sd_large_page_pool) {
+			sd_large_page_pool = mempool_create_page_pool(
+					SD_MEMPOOL_SIZE, get_order(BLK_MAX_BLOCK_SIZE));
+			if (!sd_large_page_pool) {
+				printk(KERN_ERR "sd: can't create large page mempool\n");
+				error = -ENOMEM;
+				mutex_unlock(&sd_mutex_lock);
+				goto out_free_index;
+			}
+		}
+		atomic_inc(&sd_large_page_pool_users);
+		mutex_unlock(&sd_mutex_lock);
+	}
 
 	if (sdp->removable) {
 		gd->flags |= GENHD_FL_REMOVABLE;
@@ -4061,6 +4095,14 @@ static int sd_probe(struct device *dev)
 	if (error) {
 		device_unregister(&sdkp->disk_dev);
 		put_disk(gd);
+		if (sdp->sector_size > PAGE_SIZE) {
+			mutex_lock(&sd_mutex_lock);
+			if (atomic_dec_and_test(&sd_large_page_pool_users)) {
+				mempool_destroy(sd_large_page_pool);
+				sd_large_page_pool = NULL;
+			}
+			mutex_unlock(&sd_mutex_lock);
+		}
 		goto out;
 	}
 
@@ -4101,6 +4143,7 @@ static int sd_probe(struct device *dev)
 static int sd_remove(struct device *dev)
 {
 	struct scsi_disk *sdkp = dev_get_drvdata(dev);
+	struct scsi_device *sdp = sdkp->device;
 
 	scsi_autopm_get_device(sdkp->device);
 
@@ -4110,6 +4153,16 @@ static int sd_remove(struct device *dev)
 		sd_shutdown(dev);
 
 	put_disk(sdkp->disk);
+
+	if (sdp->sector_size > PAGE_SIZE) {
+		mutex_lock(&sd_mutex_lock);
+		if (atomic_dec_and_test(&sd_large_page_pool_users)) {
+			mempool_destroy(sd_large_page_pool);
+			sd_large_page_pool = NULL;
+		}
+		mutex_unlock(&sd_mutex_lock);
+	}
+
 	return 0;
 }
 
@@ -4446,6 +4499,8 @@ static void __exit exit_sd(void)
 
 	scsi_unregister_driver(&sd_template.gendrv);
 	mempool_destroy(sd_page_pool);
+	if (sd_large_page_pool)
+		mempool_destroy(sd_large_page_pool);
 
 	class_unregister(&sd_disk_class);
 
-- 
2.39.5

Re: [PATCH 1/2] scsi: sd: fix write_same(16/10) to enable sector size > PAGE_SIZE

[Damien Le Moal]

On 2/11/26 10:50, sw.prabhu6@gmail.com wrote:
> From: Swarna Prabhu <sw.prabhu6@gmail.com>
> 
> The WRITE SAME(16) and WRITE SAME(10) scsi commands uses
> a page from a dedicated mempool('sd_page_pool') for its
> payload. This pool was initialized to allocate single
> pages, which was sufficient as long as the device sector
> size did not exceed the PAGE_SIZE.
> 
> Given that block layer now supports block size upto
> 64K ie beyond PAGE_SIZE, initialize large page pool in
> 'sd_probe()' if a higher sector device is attached ensuring
> atomicity. Adapt 'sd_set_special_bvec()' to use large page
> pool when a higher sector size device is attached.
> 
> With the above fix, enable sector sizes > PAGE_SIZE in
> scsi sd driver.

This is not a fix (as in a bug fix) but rather a new feature.

> Cc: stable@vger.kernel.org

Why ? Before this patch, scsi allows only up to 4K sector size, which is not >
PAGE_SIZE.

> Signed-off-by: Swarna Prabhu <s.prabhu@samsung.com>
> Co-developed-by: Pankaj Raghav <p.raghav@samsung.com>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  drivers/scsi/sd.c | 79 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 67 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index f50b92e63201..0e0c5dd1c668 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -112,8 +112,11 @@ static void sd_shutdown(struct device *);
>  static void scsi_disk_release(struct device *cdev);
>  
>  static DEFINE_IDA(sd_index_ida);
> +static DEFINE_MUTEX(sd_mutex_lock);
>  
>  static mempool_t *sd_page_pool;
> +static mempool_t *sd_large_page_pool;
> +static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
>  static struct lock_class_key sd_bio_compl_lkclass;
>  
>  static const char *sd_cache_types[] = {
> @@ -922,14 +925,27 @@ static void sd_config_discard(struct scsi_disk *sdkp, struct queue_limits *lim,
>  		(logical_block_size >> SECTOR_SHIFT);
>  }
>  
> -static void *sd_set_special_bvec(struct request *rq, unsigned int data_len)
> +static void *sd_set_special_bvec(struct scsi_cmnd *cmd, unsigned int data_len)
>  {
>  	struct page *page;
> +	struct request *rq = scsi_cmd_to_rq(cmd);
> +	struct scsi_device *sdp = cmd->device;
> +	unsigned sector_size = sdp->sector_size;
> +	unsigned int nr_pages = DIV_ROUND_UP(sector_size, PAGE_SIZE);
> +	int n = 0;
>  
> -	page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
> +	if (sector_size > PAGE_SIZE)
> +		page = mempool_alloc(sd_large_page_pool, GFP_ATOMIC);
> +	else
> +		page = mempool_alloc(sd_page_pool, GFP_ATOMIC);
>  	if (!page)
>  		return NULL;
> -	clear_highpage(page);
> +
> +	do {
> +		clear_highpage(page + n);
> +		n++;
> +	} while (n < nr_pages);

A for loop would be a lot cleaner and simpler.

> +
>  	bvec_set_page(&rq->special_vec, page, data_len, 0);
>  	rq->rq_flags |= RQF_SPECIAL_PAYLOAD;
>  	return bvec_virt(&rq->special_vec);
> @@ -945,7 +961,7 @@ static blk_status_t sd_setup_unmap_cmnd(struct scsi_cmnd *cmd)
>  	unsigned int data_len = 24;
>  	char *buf;
>  
> -	buf = sd_set_special_bvec(rq, data_len);
> +	buf = sd_set_special_bvec(cmd, data_len);
>  	if (!buf)
>  		return BLK_STS_RESOURCE;
>  
> @@ -1034,7 +1050,7 @@ static blk_status_t sd_setup_write_same16_cmnd(struct scsi_cmnd *cmd,
>  	u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
>  	u32 data_len = sdp->sector_size;
>  
> -	if (!sd_set_special_bvec(rq, data_len))
> +	if (!sd_set_special_bvec(cmd, data_len))
>  		return BLK_STS_RESOURCE;
>  
>  	cmd->cmd_len = 16;
> @@ -1061,7 +1077,7 @@ static blk_status_t sd_setup_write_same10_cmnd(struct scsi_cmnd *cmd,
>  	u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
>  	u32 data_len = sdp->sector_size;
>  
> -	if (!sd_set_special_bvec(rq, data_len))
> +	if (!sd_set_special_bvec(cmd, data_len))
>  		return BLK_STS_RESOURCE;
>  
>  	cmd->cmd_len = 10;
> @@ -1507,9 +1523,15 @@ static blk_status_t sd_init_command(struct scsi_cmnd *cmd)
>  static void sd_uninit_command(struct scsi_cmnd *SCpnt)
>  {
>  	struct request *rq = scsi_cmd_to_rq(SCpnt);
> +	struct scsi_device *sdp = SCpnt->device;
> +	unsigned sector_size = sdp->sector_size;
>  
> -	if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
> -		mempool_free(rq->special_vec.bv_page, sd_page_pool);
> +	if (rq->rq_flags & RQF_SPECIAL_PAYLOAD) {
> +		if (sector_size > PAGE_SIZE)
> +			mempool_free(rq->special_vec.bv_page, sd_large_page_pool);
> +		else
> +			mempool_free(rq->special_vec.bv_page, sd_page_pool);
> +	}
>  }
>  
>  static bool sd_need_revalidate(struct gendisk *disk, struct scsi_disk *sdkp)
> @@ -2920,10 +2942,7 @@ sd_read_capacity(struct scsi_disk *sdkp, struct queue_limits *lim,
>  			  "assuming 512.\n");
>  	}
>  
> -	if (sector_size != 512 &&
> -	    sector_size != 1024 &&
> -	    sector_size != 2048 &&
> -	    sector_size != 4096) {
> +	if (blk_validate_block_size(sector_size)) {
>  		sd_printk(KERN_NOTICE, sdkp, "Unsupported sector size %d.\n",
>  			  sector_size);
>  		/*
> @@ -4044,6 +4063,21 @@ static int sd_probe(struct device *dev)
>  	sdkp->max_medium_access_timeouts = SD_MAX_MEDIUM_TIMEOUTS;
>  
>  	sd_revalidate_disk(gd);
> +	if (sdp->sector_size > PAGE_SIZE) {
> +		mutex_lock(&sd_mutex_lock);
> +		if (!sd_large_page_pool) {
> +			sd_large_page_pool = mempool_create_page_pool(
> +					SD_MEMPOOL_SIZE, get_order(BLK_MAX_BLOCK_SIZE));
> +			if (!sd_large_page_pool) {
> +				printk(KERN_ERR "sd: can't create large page mempool\n");
> +				error = -ENOMEM;
> +				mutex_unlock(&sd_mutex_lock);
> +				goto out_free_index;
> +			}
> +		}
> +		atomic_inc(&sd_large_page_pool_users);
> +		mutex_unlock(&sd_mutex_lock);
> +	}

It would be a lot nicer to have this defined as a helper function that goes
together with a pool destroy function (see below).

>  
>  	if (sdp->removable) {
>  		gd->flags |= GENHD_FL_REMOVABLE;
> @@ -4061,6 +4095,14 @@ static int sd_probe(struct device *dev)
>  	if (error) {
>  		device_unregister(&sdkp->disk_dev);
>  		put_disk(gd);
> +		if (sdp->sector_size > PAGE_SIZE) {
> +			mutex_lock(&sd_mutex_lock);
> +			if (atomic_dec_and_test(&sd_large_page_pool_users)) {
> +				mempool_destroy(sd_large_page_pool);
> +				sd_large_page_pool = NULL;
> +			}
> +			mutex_unlock(&sd_mutex_lock);
> +		}

This hunk is repeated twice. Make this a helper please.

>  		goto out;
>  	}
>  
> @@ -4101,6 +4143,7 @@ static int sd_probe(struct device *dev)
>  static int sd_remove(struct device *dev)
>  {
>  	struct scsi_disk *sdkp = dev_get_drvdata(dev);
> +	struct scsi_device *sdp = sdkp->device;
>  
>  	scsi_autopm_get_device(sdkp->device);
>  
> @@ -4110,6 +4153,16 @@ static int sd_remove(struct device *dev)
>  		sd_shutdown(dev);
>  
>  	put_disk(sdkp->disk);
> +
> +	if (sdp->sector_size > PAGE_SIZE) {
> +		mutex_lock(&sd_mutex_lock);
> +		if (atomic_dec_and_test(&sd_large_page_pool_users)) {
> +			mempool_destroy(sd_large_page_pool);
> +			sd_large_page_pool = NULL;
> +		}
> +		mutex_unlock(&sd_mutex_lock);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -4446,6 +4499,8 @@ static void __exit exit_sd(void)
>  
>  	scsi_unregister_driver(&sd_template.gendrv);
>  	mempool_destroy(sd_page_pool);
> +	if (sd_large_page_pool)
> +		mempool_destroy(sd_large_page_pool);
>  
>  	class_unregister(&sd_disk_class);
>  


-- 
Damien Le Moal
Western Digital Research