Currently we use min io size as the chunk size when deciding on the limit
of atomic write size.

Using min io size is not reliable, as this may be mutated when stacking
the bottom device limits.

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

Introduce a flag - DM_TARGET_STRIPED - and check this in
dm_set_device_limits() when setting this limit.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 drivers/md/dm-stripe.c        | 3 ++-
 drivers/md/dm-table.c         | 4 ++++
 include/linux/device-mapper.h | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
index a7dc04bd55e5..c30df6715149 100644
--- a/drivers/md/dm-stripe.c
+++ b/drivers/md/dm-stripe.c
@@ -466,7 +466,8 @@ static struct target_type stripe_target = {
 	.name   = "striped",
 	.version = {1, 7, 0},
 	.features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT |
-		    DM_TARGET_ATOMIC_WRITES | DM_TARGET_PASSES_CRYPTO,
+		    DM_TARGET_ATOMIC_WRITES | DM_TARGET_PASSES_CRYPTO |
+		    DM_TARGET_STRIPED,
 	.module = THIS_MODULE,
 	.ctr    = stripe_ctr,
 	.dtr    = stripe_dtr,
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 24a857ff6d0b..4f1f7173740c 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -430,6 +430,10 @@ static int dm_set_device_limits(struct dm_target *ti, struct dm_dev *dev,
 		return 0;
 	}
 
+	/* For striped types, limit the chunk_sectors to the chunk size */
+	if (dm_target_supports_striped(ti->type))
+		limits->chunk_sectors = len >> SECTOR_SHIFT;
+
 	mutex_lock(&q->limits_lock);
 	/*
 	 * BLK_FEAT_ATOMIC_WRITES is not inherited from the bottom device in
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index cb95951547ab..a863523b69ee 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -309,6 +309,9 @@ struct target_type {
 #define DM_TARGET_ATOMIC_WRITES		0x00000400
 #define dm_target_supports_atomic_writes(type) ((type)->features & DM_TARGET_ATOMIC_WRITES)
 
+#define DM_TARGET_STRIPED		0x00000800
+#define dm_target_supports_striped(type) ((type)->features & DM_TARGET_STRIPED)
+
 struct dm_target {
 	struct dm_table *table;
 	struct target_type *type;
-- 
2.31.1

On Thu, 5 Jun 2025, John Garry wrote:

> Currently we use min io size as the chunk size when deciding on the limit
> of atomic write size.
> 
> Using min io size is not reliable, as this may be mutated when stacking
> the bottom device limits.
> 
> The block stacking limits will rely on chunk_sectors in future, so set
> this value (to the chunk size).
> 
> Introduce a flag - DM_TARGET_STRIPED - and check this in
> dm_set_device_limits() when setting this limit.
> 
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
>  drivers/md/dm-stripe.c        | 3 ++-
>  drivers/md/dm-table.c         | 4 ++++
>  include/linux/device-mapper.h | 3 +++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
> index a7dc04bd55e5..c30df6715149 100644
> --- a/drivers/md/dm-stripe.c
> +++ b/drivers/md/dm-stripe.c
> @@ -466,7 +466,8 @@ static struct target_type stripe_target = {
>  	.name   = "striped",
>  	.version = {1, 7, 0},
>  	.features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT |
> -		    DM_TARGET_ATOMIC_WRITES | DM_TARGET_PASSES_CRYPTO,
> +		    DM_TARGET_ATOMIC_WRITES | DM_TARGET_PASSES_CRYPTO |
> +		    DM_TARGET_STRIPED,
>  	.module = THIS_MODULE,
>  	.ctr    = stripe_ctr,
>  	.dtr    = stripe_dtr,
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 24a857ff6d0b..4f1f7173740c 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -430,6 +430,10 @@ static int dm_set_device_limits(struct dm_target *ti, struct dm_dev *dev,
>  		return 0;
>  	}
>  
> +	/* For striped types, limit the chunk_sectors to the chunk size */
> +	if (dm_target_supports_striped(ti->type))
> +		limits->chunk_sectors = len >> SECTOR_SHIFT;
> +

len is already in sectors, so why do we shift it right?

Could this logic be moved to the function stripe_io_hints, so that we 
don't have to add a new flag for that and that we don't have to modify the 
generic dm code?

Mikulas

>  	mutex_lock(&q->limits_lock);
>  	/*
>  	 * BLK_FEAT_ATOMIC_WRITES is not inherited from the bottom device in
> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index cb95951547ab..a863523b69ee 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -309,6 +309,9 @@ struct target_type {
>  #define DM_TARGET_ATOMIC_WRITES		0x00000400
>  #define dm_target_supports_atomic_writes(type) ((type)->features & DM_TARGET_ATOMIC_WRITES)
>  
> +#define DM_TARGET_STRIPED		0x00000800
> +#define dm_target_supports_striped(type) ((type)->features & DM_TARGET_STRIPED)
> +
>  struct dm_target {
>  	struct dm_table *table;
>  	struct target_type *type;
> -- 
> 2.31.1
>

On 09/06/2025 16:19, Mikulas Patocka wrote:
>> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
>> index 24a857ff6d0b..4f1f7173740c 100644
>> --- a/drivers/md/dm-table.c
>> +++ b/drivers/md/dm-table.c
>> @@ -430,6 +430,10 @@ static int dm_set_device_limits(struct dm_target *ti, struct dm_dev *dev,
>>   		return 0;
>>   	}
>>   
>> +	/* For striped types, limit the chunk_sectors to the chunk size */
>> +	if (dm_target_supports_striped(ti->type))
>> +		limits->chunk_sectors = len >> SECTOR_SHIFT;
>> +
> len is already in sectors, so why do we shift it right?

Actually what I am passing is not the proper value at all. len holds the 
sc->stripe_width, and that seems to be md dev size / # stripes. I think 
that we want chunk_size, right?

> 
> Could this logic be moved to the function stripe_io_hints, so that we
> don't have to add a new flag for that and that we don't have to modify the
> generic dm code?
> 

That would be better. I am going to have to change 
blk_stack_atomic_writes_limits() to work for that, but I think that code 
needs to change anyway if the bottom device has its own chunk_sectors 
(as Nilay mentioned about 4/4).

Thanks,
John

On 6/5/25 8:38 PM, John Garry wrote:
> Currently we use min io size as the chunk size when deciding on the limit
> of atomic write size.
> 
> Using min io size is not reliable, as this may be mutated when stacking
> the bottom device limits.
> 
> The block stacking limits will rely on chunk_sectors in future, so set
> this value (to the chunk size).
> 
> Introduce a flag - DM_TARGET_STRIPED - and check this in
> dm_set_device_limits() when setting this limit.
> 
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
>  drivers/md/dm-stripe.c        | 3 ++-
>  drivers/md/dm-table.c         | 4 ++++
>  include/linux/device-mapper.h | 3 +++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
> index a7dc04bd55e5..c30df6715149 100644
> --- a/drivers/md/dm-stripe.c
> +++ b/drivers/md/dm-stripe.c
> @@ -466,7 +466,8 @@ static struct target_type stripe_target = {
>  	.name   = "striped",
>  	.version = {1, 7, 0},
>  	.features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT |
> -		    DM_TARGET_ATOMIC_WRITES | DM_TARGET_PASSES_CRYPTO,
> +		    DM_TARGET_ATOMIC_WRITES | DM_TARGET_PASSES_CRYPTO |
> +		    DM_TARGET_STRIPED,
>  	.module = THIS_MODULE,
>  	.ctr    = stripe_ctr,
>  	.dtr    = stripe_dtr,
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 24a857ff6d0b..4f1f7173740c 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -430,6 +430,10 @@ static int dm_set_device_limits(struct dm_target *ti, struct dm_dev *dev,
>  		return 0;
>  	}
>  
> +	/* For striped types, limit the chunk_sectors to the chunk size */
> +	if (dm_target_supports_striped(ti->type))
> +		limits->chunk_sectors = len >> SECTOR_SHIFT;
> +
I think here "len" refers to the total size of dm target and not the 
chunk sectors. So we need to modify this and take into account chunk sectors..
We can get chunk sectors, for example, like this:

	struct stripe_c *sc = ti->private;
	limits->chunk_sectors = sc->chunk_size;

But again struct stripe_c is private to dm-stripe.c and so we can't access it
here directly in dm-table.c Better we add a new callback function for dm target
type under struct target_type and then use that callback to get chunk sector. 

struct target_type stripe_target = {
        ...
        .chunk_sectors = stripe_chunk_sectors,
        ...
}

Thanks,
--Nilay

On 06/06/2025 16:16, Nilay Shroff wrote:
>> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
>> index 24a857ff6d0b..4f1f7173740c 100644
>> --- a/drivers/md/dm-table.c
>> +++ b/drivers/md/dm-table.c
>> @@ -430,6 +430,10 @@ static int dm_set_device_limits(struct dm_target *ti, struct dm_dev *dev,
>>   		return 0;
>>   	}
>>   
>> +	/* For striped types, limit the chunk_sectors to the chunk size */
>> +	if (dm_target_supports_striped(ti->type))
>> +		limits->chunk_sectors = len >> SECTOR_SHIFT;
>> +
> I think here "len" refers to the total size of dm target and not the
> chunk sectors. So we need to modify this and take into account chunk sectors..
> We can get chunk sectors, for example, like this:
> 
> 	struct stripe_c *sc = ti->private;
> 	limits->chunk_sectors = sc->chunk_size;

right

I find that the terminology used in the dm code for stripe width and 
size a bit confusing.

> 
> But again struct stripe_c is private to dm-stripe.c and so we can't access it
> here directly in dm-table.c Better we add a new callback function for dm target
> type under struct target_type and then use that callback to get chunk sector.
> 
> struct target_type stripe_target = {
>          ...
>          .chunk_sectors = stripe_chunk_sectors,
>          ...
> }


Please see reply to Mikulas on this same topic.

Thanks,
John