Re: [dm-devel] [PATCH 4/4] dm era: Remove unreachable resize operation in pre-resume function

[Mike Snitzer <snitzer@redhat.com>]

On Wed, Feb 10 2021 at  1:12P -0500,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Fri, Jan 22 2021 at 10:25am -0500,
> Nikos Tsironis <ntsironis@arrikto.com> wrote:
> 
> > The device metadata are resized in era_ctr(), so the metadata resize
> > operation in era_preresume() never runs.
> > 
> > Also, note, that if the operation did ever run it would deadlock, since
> > the worker has not been started at this point.

It wouldn't have deadlocked, it'd have queued the work (see wake_worker)

> > 
> > 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 | 9 ---------
> >  1 file changed, 9 deletions(-)
> > 
> > diff --git a/drivers/md/dm-era-target.c b/drivers/md/dm-era-target.c
> > index 104fb110cd4e..c40e132e50cd 100644
> > --- a/drivers/md/dm-era-target.c
> > +++ b/drivers/md/dm-era-target.c
> > @@ -1567,15 +1567,6 @@ static int era_preresume(struct dm_target *ti)
> >  {
> >  	int r;
> >  	struct era *era = ti->private;
> > -	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)
> > -			return r;
> > -
> > -		era->nr_blocks = new_size;
> > -	}
> >  
> >  	start_worker(era);
> >  
> > -- 
> > 2.11.0
> > 
> 
> Resize shouldn't actually 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.
> 
> I'll look closer but ctr shouldn't do the actual resize, and the
> start_worker() should be moved above the resize code you've removed
> above.

Does this work for you?  If so I'll get it staged (like I've just
staged all your other dm-era fixes for 5.12).

 drivers/md/dm-era-target.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/md/dm-era-target.c b/drivers/md/dm-era-target.c
index d0e75fd31c1e..ec198e9cdafb 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";
@@ -1583,6 +1574,8 @@ static int era_preresume(struct dm_target *ti)
 	struct era *era = ti->private;
 	dm_block_t new_size = calc_nr_blocks(era);
 
+	start_worker(era);
+
 	if (era->nr_blocks != new_size) {
 		r = in_worker1(era, metadata_resize, &new_size);
 		if (r)
@@ -1591,8 +1584,6 @@ static int era_preresume(struct dm_target *ti)
 		era->nr_blocks = new_size;
 	}
 
-	start_worker(era);
-
 	r = in_worker0(era, metadata_era_rollover);
 	if (r) {
 		DMERR("%s: metadata_era_rollover failed", __func__);

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