On Thu, Nov 30, 2023 at 02:17:11PM -0500, Brian Foster wrote: > bcachefs grabs s_umount and sets SB_RDONLY when the fs is shutdown > via the ioctl() interface. This has a couple issues related to > interactions between shutdown and freeze: > > 1. The flags == FSOP_GOING_FLAGS_DEFAULT case is a deadlock vector > because freeze_bdev() calls into freeze_super(), which also > acquires s_umount. > > 2. If an explicit shutdown occurs while the sb is frozen, SB_RDONLY > alters the thaw path as if the sb was read-only at freeze time. > This effectively leaks the frozen state and leaves the sb frozen > indefinitely. > > The usage of SB_RDONLY here goes back to the initial bcachefs commit > and AFAICT is simply historical behavior. This behavior is unique to > bcachefs relative to the handful of other filesystems that support > the shutdown ioctl(). Typically, SB_RDONLY is reserved for the > proper remount path, which itself is restricted from modifying > frozen superblocks in reconfigure_super(). Drop the unnecessary sb > lock and flags update bch2_ioc_goingdown() to address both of these > issues.
Nice, I noticed a deadlock issue recently with s_umount here but didn't have time to fully debug it - applied!
