Re: [PATCH v2 1/1] writeback: don't block sync(2) for filesystems with no data integrity guarantees

[Joanne Koong <joannelkoong@gmail.com>]

On Thu, Mar 19, 2026 at 2:26 PM David Hildenbrand (Arm)
<david@kernel.org> wrote:
>
> On 3/19/26 20:45, Joanne Koong wrote:
> > Add a SB_I_NO_DATA_INTEGRITY superblock flag for filesystems that cannot
> > guarantee data persistence on sync (eg fuse). For superblocks with this
> > flag set, sync(2) kicks off writeback of dirty inodes but does not wait
> > for the flusher threads to complete the writeback.
> >
> > This replaces the per-inode AS_NO_DATA_INTEGRITY mapping flag added in
> > commit f9a49aa302a0 ("fs/writeback: skip AS_NO_DATA_INTEGRITY mappings
> > in wait_sb_inodes()"). The flag belongs at the superblock level because
> > data integrity is a filesystem-wide property, not a per-inode one.
> > Having this flag at the superblock level allows us to skip the logic in
> > sync_inodes_sb() entirely, rather than iterating every dirty inode in
> > wait_sb_inodes() only to skip each inode individually.
>
> Makes sense to me.
>
> [...]
>
> >       if (sb->s_user_ns != &init_user_ns)
> >               sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER;
> >       sb->s_flags &= ~(SB_NOSEC | SB_I_VERSION);
> > diff --git a/fs/sync.c b/fs/sync.c
> > index 942a60cfedfb..aedbf723830a 100644
> > --- a/fs/sync.c
> > +++ b/fs/sync.c
> > @@ -73,7 +73,12 @@ EXPORT_SYMBOL(sync_filesystem);
> >
> >  static void sync_inodes_one_sb(struct super_block *sb, void *arg)
> >  {
> > -     if (!sb_rdonly(sb))
> > +     if (sb_rdonly(sb))
> > +             return;
> > +
>
> Should we move some of the comment you deleting over here?

That's a good idea - I'll bring back the comment block that was
previously in wait_sb_inodes() and move it here

>
> > +     if (sb->s_iflags & SB_I_NO_DATA_INTEGRITY)
> > +             wakeup_flusher_threads_bdi(sb->s_bdi, WB_REASON_SYNC);
> > +     else
> >               sync_inodes_sb(sb);
> >  }
> I was wondering whether that handling should be moved to
> sync_inodes_sb(), so it would catch any (existing+future) callers.
>
> Alternatively, we could catch abuse by adding a warning to sync_inodes_sb.

Thinking about this some more, I think you're right.

At the vfs layer, the only other relevant callers are
generic_shutdown_super() when handling an unmount and syncfs for
syncing a specific fd. For these, I was thinking it'd be more useful
to return after writeback has actually completed, since the caller has
to directly invoke the operation on a specific fd/mount. But that's
wrong, we shouldn't be picking and choosing which syncs are ok vs not
ok to wait for. And I'm realizing now there's also the
hibernate/suspend call paths that call filesystems_freeze_callback()
which can also call into sync_filesystem() -> sync_inodes_sb()...

I'll make this change for v3.

Thanks,
Joanne

>
> --
> Cheers,
>
> David