On Sun, Jul 28, 2024 at 03:41:01PM GMT, Oleg Nesterov wrote: > Sorry for late reply. > > You do not need my help, I know nothing about fs ;) but just in case... > > On 07/26, Mateusz Guzik wrote: > > > > Welp. > > > > syzbot did the bisect, it's not any of the above, instead: > > > > commit b62e71be2110d8b52bf5faf3c3ed7ca1a0c113a5 > > Author: Chao Yu <chao@xxxxxxxxxx> > > Date: Sun Apr 23 15:49:15 2023 +0000 > > > > f2fs: support errors=remount-ro|continue|panic mountoption > > > > https://lore.kernel.org/linux-fsdevel/0000000000004ff2dc061e281637@xxxxxxxxxx/T/#m90c03813e12e5cdff1eeada8f9ab581d5f039c76 > > > > That said, the stuff I mentioned still looks highly suspicious so I > > have to something to investigate regardless. > > Did you see the patch from Hillf ? > https://lore.kernel.org/all/20240727011616.2144-1-hdanton@xxxxxxxx/ > it seems to fix the problem... I only skimmed the bisect while I was entertaining a toddler during a trainride. :) > > Of course I don't understand this patch, but afaics SB_RDONLY can confuse > thaw_super_locked(). If sb_rdonly() is true, thaw_super_locked() assumes > that freeze_super() didn't call sb_wait_write() -> percpu_down_write(). > > So in this case thaw_super_locked() just clears sb->s_writers.frozen and > goes to the "out_deactivate" label bypassing sb_freeze_unlock(). So the thing is that remounting a superblock read-only directly without going through the VFS first is pretty risky and is likely to cause trouble. The order of operations to transition rw->ro: (1) Check that the filsystem is unfrozen. If not, fail with EBUSY. (2) Mark all mounts of that filesystem as read-only. Fail with EBUSY if there are any active writers on any of the mounts. (3) Call into ->reconfigure() method of the filesystem so it can do it's internal thing to mount read-only. All of that requires sb->s_umount semaphore to be held. But f2fs seems to transition rw->ro without holding sb->s_umount in various callsites of: f2fs_stop_checkpoint() -> f2fs_handle_critical_error() So that would mean freeze_super() would've indeed acquired percpu_down_write(SB_FREEZE_WRITE+SB_FREEZE_PAGEFAULT+SB_FREEZE_FS) but thaw_super() would skip calling sb_freeze_unlock(SB_FREEZE_FS+SB_FREEZE_PAGEFAULT+SB_FREEZE_WRITE) and destroying the superblock during umount with an imbalance that gets noticed in rcu_sync_dtor(). So afaict this would mean that we never called rcu_sync_exit() and thus never set GP_EXIT and notice that imbalance during rcu_sync_dtor().
