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
