Re: [PATCH] dm: Fix deadlock when reloading a multipath table

[Benjamin Marzinski <bmarzins@redhat.com>]

On Wed, Oct 22, 2025 at 04:11:58PM +0200, Martin Wilck wrote:
> On Tue, 2025-10-21 at 15:16 -0400, Benjamin Marzinski wrote:
> > On Fri, Oct 10, 2025 at 12:19:51PM +0200, Martin Wilck wrote:
> > > On Thu, 2025-10-09 at 18:25 -0400, Benjamin Marzinski wrote:
> > > 
> > > 
> > > > I did check to see if holding it for the entire suspend would
> > > > cause
> > > > issues, but I didn't see any case where it would. If I missed a 
> > > > case where __noflush_suspending() should only return true if we
> > > > are
> > > > actually in the process of suspending, I can easily update that
> > > > function to do that.
> > > 
> > > If this is necessary, I agree that the flag an related function
> > > should
> > > be renamed. But there are already generic DM flags to indicate that
> > > a
> > > queue is suspend*ed*. Perhaps, instead of changing the semantics of
> > > DMF_NOFLUSH_SUSPENDING, it would make more sense to test 
> > > 
> > >   (__noflush_suspending || test_bit(DMF_BLOCK_IO_FOR_SUSPEND)
> > > 
> > > in dm_swap_table()?
> > 
> > Won't we ALWAYS be suspended when we are in dm_swap_table()? We do
> > need
> > to refresh the limits in some cases (the cases where multipath-tools
> > currently reloads the table without setting noflush). What we need to
> > know is "is this table swap happening in a noflush suspend, where
> > userspace understands that it can't modify the device table in a way
> > that would change the limits". For multipath, this is almost always
> > the
> > case. 
> 
> Ok, getting it now. The semantics of the flag are changed from "device
> is noflush-suspending" to "device is either noflush-suspending or
> noflush-suspended". It isn't easy to express this in a simple flag
> name. I'm fine with not renaming the flag, if a comment is added that
> explains the semantics clearly.
> 
> > > 
> > > I find Bart's approach very attractive; freezing might not be
> > > necessary
> > > at all in that case. We'dd just need to avoid a race where paths
> > > get
> > > reinstated while the operation that would normally have required a
> > > freeze is ongoing.
> > 
> > I agree. Even just the timing out of freezes, his
> > "[PATCH 2/3] block: Restrict the duration of sysfs attribute changes"
> > would be enough to keep this from deadlocking the system.
> > 
> 
> OK, let's see how it goes. Given your explanations, I'm ok with your
> patch, too.

I see Mikulas pulled this commit into linux-dm. Bart, does this solve
your issue? Looking at your hang, it should. Also, do you have any
interest in attempting again to get your fixes upstream?

-Ben

> 
> Martin