On Wed, Nov 05 2014 at 11:56am -0500, Mike Snitzer <snitzer@xxxxxxxxxx> wrote: > On Wed, Nov 05 2014 at 11:10am -0500, > Mike Snitzer <snitzer@xxxxxxxxxx> wrote: > > > On Wed, Nov 05 2014 at 9:37am -0500, > > Mikulas Patocka <mpatocka@xxxxxxxxxx> 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. -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
