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

[liubaolin]

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