From: Mike Snitzer <snitzer@redhat.com>
To: Nikos Tsironis <ntsironis@arrikto.com>
Cc: dm-devel@redhat.com, ejt@redhat.com, agk@redhat.com
Subject: Re: [dm-devel] [PATCH 4/4] dm era: Remove unreachable resize operation in pre-resume function
Date: Wed, 10 Feb 2021 13:48:42 -0500 [thread overview]
Message-ID: <20210210184842.GA6689@lobo> (raw)
In-Reply-To: <20210210181205.GA7904@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
next prev parent reply other threads:[~2021-02-10 18:49 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-22 15:25 [dm-devel] [PATCH 0/4] dm era: Various minor fixes Nikos Tsironis
2021-01-22 15:25 ` [dm-devel] [PATCH 1/4] dm era: Verify the data block size hasn't changed Nikos Tsironis
2021-01-22 15:25 ` [dm-devel] [PATCH 2/4] dm era: Fix bitset memory leaks Nikos Tsironis
2021-01-22 15:25 ` [dm-devel] [PATCH 3/4] dm era: Use correct value size in equality function of writeset tree Nikos Tsironis
2021-01-22 15:25 ` [dm-devel] [PATCH 4/4] dm era: Remove unreachable resize operation in pre-resume function Nikos Tsironis
2021-02-10 18:12 ` Mike Snitzer
2021-02-10 18:48 ` Mike Snitzer [this message]
2021-02-11 14:19 ` Nikos Tsironis
2021-02-10 17:56 ` [dm-devel] [PATCH 0/4] dm era: Various minor fixes Ming Hung Tsai
2021-02-10 18:34 ` Mike Snitzer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210210184842.GA6689@lobo \
--to=snitzer@redhat.com \
--cc=agk@redhat.com \
--cc=dm-devel@redhat.com \
--cc=ejt@redhat.com \
--cc=ntsironis@arrikto.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).