[dm-devel] [PATCH] dm-zero: support discards

[dm-devel] [PATCH] dm-zero: support discards

[Mikulas Patocka]

This patch adds discard support to dm-zero. The discards are ignored.
It is useful when the user combines dm-zero with other discard-supporting
targets in the same table; without dm-zero support, discards would be
disabled for the whole combined device.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-table.c |    9 ++++++++-
 drivers/md/dm-zero.c  |    4 +++-
 2 files changed, 11 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/md/dm-zero.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-zero.c
+++ linux-2.6/drivers/md/dm-zero.c
@@ -27,6 +27,7 @@ static int zero_ctr(struct dm_target *ti
 	 * Silently drop discards, avoiding -EOPNOTSUPP.
 	 */
 	ti->num_discard_bios = 1;
+	ti->discards_supported = true;
 
 	return 0;
 }
@@ -45,6 +46,7 @@ static int zero_map(struct dm_target *ti
 		zero_fill_bio(bio);
 		break;
 	case REQ_OP_WRITE:
+	case REQ_OP_DISCARD:
 		/* writes get silently dropped */
 		break;
 	default:
@@ -59,7 +61,7 @@ static int zero_map(struct dm_target *ti
 
 static struct target_type zero_target = {
 	.name   = "zero",
-	.version = {1, 1, 0},
+	.version = {1, 2, 0},
 	.features = DM_TARGET_NOWAIT,
 	.module = THIS_MODULE,
 	.ctr    = zero_ctr,
Index: linux-2.6/drivers/md/dm-table.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-table.c
+++ linux-2.6/drivers/md/dm-table.c
@@ -1670,8 +1670,15 @@ int dm_calculate_queue_limits(struct dm_
 
 		blk_set_stacking_limits(&ti_limits);
 
-		if (!ti->type->iterate_devices)
+		if (!ti->type->iterate_devices) {
+			if (ti->discards_supported) {
+				ti_limits.max_discard_sectors = UINT_MAX;
+				ti_limits.max_hw_discard_sectors = UINT_MAX;
+				ti_limits.discard_granularity = 512;
+				ti_limits.discard_alignment = 0;
+			}
 			goto combine_limits;
+		}
 
 		/*
 		 * Combine queue limits of all the devices this target uses.
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm-zero: support discards

[Milan Broz]

On 3/6/23 20:10, Mikulas Patocka wrote:
> This patch adds discard support to dm-zero. The discards are ignored.
> It is useful when the user combines dm-zero with other discard-supporting
> targets in the same table; without dm-zero support, discards would be
> disabled for the whole combined device.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Tested-by: Milan Broz <gmazyland@gmail.com>

Just for the context: cryptsetup use dm-zero to mask certain areas
(in Bitlocker-compatible mapping) and discard flag disappeared in this situation
(discard is not supported configuration here, but it should work :)
Originally reported here https://gitlab.com/cryptsetup/cryptsetup/-/issues/718

Propagating discard support in dm-zero is definitely useful for other uses, though.

Thanks,
Milan

> 
> ---
>   drivers/md/dm-table.c |    9 ++++++++-
>   drivers/md/dm-zero.c  |    4 +++-
>   2 files changed, 11 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/md/dm-zero.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/dm-zero.c
> +++ linux-2.6/drivers/md/dm-zero.c
> @@ -27,6 +27,7 @@ static int zero_ctr(struct dm_target *ti
>   	 * Silently drop discards, avoiding -EOPNOTSUPP.
>   	 */
>   	ti->num_discard_bios = 1;
> +	ti->discards_supported = true;
>   
>   	return 0;
>   }
> @@ -45,6 +46,7 @@ static int zero_map(struct dm_target *ti
>   		zero_fill_bio(bio);
>   		break;
>   	case REQ_OP_WRITE:
> +	case REQ_OP_DISCARD:
>   		/* writes get silently dropped */
>   		break;
>   	default:
> @@ -59,7 +61,7 @@ static int zero_map(struct dm_target *ti
>   
>   static struct target_type zero_target = {
>   	.name   = "zero",
> -	.version = {1, 1, 0},
> +	.version = {1, 2, 0},
>   	.features = DM_TARGET_NOWAIT,
>   	.module = THIS_MODULE,
>   	.ctr    = zero_ctr,
> Index: linux-2.6/drivers/md/dm-table.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/dm-table.c
> +++ linux-2.6/drivers/md/dm-table.c
> @@ -1670,8 +1670,15 @@ int dm_calculate_queue_limits(struct dm_
>   
>   		blk_set_stacking_limits(&ti_limits);
>   
> -		if (!ti->type->iterate_devices)
> +		if (!ti->type->iterate_devices) {
> +			if (ti->discards_supported) {
> +				ti_limits.max_discard_sectors = UINT_MAX;
> +				ti_limits.max_hw_discard_sectors = UINT_MAX;
> +				ti_limits.discard_granularity = 512;
> +				ti_limits.discard_alignment = 0;
> +			}
>   			goto combine_limits;
> +		}
>   
>   		/*
>   		 * Combine queue limits of all the devices this target uses.
> 

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2] dm-zero, dm-error: support discards

[Mikulas Patocka]

dm-zero, dm-error: support discards

This patch adds discard support to dm-zero and dm-error. dm-zero ignores
the discards, dm-error return -EIO. It is useful when the user combines
dm-zero or dm-error with other discard-supporting targets in the same
table; without dm-zero or dm-error support, discards would be disabled for
the whole combined device.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-table.c  |    6 +++++-
 drivers/md/dm-target.c |   11 ++++++++++-
 drivers/md/dm-zero.c   |   12 +++++++++++-
 3 files changed, 26 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/md/dm-zero.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-zero.c
+++ linux-2.6/drivers/md/dm-zero.c
@@ -27,6 +27,7 @@ static int zero_ctr(struct dm_target *ti
 	 * Silently drop discards, avoiding -EOPNOTSUPP.
 	 */
 	ti->num_discard_bios = 1;
+	ti->discards_supported = true;
 
 	return 0;
 }
@@ -45,6 +46,7 @@ static int zero_map(struct dm_target *ti
 		zero_fill_bio(bio);
 		break;
 	case REQ_OP_WRITE:
+	case REQ_OP_DISCARD:
 		/* writes get silently dropped */
 		break;
 	default:
@@ -57,13 +59,21 @@ static int zero_map(struct dm_target *ti
 	return DM_MAPIO_SUBMITTED;
 }
 
+static void zero_io_hints(struct dm_target *ti, struct queue_limits *limits)
+{
+	limits->max_discard_sectors = UINT_MAX;
+	limits->max_hw_discard_sectors = UINT_MAX;
+	limits->discard_granularity = 512;
+}
+
 static struct target_type zero_target = {
 	.name   = "zero",
-	.version = {1, 1, 0},
+	.version = {1, 2, 0},
 	.features = DM_TARGET_NOWAIT,
 	.module = THIS_MODULE,
 	.ctr    = zero_ctr,
 	.map    = zero_map,
+	.io_hints = zero_io_hints,
 };
 
 static int __init dm_zero_init(void)
Index: linux-2.6/drivers/md/dm-table.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-table.c
+++ linux-2.6/drivers/md/dm-table.c
@@ -1670,8 +1670,12 @@ int dm_calculate_queue_limits(struct dm_
 
 		blk_set_stacking_limits(&ti_limits);
 
-		if (!ti->type->iterate_devices)
+		if (!ti->type->iterate_devices) {
+			/* Set I/O hints portion of queue limits */
+			if (ti->type->io_hints)
+				ti->type->io_hints(ti, &ti_limits);
 			goto combine_limits;
+		}
 
 		/*
 		 * Combine queue limits of all the devices this target uses.
Index: linux-2.6/drivers/md/dm-target.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-target.c
+++ linux-2.6/drivers/md/dm-target.c
@@ -119,6 +119,7 @@ static int io_err_ctr(struct dm_target *
 	 * Return error for discards instead of -EOPNOTSUPP
 	 */
 	tt->num_discard_bios = 1;
+	tt->discards_supported = true;
 
 	return 0;
 }
@@ -145,6 +146,13 @@ static void io_err_release_clone_rq(stru
 {
 }
 
+static void io_err_io_hints(struct dm_target *ti, struct queue_limits *limits)
+{
+	limits->max_discard_sectors = UINT_MAX;
+	limits->max_hw_discard_sectors = UINT_MAX;
+	limits->discard_granularity = 512;
+}
+
 static long io_err_dax_direct_access(struct dm_target *ti, pgoff_t pgoff,
 		long nr_pages, enum dax_access_mode mode, void **kaddr,
 		pfn_t *pfn)
@@ -154,13 +162,14 @@ static long io_err_dax_direct_access(str
 
 static struct target_type error_target = {
 	.name = "error",
-	.version = {1, 5, 0},
+	.version = {1, 6, 0},
 	.features = DM_TARGET_WILDCARD,
 	.ctr  = io_err_ctr,
 	.dtr  = io_err_dtr,
 	.map  = io_err_map,
 	.clone_and_map_rq = io_err_clone_and_map_rq,
 	.release_clone_rq = io_err_release_clone_rq,
+	.io_hints = io_err_io_hints,
 	.direct_access = io_err_dax_direct_access,
 };
 
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v2] dm-zero, dm-error: support discards

[Milan Broz]

On 3/7/23 18:32, Mikulas Patocka wrote:
> dm-zero, dm-error: support discards
> 
> This patch adds discard support to dm-zero and dm-error. dm-zero ignores
> the discards, dm-error return -EIO. It is useful when the user combines
> dm-zero or dm-error with other discard-supporting targets in the same
> table; without dm-zero or dm-error support, discards would be disabled for
> the whole combined device.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

For the v2 (including the error target):

Tested-by: Milan Broz <gmazyland@gmail.com>

But I would better split it to two patches (per target).

And as kabi mentioned elsewhere - I understand it the way that discards
should fail for error target, as it can be in somewhere down the stack
just write of zeroes (so the patch is IMO correct).

Milan

> 
> ---
>   drivers/md/dm-table.c  |    6 +++++-
>   drivers/md/dm-target.c |   11 ++++++++++-
>   drivers/md/dm-zero.c   |   12 +++++++++++-
>   3 files changed, 26 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/drivers/md/dm-zero.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/dm-zero.c
> +++ linux-2.6/drivers/md/dm-zero.c
> @@ -27,6 +27,7 @@ static int zero_ctr(struct dm_target *ti
>   	 * Silently drop discards, avoiding -EOPNOTSUPP.
>   	 */
>   	ti->num_discard_bios = 1;
> +	ti->discards_supported = true;
>   
>   	return 0;
>   }
> @@ -45,6 +46,7 @@ static int zero_map(struct dm_target *ti
>   		zero_fill_bio(bio);
>   		break;
>   	case REQ_OP_WRITE:
> +	case REQ_OP_DISCARD:
>   		/* writes get silently dropped */
>   		break;
>   	default:
> @@ -57,13 +59,21 @@ static int zero_map(struct dm_target *ti
>   	return DM_MAPIO_SUBMITTED;
>   }
>   
> +static void zero_io_hints(struct dm_target *ti, struct queue_limits *limits)
> +{
> +	limits->max_discard_sectors = UINT_MAX;
> +	limits->max_hw_discard_sectors = UINT_MAX;
> +	limits->discard_granularity = 512;
> +}
> +
>   static struct target_type zero_target = {
>   	.name   = "zero",
> -	.version = {1, 1, 0},
> +	.version = {1, 2, 0},
>   	.features = DM_TARGET_NOWAIT,
>   	.module = THIS_MODULE,
>   	.ctr    = zero_ctr,
>   	.map    = zero_map,
> +	.io_hints = zero_io_hints,
>   };
>   
>   static int __init dm_zero_init(void)
> Index: linux-2.6/drivers/md/dm-table.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/dm-table.c
> +++ linux-2.6/drivers/md/dm-table.c
> @@ -1670,8 +1670,12 @@ int dm_calculate_queue_limits(struct dm_
>   
>   		blk_set_stacking_limits(&ti_limits);
>   
> -		if (!ti->type->iterate_devices)
> +		if (!ti->type->iterate_devices) {
> +			/* Set I/O hints portion of queue limits */
> +			if (ti->type->io_hints)
> +				ti->type->io_hints(ti, &ti_limits);
>   			goto combine_limits;
> +		}
>   
>   		/*
>   		 * Combine queue limits of all the devices this target uses.
> Index: linux-2.6/drivers/md/dm-target.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/dm-target.c
> +++ linux-2.6/drivers/md/dm-target.c
> @@ -119,6 +119,7 @@ static int io_err_ctr(struct dm_target *
>   	 * Return error for discards instead of -EOPNOTSUPP
>   	 */
>   	tt->num_discard_bios = 1;
> +	tt->discards_supported = true;
>   
>   	return 0;
>   }
> @@ -145,6 +146,13 @@ static void io_err_release_clone_rq(stru
>   {
>   }
>   
> +static void io_err_io_hints(struct dm_target *ti, struct queue_limits *limits)
> +{
> +	limits->max_discard_sectors = UINT_MAX;
> +	limits->max_hw_discard_sectors = UINT_MAX;
> +	limits->discard_granularity = 512;
> +}
> +
>   static long io_err_dax_direct_access(struct dm_target *ti, pgoff_t pgoff,
>   		long nr_pages, enum dax_access_mode mode, void **kaddr,
>   		pfn_t *pfn)
> @@ -154,13 +162,14 @@ static long io_err_dax_direct_access(str
>   
>   static struct target_type error_target = {
>   	.name = "error",
> -	.version = {1, 5, 0},
> +	.version = {1, 6, 0},
>   	.features = DM_TARGET_WILDCARD,
>   	.ctr  = io_err_ctr,
>   	.dtr  = io_err_dtr,
>   	.map  = io_err_map,
>   	.clone_and_map_rq = io_err_clone_and_map_rq,
>   	.release_clone_rq = io_err_release_clone_rq,
> +	.io_hints = io_err_io_hints,
>   	.direct_access = io_err_dax_direct_access,
>   };
>   
> 

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v2] dm-zero, dm-error: support discards

[Milan Broz]

On 3/10/23 09:05, Milan Broz wrote:
> On 3/7/23 18:32, Mikulas Patocka wrote:
>> dm-zero, dm-error: support discards
>>
>> This patch adds discard support to dm-zero and dm-error. dm-zero ignores
>> the discards, dm-error return -EIO. It is useful when the user combines
>> dm-zero or dm-error with other discard-supporting targets in the same
>> table; without dm-zero or dm-error support, discards would be disabled for
>> the whole combined device.
>>
>> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Mike, do you plan this patch for 6.4 (I do not see it in for-next branch yet)
or it needs some changes still?

Thanks,
Milan

> 
> For the v2 (including the error target):
> 
> Tested-by: Milan Broz <gmazyland@gmail.com>
> 
> But I would better split it to two patches (per target).
> 
> And as kabi mentioned elsewhere - I understand it the way that discards
> should fail for error target, as it can be in somewhere down the stack
> just write of zeroes (so the patch is IMO correct).
> 
> Milan
> 
>>
>> ---
>>    drivers/md/dm-table.c  |    6 +++++-
>>    drivers/md/dm-target.c |   11 ++++++++++-
>>    drivers/md/dm-zero.c   |   12 +++++++++++-
>>    3 files changed, 26 insertions(+), 3 deletions(-)
>>
>> Index: linux-2.6/drivers/md/dm-zero.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/md/dm-zero.c
>> +++ linux-2.6/drivers/md/dm-zero.c
>> @@ -27,6 +27,7 @@ static int zero_ctr(struct dm_target *ti
>>    	 * Silently drop discards, avoiding -EOPNOTSUPP.
>>    	 */
>>    	ti->num_discard_bios = 1;
>> +	ti->discards_supported = true;
>>    
>>    	return 0;
>>    }
>> @@ -45,6 +46,7 @@ static int zero_map(struct dm_target *ti
>>    		zero_fill_bio(bio);
>>    		break;
>>    	case REQ_OP_WRITE:
>> +	case REQ_OP_DISCARD:
>>    		/* writes get silently dropped */
>>    		break;
>>    	default:
>> @@ -57,13 +59,21 @@ static int zero_map(struct dm_target *ti
>>    	return DM_MAPIO_SUBMITTED;
>>    }
>>    
>> +static void zero_io_hints(struct dm_target *ti, struct queue_limits *limits)
>> +{
>> +	limits->max_discard_sectors = UINT_MAX;
>> +	limits->max_hw_discard_sectors = UINT_MAX;
>> +	limits->discard_granularity = 512;
>> +}
>> +
>>    static struct target_type zero_target = {
>>    	.name   = "zero",
>> -	.version = {1, 1, 0},
>> +	.version = {1, 2, 0},
>>    	.features = DM_TARGET_NOWAIT,
>>    	.module = THIS_MODULE,
>>    	.ctr    = zero_ctr,
>>    	.map    = zero_map,
>> +	.io_hints = zero_io_hints,
>>    };
>>    
>>    static int __init dm_zero_init(void)
>> Index: linux-2.6/drivers/md/dm-table.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/md/dm-table.c
>> +++ linux-2.6/drivers/md/dm-table.c
>> @@ -1670,8 +1670,12 @@ int dm_calculate_queue_limits(struct dm_
>>    
>>    		blk_set_stacking_limits(&ti_limits);
>>    
>> -		if (!ti->type->iterate_devices)
>> +		if (!ti->type->iterate_devices) {
>> +			/* Set I/O hints portion of queue limits */
>> +			if (ti->type->io_hints)
>> +				ti->type->io_hints(ti, &ti_limits);
>>    			goto combine_limits;
>> +		}
>>    
>>    		/*
>>    		 * Combine queue limits of all the devices this target uses.
>> Index: linux-2.6/drivers/md/dm-target.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/md/dm-target.c
>> +++ linux-2.6/drivers/md/dm-target.c
>> @@ -119,6 +119,7 @@ static int io_err_ctr(struct dm_target *
>>    	 * Return error for discards instead of -EOPNOTSUPP
>>    	 */
>>    	tt->num_discard_bios = 1;
>> +	tt->discards_supported = true;
>>    
>>    	return 0;
>>    }
>> @@ -145,6 +146,13 @@ static void io_err_release_clone_rq(stru
>>    {
>>    }
>>    
>> +static void io_err_io_hints(struct dm_target *ti, struct queue_limits *limits)
>> +{
>> +	limits->max_discard_sectors = UINT_MAX;
>> +	limits->max_hw_discard_sectors = UINT_MAX;
>> +	limits->discard_granularity = 512;
>> +}
>> +
>>    static long io_err_dax_direct_access(struct dm_target *ti, pgoff_t pgoff,
>>    		long nr_pages, enum dax_access_mode mode, void **kaddr,
>>    		pfn_t *pfn)
>> @@ -154,13 +162,14 @@ static long io_err_dax_direct_access(str
>>    
>>    static struct target_type error_target = {
>>    	.name = "error",
>> -	.version = {1, 5, 0},
>> +	.version = {1, 6, 0},
>>    	.features = DM_TARGET_WILDCARD,
>>    	.ctr  = io_err_ctr,
>>    	.dtr  = io_err_dtr,
>>    	.map  = io_err_map,
>>    	.clone_and_map_rq = io_err_clone_and_map_rq,
>>    	.release_clone_rq = io_err_release_clone_rq,
>> +	.io_hints = io_err_io_hints,
>>    	.direct_access = io_err_dax_direct_access,
>>    };
>>    
>>
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://listman.redhat.com/mailman/listinfo/dm-devel
> 

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel