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. Martin
