Re: [PATCH v3 0/8] ovl: only sync what was written through the overlay

[Josef Bacik <josef@toxicpanda.com>]

On Fri, Nov 06, 2026 at 04:32:50PM +0100, Christian Brauner wrote:
> The thing that's a bit ugly is that overlayfs open-codes the tail of
> sync_filesystems(): ovl_sync_fs() calling upper_sb->s_op->sync_fs() and
> sync_blockdev() itself is one more place that needs to know what
> sync_filesystem() does.
>
> So we should probably add a helper to fs/sync.c for the ->sync_fs() and
> block device part and use that.

v4 went out a couple of hours before this, it only swaps in v2 of Amir's
patch, so all of this applies to it as well.

I've added sync_fs_and_bdev(sb, wait) for v5, sync_filesystem() calls it
twice and ovl_sync_fs() once.

> Eheeeem, isn't that kind of a bad proxy for this? What ->sync_fs() +
> block device do is a property of the upper filesystems and how is that
> related to RENAME_WHITEOUT? There's a few counter examples you even
> give:

[...]

> So on ubifs a syncfs() in relaxed mode returns with metadata still in
> memory. Meh. I know the flag got droppped but that was an overlayfs
> speicfic thing. A generic property of ->sync_fs() is a different thing?

I dropped the flag after v1 because nobody liked it, me included, and I
don't want to touch every file system for this.

What relaxed leaves to ->sync_fs() and the block device is only the
upper inodes that have no overlay inode, that's whiteouts and the work
directories. With creation fixed (below) everything else is on the dirty
list and gets written.

For the file systems that pass the probe: ext4, xfs and btrfs log all
metadata. ext4 without a journal has it in the buffers of the block
device, which we write. f2fs writes its dirty inodes in the checkpoint,
f2fs_sync_inode_meta(). ubifs writes names, whiteouts and links to its
journal when they are made, only setattr leaves an inode dirty in the
VFS. The only setattr overlayfs does on an inode it doesn't track is the
chmod of the work directory to 0 at mount time, right after it was
created with mode 0. That's the counter example from the cover letter,
and it's the only one I could find.

Anybody who needs more than that has fsync=on and fsync=strict, which
sync the whole upper file system like before.

> Afaict, creation isn't covered. ovl_instantiate() ends up in
> ovl_inode_init() which now uses __ovl_copyattr(). So a new file,
> directory, symlink or hardlink only dirties the parent but not the new
> overlay inode.
>
> Fine for metadata, but a long symlink isn't metadata. We should mark the
> new inode dirty where it's instantiated.

Yup, ovl_instantiate() marks it in v5. Nothing gets lost today, ext4
journals the block of a long symlink, xfs logs it, btrfs has it in the
tree and f2fs writes it in f2fs_symlink(), but we shouldn't depend on
that. I've added a long symlink to the fstest.

> * ovl_park_inode() calls mark_inode_dirty_sync(). Jan's deferred inode
>   reclaim (cf. [1]) removes exactly that from iput() because it does IO
>   from dentry reclaim.
>
>   He replaces it with queue_dirtytime_writeback(). That moves the inode
>   to the head of b_dirty_time and wakes the flusher.
>
> So it would be nice if we could base your thing on top of Jan's series
> (I hope he'll send v2 soon!) and call queue_dirtytime_writeback() thing.

The one in ovl_park_inode() is called from ->write_inode(), not from
iput(), and it's on the overlay inode, which has no ->dirty_inode(), so
it only moves it to another list.

The one that does IO from dentry reclaim is iput() of an overlay inode
with I_DIRTY_TIME, that's sync_lazytime() -> ovl_sync_lazytime() ->
mark_inode_dirty_sync(upper). Jan's patch 2 takes that out of iput(), so
it goes away without any change on my side, and then using
queue_dirtytime_writeback() in ovl_park_inode() makes sense too. It
would have to be exported for that.

I only need patches 1 and 2 of that series, and they don't depend on the
rest of it. Jan, could those two go in ahead of the deferred reclaim
part? Then I'll rebase on top of them. Otherwise I'd rather do this as a
follow up than wait for the whole thing.

> > An overlay inode that loses its last reference while it is dirty is
> > kept until it has been written and then sits on the LRU, like with
> > any other file system.
>
> Hm, Jan's series fixes that as well afaict. :)

I don't see how, what am I missing? overlayfs has inode_just_drop() for
->drop_inode(), so a dirty overlay inode is evicted on the last iput()
and we forget that the upper inode has to be written. Patch 6 keeps it
around until it's clean.

> There's some Sashiko thing btw you should look into?

The only mails I got from it are on v1, those are fixed since v2, except
for dirtied_time_when of a parked inode, which I left alone on purpose,
it's in the v2 changelog. Thanks,

Josef