[PATCH v3 0/5] block/md/dm: set chunk_sectors from stacked dev stripe size

[PATCH v3 0/5] block/md/dm: set chunk_sectors from stacked dev stripe size

[John Garry]

This value in io_min is used to configure any atomic write limit for the
stacked device. The idea is that the atomic write unit max is a
power-of-2 factor of the stripe size, and the stripe size is available
in io_min.

Using io_min causes issues, as:
a. it may be mutated
b. the check for io_min being set for determining if we are dealing with
a striped device is hard to get right, as reported in [0].

This series now sets chunk_sectors limit to share stripe size.

[0] https://lore.kernel.org/linux-block/888f3b1d-7817-4007-b3b3-1a2ea04df771@linux.ibm.com/T/#mecca17129f72811137d3c2f1e477634e77f06781

Based on 0d519bb0de3b ("brd: fix sleeping function called from invalid
context in brd_insert_page()")

This series fixes issues for v6.16, but it's prob better to have this in
v6.17 at this stage.

Differences to v2:
- Add RB tags (thanks!)

Differences to RFC:
- sanitize chunk_sectors for atomic write limits
- set chunk_sectors in stripe_io_hints()

John Garry (5):
  block: sanitize chunk_sectors for atomic write limits
  md/raid0: set chunk_sectors limit
  md/raid10: set chunk_sectors limit
  dm-stripe: limit chunk_sectors to the stripe size
  block: use chunk_sectors when evaluating stacked atomic write limits

 block/blk-settings.c   | 60 ++++++++++++++++++++++++++----------------
 drivers/md/dm-stripe.c |  1 +
 drivers/md/raid0.c     |  1 +
 drivers/md/raid10.c    |  1 +
 4 files changed, 40 insertions(+), 23 deletions(-)

-- 
2.43.5

[PATCH v3 1/5] block: sanitize chunk_sectors for atomic write limits

[John Garry]

Currently we just ensure that a non-zero value in chunk_sectors aligns
with any atomic write boundary, as the blk boundary functionality uses
both these values.

However it is also improper to have atomic write unit max > chunk_sectors
(for non-zero chunk_sectors), as this would lead to splitting of atomic
write bios (which is disallowed).

Sanitize atomic write unit max against chunk_sectors to avoid any
potential problems.

Fixes: d00eea91deaf3 ("block: Add extra checks in blk_validate_atomic_write_limits()")
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-settings.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index a000daafbfb4..7ca21fb32598 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -180,7 +180,7 @@ static void blk_atomic_writes_update_limits(struct queue_limits *lim)
 
 static void blk_validate_atomic_write_limits(struct queue_limits *lim)
 {
-	unsigned int boundary_sectors;
+	unsigned int boundary_sectors, chunk_bytes;
 
 	if (!(lim->features & BLK_FEAT_ATOMIC_WRITES))
 		goto unsupported;
@@ -202,6 +202,13 @@ static void blk_validate_atomic_write_limits(struct queue_limits *lim)
 			 lim->atomic_write_hw_max))
 		goto unsupported;
 
+	chunk_bytes = lim->chunk_sectors << SECTOR_SHIFT;
+	if (chunk_bytes) {
+		if (WARN_ON_ONCE(lim->atomic_write_hw_unit_max >
+			chunk_bytes))
+			goto unsupported;
+	}
+
 	boundary_sectors = lim->atomic_write_hw_boundary >> SECTOR_SHIFT;
 
 	if (boundary_sectors) {
-- 
2.43.5

[PATCH v3 2/5] md/raid0: set chunk_sectors limit

[John Garry]

Currently we use min io size as the chunk size when deciding on the
atomic write size limits - see blk_stack_atomic_writes_head().

The limit min_io size is not a reliable value to store the chunk size, as
this may be mutated by the block stacking code. Such an example would be
for the min io size less than the physical block size, and the min io size
is raised to the physical block size - see blk_stack_limits().

The block stacking limits will rely on chunk_sectors in future,
so set this value (to the chunk size).

Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Yu Kuai <yukuai3@huawei.com>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 drivers/md/raid0.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index d8f639f4ae12..cbe2a9054cb9 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -384,6 +384,7 @@ static int raid0_set_limits(struct mddev *mddev)
 	lim.max_write_zeroes_sectors = mddev->chunk_sectors;
 	lim.io_min = mddev->chunk_sectors << 9;
 	lim.io_opt = lim.io_min * mddev->raid_disks;
+	lim.chunk_sectors = mddev->chunk_sectors;
 	lim.features |= BLK_FEAT_ATOMIC_WRITES;
 	err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
 	if (err)
-- 
2.43.5

[PATCH v3 3/5] md/raid10: set chunk_sectors limit

[John Garry]

Same as done for raid0, set chunk_sectors limit to appropriately set the
atomic write size limit.

Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Yu Kuai <yukuai3@huawei.com>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 drivers/md/raid10.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index b74780af4c22..97065bb26f43 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4004,6 +4004,7 @@ static int raid10_set_queue_limits(struct mddev *mddev)
 	md_init_stacking_limits(&lim);
 	lim.max_write_zeroes_sectors = 0;
 	lim.io_min = mddev->chunk_sectors << 9;
+	lim.chunk_sectors = mddev->chunk_sectors;
 	lim.io_opt = lim.io_min * raid10_nr_stripes(conf);
 	lim.features |= BLK_FEAT_ATOMIC_WRITES;
 	err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);
