Hi, Yuezhang,
> xfstests generic/482 tests the file system consistency after each
> FUA operation. It fails when run on exfat.
>
> exFAT clears the volume dirty flag with a FUA operation during sync.
> Since s_lock is not held when data is being written to a file, sync
> can be executed at the same time. When data is being written to a
> file, the FAT chain is updated first, and then the file size is
> updated. If sync is executed between updating them, the length of the
> FAT chain may be inconsistent with the file size.
>
> To avoid the situation where the file system is inconsistent but the
> volume dirty flag is cleared, this commit moves the clearing of the
> volume dirty flag from exfat_fs_sync() to exfat_put_super(), so that
> the volume dirty flag is not cleared until unmounting. After the
> move, there is no additional action during sync, so exfat_fs_sync()
> can be deleted.
It doesn't seem like FUA is the core issue. To set the volume to a clear
state in sync_filesystem, it might be possible to block the writer_iter,
mkwrite, and truncate operations.
However, as of now, it seems that moving to put_super is the simplest
and most reliable method, and FAT-fs is currently operating in that manner.
However, it seems that a modification is also needed to keep the state
dirty if it is already dirty at the time of mount, as in the FAT-fs below.
commit b88a105802e9 ("fat: mark fs as dirty on mount and clean on umount")
Could you send additional patches along with this patch as a series?
>
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@xxxxxxxx>
> ---
> fs/exfat/super.c | 30 +++++++-----------------------
> 1 file changed, 7 insertions(+), 23 deletions(-)
>
> diff --git a/fs/exfat/super.c b/fs/exfat/super.c
> index 8465033a6cf0..7ed858937d45 100644
> --- a/fs/exfat/super.c
> +++ b/fs/exfat/super.c
> @@ -36,31 +36,12 @@ static void exfat_put_super(struct super_block *sb)
> struct exfat_sb_info *sbi = EXFAT_SB(sb);
>
> mutex_lock(&sbi->s_lock);
> + exfat_clear_volume_dirty(sb);
> exfat_free_bitmap(sbi);
> brelse(sbi->boot_bh);
> mutex_unlock(&sbi->s_lock);
> }
>
> -static int exfat_sync_fs(struct super_block *sb, int wait)
> -{
> - struct exfat_sb_info *sbi = EXFAT_SB(sb);
> - int err = 0;
> -
> - if (unlikely(exfat_forced_shutdown(sb)))
> - return 0;
> -
> - if (!wait)
> - return 0;
> -
> - /* If there are some dirty buffers in the bdev inode */
> - mutex_lock(&sbi->s_lock);
> - sync_blockdev(sb->s_bdev);
> - if (exfat_clear_volume_dirty(sb))
> - err = -EIO;
> - mutex_unlock(&sbi->s_lock);
> - return err;
> -}
> -
> static int exfat_statfs(struct dentry *dentry, struct kstatfs *buf)
> {
> struct super_block *sb = dentry->d_sb;
> @@ -219,7 +200,6 @@ static const struct super_operations exfat_sops = {
> .write_inode = exfat_write_inode,
> .evict_inode = exfat_evict_inode,
> .put_super = exfat_put_super,
> - .sync_fs = exfat_sync_fs,
> .statfs = exfat_statfs,
> .show_options = exfat_show_options,
> .shutdown = exfat_shutdown,
> @@ -751,10 +731,14 @@ static void exfat_free(struct fs_context *fc)
>
> static int exfat_reconfigure(struct fs_context *fc)
> {
> + struct super_block *sb = fc->root->d_sb;
> fc->sb_flags |= SB_NODIRATIME;
>
> - /* volume flag will be updated in exfat_sync_fs */
> - sync_filesystem(fc->root->d_sb);
> + sync_filesystem(sb);
> + mutex_lock(&EXFAT_SB(sb)->s_lock);
> + exfat_clear_volume_dirty(sb);
> + mutex_unlock(&EXFAT_SB(sb)->s_lock);
> +
> return 0;
> }
>
> --
> 2.43.0
