Re: [PATCH 1/2] ntfs: fix volume flag update races

[liubaolin <liubaolin12138@163.com>]

在 2026/9/3 14:37, Hongling Zeng 写道:
> Thanks for the careful reading
> 
> These two sites cannot actually race ntfs_sync_fs(): ntfs_reconfigure() 
> (via do_remount()) and ntfs_put_super() (via generic_shutdown_super()) 
> both run with sb->s_umount
> held for write, while the sync(2)/syncfs(2) paths only take it for read 
> through iterate_supers(). So the check and the clear inside 
> ntfs_clear_volume_flags() are
> effectively a single non-interleavable pair with respect to sync — the 
> window is not merely narrow but structurally closed.
> 
> We deliberately keep ntfs_clear_volume_dirty_if_no_errors() for the 
> paths that can genuinely race sync (currently only ntfs_sync_fs() 
> itself), so converting these two would
> blur that line rather than improve consistency.

Hi Hongling,

    Thank you for the explanation.
    However, I believe we may be discussing different races.
    Your explanation addresses why ntfs_reconfigure()/ntfs_put_super() 
cannot run concurrently with ntfs_sync_fs().
    My concern is whether these paths can race with an NTFS writer/error 
path while checking NVolErrors(vol) and clearing VOLUME_IS_DIRTY.

   For ntfs_put_super(), I agree that no change is needed.
   It runs only during the final filesystem shutdown, when ordinary 
userspace writers and related I/O can no longer run concurrently with it.

   For a normal read-only remount, reconfigure_super() calls 
sb_prepare_remount_readonly(). This blocks new mount writers and checks 
for existing writers; if writers are present, the remount does not 
proceed. Therefore, the normal remount-ro path does not have the 
concurrency issue I was concerned about.

   However, an emergency/forced remount uses SB_FORCE and skips 
sb_prepare_remount_readonly(), calling only sb_start_ro_state_change(). 
This does not wait for writers that have already entered the filesystem. 
Such writers use sb->s_writers and do not need to acquire s_umount.
The following sequence is therefore possible:
NTFS writer                         forced remount
   -----------                         -------------
   file_start_write()
   Enter the NTFS write path
   Set or prepare to set VOLUME_IS_DIRTY
   Pause
                                       Acquire s_umount for writing
                                       SB_FORCE
                                       sb_start_ro_state_change()
                                       ntfs_reconfigure()
                                         sync_filesystem()
                                         NVolErrors() == false
                                         Clear VOLUME_IS_DIRTY
                                       Set SB_RDONLY
   Resume the writer
   Continue modifying metadata
   An error occurs
   NVolSetErrors(vol)

The final state may be:
   NVolErrors(vol) == true
   VOLUME_IS_DIRTY == false

   Therefore, the s_umount locking relationship only rules out 
concurrency with ntfs_sync_fs(); it does not rule out concurrency 
between a forced/emergency remount and an NTFS writer that is already in 
progress.

   Please consider whether this case also needs to be handled. If you 
agree with my analysis, I suggest adding a follow-up patch to address 
this race.
   This is my current understanding of the issue. I welcome further 
discussion, and please feel free to correct me if any part of my 
analysis is inaccurate.

Thanks,
Baolin.