-- 
2.43.5

[PATCH v3 4/5] dm-stripe: limit chunk_sectors to the stripe size

[John Garry]

Same as done for raid0, set chunk_sectors limit to appropriately set the
atomic write size limit.

Setting chunk_sectors limit in this way overrides the stacked limit
already calculated based on the bottom device limits. This is ok, as
when any bios are sent to the bottom devices, the block layer will still
respect the bottom device chunk_sectors.

Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 drivers/md/dm-stripe.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
index a1b7535c508a..8f61030d3b2d 100644
--- a/drivers/md/dm-stripe.c
+++ b/drivers/md/dm-stripe.c
@@ -459,6 +459,7 @@ static void stripe_io_hints(struct dm_target *ti,
 	struct stripe_c *sc = ti->private;
 	unsigned int chunk_size = sc->chunk_size << SECTOR_SHIFT;
 
+	limits->chunk_sectors = sc->chunk_size;
 	limits->io_min = chunk_size;
 	limits->io_opt = chunk_size * sc->stripes;
 }
-- 
2.43.5

[PATCH v3 5/5] block: use chunk_sectors when evaluating stacked atomic write limits

[John Garry]

The atomic write unit max value is limited by any stacked device stripe
size.

It is required that the atomic write unit is a power-of-2 factor of the
stripe size.

Currently we use io_min limit to hold the stripe size, and check for a
io_min <= SECTOR_SIZE when deciding if we have a striped stacked device.

Nilay reports that this causes a problem when the physical block size is
greater than SECTOR_SIZE [0].

Furthermore, io_min may be mutated when stacking devices, and this makes
it a poor candidate to hold the stripe size. Such an example (of when
io_min may change) would be when the io_min is less than the physical
block size.

Use chunk_sectors to hold the stripe size, which is more appropriate.

[0] https://lore.kernel.org/linux-block/888f3b1d-7817-4007-b3b3-1a2ea04df771@linux.ibm.com/T/#mecca17129f72811137d3c2f1e477634e77f06781

Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-settings.c | 51 +++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 7ca21fb32598..20d3563f5d3f 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -596,41 +596,47 @@ static bool blk_stack_atomic_writes_boundary_head(struct queue_limits *t,
 	return true;
 }
 
+static inline unsigned int max_pow_of_two_factor(const unsigned int nr)
+{
+	return 1 << (ffs(nr) - 1);
+}
 
