[dm-devel] [PATCH v2] dm era: only resize metadata in preresume

[dm-devel] [PATCH v2] dm era: only resize metadata in preresume

[Nikos Tsironis]

Metadata resize shouldn't happen in the ctr. The ctr loads a temporary
(inactive) table that will only become active upon resume. That is why
resize should always be done in terms of resume. Otherwise a load (ctr)
whose inactive table never becomes active will incorrectly resize the
metadata.

Also, perform the resize directly in preresume, instead of using the
worker to do it.

The worker might run other metadata operations, e.g., it could start
digestion, before resizing the metadata. These operations will end up
using the old size.

This could lead to errors, like:

  device-mapper: era: metadata_digest_transcribe_writeset: dm_array_set_value failed
  device-mapper: era: process_old_eras: digest step failed, stopping digestion

The reason of the above error is that the worker started the digestion
of the archived writeset using the old, larger size.

As a result, metadata_digest_transcribe_writeset tried to write beyond
the end of the era array.

Fixes: eec40579d84873 ("dm: add era target")
Cc: stable@vger.kernel.org # v3.15+
Signed-off-by: Nikos Tsironis <ntsironis@arrikto.com>
---
 drivers/md/dm-era-target.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/md/dm-era-target.c b/drivers/md/dm-era-target.c
index d0e75fd31c1e..d9ac7372108c 100644
--- a/drivers/md/dm-era-target.c
+++ b/drivers/md/dm-era-target.c
@@ -1501,15 +1501,6 @@ static int era_ctr(struct dm_target *ti, unsigned argc, char **argv)
 	}
 	era->md = md;
 
-	era->nr_blocks = calc_nr_blocks(era);
-
-	r = metadata_resize(era->md, &era->nr_blocks);
-	if (r) {
-		ti->error = "couldn't resize metadata";
-		era_destroy(era);
-		return -ENOMEM;
-	}
-
 	era->wq = alloc_ordered_workqueue("dm-" DM_MSG_PREFIX, WQ_MEM_RECLAIM);
 	if (!era->wq) {
 		ti->error = "could not create workqueue for metadata object";
@@ -1584,9 +1575,17 @@ static int era_preresume(struct dm_target *ti)
 	dm_block_t new_size = calc_nr_blocks(era);
 
 	if (era->nr_blocks != new_size) {
-		r = in_worker1(era, metadata_resize, &new_size);
-		if (r)
+		r = metadata_resize(era->md, &new_size);
+		if (r) {
+			DMERR("%s: metadata_resize failed", __func__);
 			return r;
+		}
+
+		r = metadata_commit(era->md);
+		if (r) {
+			DMERR("%s: metadata_commit failed", __func__);
+			return r;
+		}
 
 		era->nr_blocks = new_size;
 	}
-- 
2.11.0

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

Re: [dm-devel] [PATCH v2] dm era: only resize metadata in preresume

[Mike Snitzer]

On Thu, Feb 11 2021 at  9:22am -0500,
Nikos Tsironis <ntsironis@arrikto.com> wrote:

> Metadata resize shouldn't happen in the ctr. The ctr loads a temporary
> (inactive) table that will only become active upon resume. That is why
> resize should always be done in terms of resume. Otherwise a load (ctr)
> whose inactive table never becomes active will incorrectly resize the
> metadata.
> 
> Also, perform the resize directly in preresume, instead of using the
> worker to do it.
> 
> The worker might run other metadata operations, e.g., it could start
> digestion, before resizing the metadata. These operations will end up
> using the old size.
> 
> This could lead to errors, like:
> 
>   device-mapper: era: metadata_digest_transcribe_writeset: dm_array_set_value failed
>   device-mapper: era: process_old_eras: digest step failed, stopping digestion
> 
> The reason of the above error is that the worker started the digestion
> of the archived writeset using the old, larger size.
> 
> As a result, metadata_digest_transcribe_writeset tried to write beyond
> the end of the era array.
> 
> Fixes: eec40579d84873 ("dm: add era target")
> Cc: stable@vger.kernel.org # v3.15+
> Signed-off-by: Nikos Tsironis <ntsironis@arrikto.com>
> ---
>  drivers/md/dm-era-target.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)

Thanks, I replaced the patch I created (that used a worker to resize
from preresume) with this.

Now staged for 5.12.

Mike

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