From: Mike Snitzer <snitzer@redhat.com>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: device-mapper development <dm-devel@redhat.com>,
"Bryn M. Reeves" <breeves@redhat.com>,
ejt@redhat.com, Alasdair G Kergon <agk@redhat.com>,
Zdenek Kabelac <zkabelac@redhat.com>
Subject: Re: staged dm_internal_{suspend, resume} related changes for wider review
Date: Wed, 5 Nov 2014 09:11:48 -0500 [thread overview]
Message-ID: <20141105141147.GA14285@redhat.com> (raw)
In-Reply-To: <alpine.LRH.2.02.1411050801270.19559@file01.intranet.prod.int.rdu2.redhat.com>
On Wed, Nov 05 2014 at 8:05am -0500,
Mikulas Patocka <mpatocka@redhat.com> wrote:
>
>
> On Wed, 5 Nov 2014, Mikulas Patocka wrote:
>
> > You can for example set the flag in the prison meaning that the prison is
> > suspended and then call dm_internal_suspend immediatelly followed by
> > dm_internal_resume - that will clear in-progress bios and prevent new bios
> > from coming in (and we don't need to change dm_internal_suspend and
> > dm_internal_resume to become so big).
It may _seem_ like they have gotten big given the code was refactored to
share code with dm_suspend and dm_resume. BUT I know you see that the
actual code complexity isn't big. I especially wanted you (and/or Bryn)
to evaluate the performance implications that my changes had on
dm-stats. I'm pretty confident there won't be much if any performance
difference (given the code is identical to what you had, except some
extra checks are made but ultimately not used, e.g. lockfs/unlockfs).
The end result of the dm_internal_{suspend,resume} changes is an
interface that is useful for DM targets in addition to dm-stats. That
is the kind of advancement DM needs.
Please focus on the performance impact of my changes, if any, and we'll
go from there.
> No, the correct sequence is this (do this in presuspend handler):
>
> 1. call dm_internal_suspend on all thin devices
> 2. set the flag in the prison meaning that the prison is blocked
> 3. call dm_internal_resume on all thin devices
I really didn't like the idea of reusing the bio-prison to achieve the
suspend of all thins to begin with. This proposal is even more suspect
given the desire to call dm_internal_suspend and dm_internal_resume from
pool_presuspend.
It just isn't code that I want to see making its way into the tree.
Sets bad precedent of hacking around problems within a target for
questionable gain (questionable in that there really isn't a
pattern/infrastructure for other more complex targets to follow so
they'd need to invent their own hack should they have a comparable
problem).
next prev parent reply other threads:[~2014-11-05 14:11 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20141028014825.GA21949@redhat.com>
[not found] ` <544F988D.7090901@redhat.com>
[not found] ` <20141028134012.GA25229@redhat.com>
[not found] ` <20141028141749.GD25229@redhat.com>
[not found] ` <20141028143742.GE25229@redhat.com>
[not found] ` <20141028171003.GG25229@redhat.com>
[not found] ` <20141028224753.GA29288@redhat.com>
[not found] ` <20141028232638.GB29288@redhat.com>
[not found] ` <20141029002125.GC29288@redhat.com>
2014-10-29 1:22 ` staged dm_internal_{suspend, resume} related changes for wider review Mike Snitzer
2014-10-29 19:06 ` Mike Snitzer
2014-10-29 20:49 ` Mike Snitzer
2014-11-03 23:25 ` Mikulas Patocka
2014-11-04 0:17 ` Mike Snitzer
2014-11-05 1:16 ` Mike Snitzer
2014-11-05 12:43 ` Mikulas Patocka
2014-11-05 13:05 ` Mikulas Patocka
2014-11-05 14:11 ` Mike Snitzer [this message]
2014-11-05 14:37 ` Mikulas Patocka
2014-11-05 14:50 ` Zdenek Kabelac
2014-11-05 16:10 ` Mike Snitzer
2014-11-05 16:56 ` Mike Snitzer
2014-11-05 20:16 ` Mike Snitzer
2014-11-05 16:29 ` Mike Snitzer
2014-11-07 16:20 ` Mikulas Patocka
2014-11-07 18:33 ` Mike Snitzer
2015-01-02 22:56 ` Mikulas Patocka
2015-01-05 15:50 ` Mike Snitzer
2015-01-08 23:52 ` [PATCH] dm: handle multiple internal suspends correctly (was: staged dm_internal_{suspend, resume} related changes for wider review) Mikulas Patocka
2014-11-07 23:16 ` [PATCH] Suspend all active bios when the pool is suspended " Mikulas Patocka
2014-11-07 23:29 ` Mike Snitzer
2014-11-09 1:17 ` [PATCH] " 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=20141105141147.GA14285@redhat.com \
--to=snitzer@redhat.com \
--cc=agk@redhat.com \
--cc=breeves@redhat.com \
--cc=dm-devel@redhat.com \
--cc=ejt@redhat.com \
--cc=mpatocka@redhat.com \
--cc=zkabelac@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.