-/* Check stacking of first bottom device */
-static bool blk_stack_atomic_writes_head(struct queue_limits *t,
-				struct queue_limits *b)
+static void blk_stack_atomic_writes_chunk_sectors(struct queue_limits *t)
 {
-	if (b->atomic_write_hw_boundary &&
-	    !blk_stack_atomic_writes_boundary_head(t, b))
-		return false;
+	unsigned int chunk_bytes = t->chunk_sectors << SECTOR_SHIFT;
 
-	if (t->io_min <= SECTOR_SIZE) {
-		/* No chunk sectors, so use bottom device values directly */
-		t->atomic_write_hw_unit_max = b->atomic_write_hw_unit_max;
-		t->atomic_write_hw_unit_min = b->atomic_write_hw_unit_min;
-		t->atomic_write_hw_max = b->atomic_write_hw_max;
-		return true;
-	}
+	if (!t->chunk_sectors)
+		return;
 
 	/*
 	 * Find values for limits which work for chunk size.
 	 * b->atomic_write_hw_unit_{min, max} may not be aligned with chunk
-	 * size (t->io_min), as chunk size is not restricted to a power-of-2.
+	 * size, as the chunk size is not restricted to a power-of-2.
 	 * So we need to find highest power-of-2 which works for the chunk
 	 * size.
-	 * As an example scenario, we could have b->unit_max = 16K and
-	 * t->io_min = 24K. For this case, reduce t->unit_max to a value
-	 * aligned with both limits, i.e. 8K in this example.
+	 * As an example scenario, we could have t->unit_max = 16K and
+	 * t->chunk_sectors = 24KB. For this case, reduce t->unit_max to a
+	 * value aligned with both limits, i.e. 8K in this example.
 	 */
-	t->atomic_write_hw_unit_max = b->atomic_write_hw_unit_max;
-	while (t->io_min % t->atomic_write_hw_unit_max)
-		t->atomic_write_hw_unit_max /= 2;
+	t->atomic_write_hw_unit_max = min(t->atomic_write_hw_unit_max,
+					max_pow_of_two_factor(chunk_bytes));
 
-	t->atomic_write_hw_unit_min = min(b->atomic_write_hw_unit_min,
+	t->atomic_write_hw_unit_min = min(t->atomic_write_hw_unit_min,
 					  t->atomic_write_hw_unit_max);
-	t->atomic_write_hw_max = min(b->atomic_write_hw_max, t->io_min);
+	t->atomic_write_hw_max = min(t->atomic_write_hw_max, chunk_bytes);
+}
 
+/* Check stacking of first bottom device */
+static bool blk_stack_atomic_writes_head(struct queue_limits *t,
+				struct queue_limits *b)
+{
+	if (b->atomic_write_hw_boundary &&
+	    !blk_stack_atomic_writes_boundary_head(t, b))
+		return false;
+
+	t->atomic_write_hw_unit_max = b->atomic_write_hw_unit_max;
+	t->atomic_write_hw_unit_min = b->atomic_write_hw_unit_min;
+	t->atomic_write_hw_max = b->atomic_write_hw_max;
 	return true;
 }
 
@@ -658,6 +664,7 @@ static void blk_stack_atomic_writes_limits(struct queue_limits *t,
 
 	if (!blk_stack_atomic_writes_head(t, b))
 		goto unsupported;
+	blk_stack_atomic_writes_chunk_sectors(t);
 	return;
 
 unsupported:
-- 
2.43.5

Re: [PATCH v3 5/5] block: use chunk_sectors when evaluating stacked atomic write limits

[Mikulas Patocka]

On Thu, 3 Jul 2025, John Garry wrote:

> The atomic write unit max value is limited by any stacked device stripe
> size.
> 
> It is required that the atomic write unit is a power-of-2 factor of the
> stripe size.
> 
> Currently we use io_min limit to hold the stripe size, and check for a
> io_min <= SECTOR_SIZE when deciding if we have a striped stacked device.
> 
> Nilay reports that this causes a problem when the physical block size is
> greater than SECTOR_SIZE [0].
> 
> Furthermore, io_min may be mutated when stacking devices, and this makes
> it a poor candidate to hold the stripe size. Such an example (of when
> io_min may change) would be when the io_min is less than the physical
> block size.
> 
> Use chunk_sectors to hold the stripe size, which is more appropriate.
> 
> [0] https://lore.kernel.org/linux-block/888f3b1d-7817-4007-b3b3-1a2ea04df771@linux.ibm.com/T/#mecca17129f72811137d3c2f1e477634e77f06781
> 
> Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
>  block/blk-settings.c | 51 +++++++++++++++++++++++++-------------------
>  1 file changed, 29 insertions(+), 22 deletions(-)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 7ca21fb32598..20d3563f5d3f 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -596,41 +596,47 @@ static bool blk_stack_atomic_writes_boundary_head(struct queue_limits *t,
>  	return true;
>  }
>  
> +static inline unsigned int max_pow_of_two_factor(const unsigned int nr)
> +{
> +	return 1 << (ffs(nr) - 1);

This could be changed to "nr & -nr".

> +}
>  
> -/* Check stacking of first bottom device */
> -static bool blk_stack_atomic_writes_head(struct queue_limits *t,
> -				struct queue_limits *b)
> +static void blk_stack_atomic_writes_chunk_sectors(struct queue_limits *t)
>  {
> -	if (b->atomic_write_hw_boundary &&
> -	    !blk_stack_atomic_writes_boundary_head(t, b))
> -		return false;
> +	unsigned int chunk_bytes = t->chunk_sectors << SECTOR_SHIFT;

What about integer overflow?

> -	if (t->io_min <= SECTOR_SIZE) {
> -		/* No chunk sectors, so use bottom device values directly */
> -		t->atomic_write_hw_unit_max = b->atomic_write_hw_unit_max;
> -		t->atomic_write_hw_unit_min = b->atomic_write_hw_unit_min;
> -		t->atomic_write_hw_max = b->atomic_write_hw_max;
> -		return true;
> -	}
> +	if (!t->chunk_sectors)
> +		return;
>  
>  	/*
>  	 * Find values for limits which work for chunk size.
>  	 * b->atomic_write_hw_unit_{min, max} may not be aligned with chunk
> -	 * size (t->io_min), as chunk size is not restricted to a power-of-2.
> +	 * size, as the chunk size is not restricted to a power-of-2.
>  	 * So we need to find highest power-of-2 which works for the chunk
>  	 * size.
> -	 * As an example scenario, we could have b->unit_max = 16K and
> -	 * t->io_min = 24K. For this case, reduce t->unit_max to a value
> -	 * aligned with both limits, i.e. 8K in this example.
> +	 * As an example scenario, we could have t->unit_max = 16K and
> +	 * t->chunk_sectors = 24KB. For this case, reduce t->unit_max to a
> +	 * value aligned with both limits, i.e. 8K in this example.
>  	 */
> -	t->atomic_write_hw_unit_max = b->atomic_write_hw_unit_max;
> -	while (t->io_min % t->atomic_write_hw_unit_max)
> -		t->atomic_write_hw_unit_max /= 2;
> +	t->atomic_write_hw_unit_max = min(t->atomic_write_hw_unit_max,
> +					max_pow_of_two_factor(chunk_bytes));
>  
> -	t->atomic_write_hw_unit_min = min(b->atomic_write_hw_unit_min,
> +	t->atomic_write_hw_unit_min = min(t->atomic_write_hw_unit_min,
>  					  t->atomic_write_hw_unit_max);
> -	t->atomic_write_hw_max = min(b->atomic_write_hw_max, t->io_min);
> +	t->atomic_write_hw_max = min(t->atomic_write_hw_max, chunk_bytes);
> +}
>  
> +/* Check stacking of first bottom device */
> +static bool blk_stack_atomic_writes_head(struct queue_limits *t,
> +				struct queue_limits *b)
> +{
> +	if (b->atomic_write_hw_boundary &&
> +	    !blk_stack_atomic_writes_boundary_head(t, b))
> +		return false;
> +
> +	t->atomic_write_hw_unit_max = b->atomic_write_hw_unit_max;
> +	t->atomic_write_hw_unit_min = b->atomic_write_hw_unit_min;
> +	t->atomic_write_hw_max = b->atomic_write_hw_max;
>  	return true;
>  }
>  
> @@ -658,6 +664,7 @@ static void blk_stack_atomic_writes_limits(struct queue_limits *t,
>  
>  	if (!blk_stack_atomic_writes_head(t, b))
>  		goto unsupported;
> +	blk_stack_atomic_writes_chunk_sectors(t);
>  	return;
>  
>  unsupported:
> -- 
> 2.43.5
> 

Re: [PATCH v3 5/5] block: use chunk_sectors when evaluating stacked atomic write limits

[John Garry]

On 03/07/2025 14:31, Mikulas Patocka wrote:
> 
> 
> On Thu, 3 Jul 2025, John Garry wrote:
> 
>> The atomic write unit max value is limited by any stacked device stripe
>> size.
>>
>> It is required that the atomic write unit is a power-of-2 factor of the
>> stripe size.
>>
>> Currently we use io_min limit to hold the stripe size, and check for a
>> io_min <= SECTOR_SIZE when deciding if we have a striped stacked device.
>>
>> Nilay reports that this causes a problem when the physical block size is
>> greater than SECTOR_SIZE [0].
>>
>> Furthermore, io_min may be mutated when stacking devices, and this makes
>> it a poor candidate to hold the stripe size. Such an example (of when
>> io_min may change) would be when the io_min is less than the physical
>> block size.
>>
>> Use chunk_sectors to hold the stripe size, which is more appropriate.
>>
>> [0] https://urldefense.com/v3/__https://lore.kernel.org/linux-block/888f3b1d-7817-4007-b3b3-1a2ea04df771@linux.ibm.com/T/*mecca17129f72811137d3c2f1e477634e77f06781__;Iw!!ACWV5N9M2RV99hQ!OoKnbVR6yxyDj7-7bpZceNOD59hud0wfw_-fZLPgcGi9XdFQyfpfFFmbYzR_HdvM8epaJqe_dCGnIEgDPMze$
>>
>> Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
>> Signed-off-by: John Garry <john.g.garry@oracle.com>
>> ---
>>   block/blk-settings.c | 51 +++++++++++++++++++++++++-------------------
>>   1 file changed, 29 insertions(+), 22 deletions(-)
>>
>> diff --git a/block/blk-settings.c b/block/blk-settings.c
>> index 7ca21fb32598..20d3563f5d3f 100644
>> --- a/block/blk-settings.c
>> +++ b/block/blk-settings.c
>> @@ -596,41 +596,47 @@ static bool blk_stack_atomic_writes_boundary_head(struct queue_limits *t,
>>   	return true;
>>   }
>>   
>> +static inline unsigned int max_pow_of_two_factor(const unsigned int nr)
>> +{
>> +	return 1 << (ffs(nr) - 1);
> 
> This could be changed to "nr & -nr".

Sure, but I doubt if that is a more natural form.

> 
>> +}
>>   
>> -/* Check stacking of first bottom device */
>> -static bool blk_stack_atomic_writes_head(struct queue_limits *t,
>> -				struct queue_limits *b)
>> +static void blk_stack_atomic_writes_chunk_sectors(struct queue_limits *t)
>>   {
>> -	if (b->atomic_write_hw_boundary &&
>> -	    !blk_stack_atomic_writes_boundary_head(t, b))
>> -		return false;
>> +	unsigned int chunk_bytes = t->chunk_sectors << SECTOR_SHIFT;
> 
> What about integer overflow?

I suppose theoretically it could happen, and I'm happy to change.

However there seems to be precedent in assuming it won't:

- in stripe_op_hints(), we hold chunk_size in an unsigned int
- in raid0_set_limits(), we hold mddev->chunk_sectors << 9 in 
lim.io_min, which is an unsigned int type.

Please let me know your thoughts on also changing these sort of 
instances. Is it realistic to expect chunk_bytes > UINT_MAX?

Thanks,
John

Re: [PATCH v3 5/5] block: use chunk_sectors when evaluating stacked atomic write limits

[Mikulas Patocka]

On Thu, 3 Jul 2025, John Garry wrote:

> > >   -/* Check stacking of first bottom device */
> > > -static bool blk_stack_atomic_writes_head(struct queue_limits *t,
> > > -				struct queue_limits *b)
> > > +static void blk_stack_atomic_writes_chunk_sectors(struct queue_limits *t)
> > >   {
> > > -	if (b->atomic_write_hw_boundary &&
> > > -	    !blk_stack_atomic_writes_boundary_head(t, b))
> > > -		return false;
> > > +	unsigned int chunk_bytes = t->chunk_sectors << SECTOR_SHIFT;
> > 
> > What about integer overflow?
> 
> I suppose theoretically it could happen, and I'm happy to change.
> 
> However there seems to be precedent in assuming it won't:
> 
> - in stripe_op_hints(), we hold chunk_size in an unsigned int
> - in raid0_set_limits(), we hold mddev->chunk_sectors << 9 in lim.io_min,
> which is an unsigned int type.
> 
> Please let me know your thoughts on also changing these sort of instances. Is
> it realistic to expect chunk_bytes > UINT_MAX?
> 
> Thanks,
> John

dm-stripe can be created with a stripe size that is more than 0xffffffff 
bytes.

Though, the integer overflow already exists in the existing dm-stripe 
target:
static void stripe_io_hints(struct dm_target *ti,
                            struct queue_limits *limits)
{
        struct stripe_c *sc = ti->private;
        unsigned int chunk_size = sc->chunk_size << SECTOR_SHIFT;

        limits->io_min = chunk_size;
        limits->io_opt = chunk_size * sc->stripes;
}
What should we set there as io_min and io_opt if sc->chunk_size << 
SECTOR_SHIFT overflows? Should we set nothing?

Mikulas

Re: [PATCH v3 5/5] block: use chunk_sectors when evaluating stacked atomic write limits

[John Garry]

On 03/07/2025 16:36, Mikulas Patocka wrote:
>> I suppose theoretically it could happen, and I'm happy to change.
>>
>> However there seems to be precedent in assuming it won't:
>>
>> - in stripe_op_hints(), we hold chunk_size in an unsigned int
>> - in raid0_set_limits(), we hold mddev->chunk_sectors << 9 in lim.io_min,
>> which is an unsigned int type.
>>
>> Please let me know your thoughts on also changing these sort of instances. Is
>> it realistic to expect chunk_bytes > UINT_MAX?
>>
>> Thanks,
>> John
> dm-stripe can be created with a stripe size that is more than 0xffffffff
> bytes.
> 
> Though, the integer overflow already exists in the existing dm-stripe
> target:
> static void stripe_io_hints(struct dm_target *ti,
>                              struct queue_limits *limits)
> {
>          struct stripe_c *sc = ti->private;
>          unsigned int chunk_size = sc->chunk_size << SECTOR_SHIFT;
> 
>          limits->io_min = chunk_size;
>          limits->io_opt = chunk_size * sc->stripes;
> }
> What should we set there as io_min and io_opt if sc->chunk_size <<
> SECTOR_SHIFT overflows?


> Should we set nothing?

For io_min/opt, maybe reduce to a factor of the stripe size / width (and 
which fits in a unsigned int).

I am not sure if it is even sane to have such huge values in io_min and 
the bottom disk io_min should be used directly instead.

Martin Petersen might have a better idea.. he added those sysfs files :)

Thanks,
John

Re: [PATCH v3 5/5] block: use chunk_sectors when evaluating stacked atomic write limits

[Martin K. Petersen]

John,

> For io_min/opt, maybe reduce to a factor of the stripe size / width
> (and which fits in a unsigned int).
>
> I am not sure if it is even sane to have such huge values in io_min
> and the bottom disk io_min should be used directly instead.

The intent for io_min was to convey the physical_block_size in the case
of an individual drive. And for it to be set to the stripe chunk size in
stacking scenarios that would otherwise involve read-modify-write (i.e.
RAID5 and RAID6).

io_opt was meant to communicate the stripe width. Reporting very large
values for io_opt is generally counterproductive since we can't write
multiple gigabytes in a single operation anyway.

logical <= physical <= io_min <= io_opt <= max_sectors <= max_hw_sectors

-- 
Martin K. Petersen

Re: [PATCH v3 5/5] block: use chunk_sectors when evaluating stacked atomic write limits

[John Garry]

On 09/07/2025 02:39, Martin K. Petersen wrote:
> 
> John,
> 
>> For io_min/opt, maybe reduce to a factor of the stripe size / width
>> (and which fits in a unsigned int).
>>
>> I am not sure if it is even sane to have such huge values in io_min
>> and the bottom disk io_min should be used directly instead.
> 
> The intent for io_min was to convey the physical_block_size in the case
> of an individual drive. And for it to be set to the stripe chunk size in
> stacking scenarios that would otherwise involve read-modify-write (i.e.
> RAID5 and RAID6).

And the same is done for md raid0/dm stripe; I suppose that the idea is 
to encourage larger than chunk size writes to see the performance 
advantage in striping.

 > > io_opt was meant to communicate the stripe width. Reporting very large
> values for io_opt is generally counterproductive since we can't write
> multiple gigabytes in a single operation anyway.
>

Right, and so it seems counterproductive to have chunk size much bigger 
than bottom device io_opt

> logical <= physical <= io_min <= io_opt <= max_sectors <= max_hw_sectors
> 

Re: [PATCH v3 5/5] block: use chunk_sectors when evaluating stacked atomic write limits

[John Garry]

On 09/07/2025 02:39, Martin K. Petersen wrote:
> The intent for io_min was to convey the physical_block_size in the case
> of an individual drive. And for it to be set to the stripe chunk size in
> stacking scenarios that would otherwise involve read-modify-write (i.e.
> RAID5 and RAID6).
> 
> io_opt was meant to communicate the stripe width. Reporting very large
> values for io_opt is generally counterproductive since we can't write
> multiple gigabytes in a single operation anyway.
> 
> logical <= physical <= io_min <= io_opt <= max_sectors <= max_hw_sectors

Does pbs need to be a power-of-2?

The block queue limits and splitting code seems to rely on that, but it 
is not policed AFAICS.

The stacking code seems to just want it to be a multiple of lbs, which 
itself must be a power-of-2.

Thanks,
John

Re: [PATCH v3 5/5] block: use chunk_sectors when evaluating stacked atomic write limits

[Martin K. Petersen]

John,

> Does pbs need to be a power-of-2?

For ATA and SCSI, definitely. NVMe could technically report a multiple
of the logical block size but I've never come across anything that
didn't report a power of two.

-- 
Martin K. Petersen