2022-01-31 11:40:28

by Jane Chu

[permalink] [raw]
Subject: [PATCH v5 3/7] dm: make dm aware of target's DAXDEV_RECOVERY capability

If one of the MD raid participating target dax device supports
DAXDEV_RECOVERY, then it'll be declared on the whole that the
MD device is capable of DAXDEV_RECOVERY.
And only when the recovery process reaches to the target driver,
it becomes deterministic whether a certain dax address range
maybe recovered, or not.

Signed-off-by: Jane Chu <[email protected]>
---
drivers/md/dm-table.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index e43096cfe9e2..8af8a81b6172 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -844,6 +844,36 @@ static bool dm_table_supports_dax(struct dm_table *t,
return true;
}

+/* Check whether device is capable of dax poison recovery */
+static int device_poison_recovery_capable(struct dm_target *ti,
+ struct dm_dev *dev, sector_t start, sector_t len, void *data)
+{
+ if (!dev->dax_dev)
+ return false;
+ return dax_recovery_capable(dev->dax_dev);
+}
+
+static bool dm_table_supports_poison_recovery(struct dm_table *t,
+ iterate_devices_callout_fn func)
+{
+ struct dm_target *ti;
+ unsigned int i;
+
+ /* Check if any DAX target supports poison recovery */
+ for (i = 0; i < dm_table_get_num_targets(t); i++) {
+ ti = dm_table_get_target(t, i);
+
+ if (!ti->type->direct_access)
+ return false;
+
+ if (ti->type->iterate_devices &&
+ ti->type->iterate_devices(ti, func, NULL))
+ return true;
+ }
+
+ return false;
+}
+
static int device_is_rq_stackable(struct dm_target *ti, struct dm_dev *dev,
sector_t start, sector_t len, void *data)
{
@@ -2014,6 +2044,9 @@ int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
blk_queue_flag_set(QUEUE_FLAG_DAX, q);
if (dm_table_supports_dax(t, device_not_dax_synchronous_capable))
set_dax_synchronous(t->md->dax_dev);
+ if (dm_table_supports_poison_recovery(t,
+ device_poison_recovery_capable))
+ set_dax_recovery(t->md->dax_dev);
}
else
blk_queue_flag_clear(QUEUE_FLAG_DAX, q);
--
2.18.4


2022-02-06 13:06:38

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH v5 3/7] dm: make dm aware of target's DAXDEV_RECOVERY capability

On Fri, Jan 28, 2022 at 1:32 PM Jane Chu <[email protected]> wrote:
>
> If one of the MD raid participating target dax device supports
> DAXDEV_RECOVERY, then it'll be declared on the whole that the
> MD device is capable of DAXDEV_RECOVERY.
> And only when the recovery process reaches to the target driver,
> it becomes deterministic whether a certain dax address range
> maybe recovered, or not.
>
> Signed-off-by: Jane Chu <[email protected]>
> ---
> drivers/md/dm-table.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index e43096cfe9e2..8af8a81b6172 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -844,6 +844,36 @@ static bool dm_table_supports_dax(struct dm_table *t,
> return true;
> }
>
> +/* Check whether device is capable of dax poison recovery */
> +static int device_poison_recovery_capable(struct dm_target *ti,
> + struct dm_dev *dev, sector_t start, sector_t len, void *data)
> +{
> + if (!dev->dax_dev)
> + return false;
> + return dax_recovery_capable(dev->dax_dev);

Hmm it's not clear to me that dax_recovery_capable is necessary. If a
dax_dev does not support recovery it can simply fail the
dax_direct_access() call with the DAX_RECOVERY flag set.

So all DM needs to worry about is passing the new @flags parameter
through the stack.

2022-02-06 17:13:12

by Jane Chu

[permalink] [raw]
Subject: Re: [PATCH v5 3/7] dm: make dm aware of target's DAXDEV_RECOVERY capability

On 2/3/2022 9:34 PM, Dan Williams wrote:
> On Fri, Jan 28, 2022 at 1:32 PM Jane Chu <[email protected]> wrote:
>>
>> If one of the MD raid participating target dax device supports
>> DAXDEV_RECOVERY, then it'll be declared on the whole that the
>> MD device is capable of DAXDEV_RECOVERY.
>> And only when the recovery process reaches to the target driver,
>> it becomes deterministic whether a certain dax address range
>> maybe recovered, or not.
>>
>> Signed-off-by: Jane Chu <[email protected]>
>> ---
>> drivers/md/dm-table.c | 33 +++++++++++++++++++++++++++++++++
>> 1 file changed, 33 insertions(+)
>>
>> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
>> index e43096cfe9e2..8af8a81b6172 100644
>> --- a/drivers/md/dm-table.c
>> +++ b/drivers/md/dm-table.c
>> @@ -844,6 +844,36 @@ static bool dm_table_supports_dax(struct dm_table *t,
>> return true;
>> }
>>
>> +/* Check whether device is capable of dax poison recovery */
>> +static int device_poison_recovery_capable(struct dm_target *ti,
>> + struct dm_dev *dev, sector_t start, sector_t len, void *data)
>> +{
>> + if (!dev->dax_dev)
>> + return false;
>> + return dax_recovery_capable(dev->dax_dev);
>
> Hmm it's not clear to me that dax_recovery_capable is necessary. If a
> dax_dev does not support recovery it can simply fail the
> dax_direct_access() call with the DAX_RECOVERY flag set.
>
> So all DM needs to worry about is passing the new @flags parameter
> through the stack.

Yeah, given your idea about adding the .recovery_write to pgmap_ops, it
wouldn't be needed.

thanks,
-jane