[dm-devel] staged dm_internal_{suspend, resume} related changes for wider review

[Mike Snitzer]

On Wed, Nov 05 2014 at 11:56am -0500,
Mike Snitzer <snitzer at redhat.com> wrote:

> On Wed, Nov 05 2014 at 11:10am -0500,
> Mike Snitzer <snitzer at redhat.com> wrote:
> 
> > On Wed, Nov 05 2014 at  9:37am -0500,
> > Mikulas Patocka <mpatocka at redhat.com> wrote:
> > 
> > > This is not about performance, it is about unclear behavior.
> > > 
> > > If someone does internal_suspend followed by remove, what should be the 
> > > correct behavior? The current code deadlocks in this case.
> > 
> > You need to be specific, if internal suspend was used by the thin-pool
> > suspend to suspend thin devices you'll need the thin-pool resumed before
> > you can remove any thin device!
> >
> > Like any interface there is a right way and a wrong way to use it.
> > dm_internal_suspend must always be followed by dm_internal_resume.
> > I cannot yet see a hole where properly written code is exposed here.
> 
> But thinking further about what you said, you're correctly concerned
> about the potential for dm-stats to have used dm_internal_suspend and
> then someone attempting to remove that device while it is internally
> suspended.

As we discussed (but for benefit of others): dm-stats does the
dm_internal_suspend+dm_internal_resume within a single DM message
(ioctl) so there is no potential for race with device delete.

> We just need a patch like this:

That untested patch was flawed, caused deadlock due to wait_on_bit()
while holding _hash_lock -- so resume ioctl wasn't possible.

So I'm still really not sure what your original point was about "If
someone does internal_suspend followed by remove, what should be the
correct behavior? The current code deadlocks in this case."

There is no deadlock:

dmsetup suspend thin-pool
dmsetup remove thin-thin1
dmsetup resume thin-pool

the thin-pool suspend will internal suspend thin-thin1, but I can still
remove thin-thin1 (provided it isn't in-use), and later resuming the
thin-pool works fine too.