[PATCH v2 00/10] ovl: only sync what was written through the overlay

[PATCH v2 00/10] ovl: only sync what was written through the overlay

[Josef Bacik]

v1: https://lore.kernel.org/all/20261027-b4-ovl-containerized-sync-v1-0-802e512379a6@toxicpanda.com/

Jan, Amir suggested I ask you to look at the writeback and lazytime
parts again, that's patches 4 to 6 and 9.

v1->v2:
- Dropped the superblock flag and the ext4, xfs and btrfs patches. This
  is all in overlayfs now and works the same on any upper file system.
  (Christoph, Amir)
- Upper inodes without an overlay inode get a fake one that goes on the
  dirty list, patches 7 to 9.
- The periodic flusher pass doesn't write the upper inode any more, and
  syncfs() doesn't make ext4 commit per inode.
- Record write errors in s_wb_err of the overlay.
- Make the root inode appear hashed so it can be on the dirty list,
  don't mark unhashed inodes dirty, read i_writecount with acquire.
  (Sashiko)
- Left alone: dirtied_time_when isn't set for an overlay inode that gets
  I_DIRTY_TIME from ->write_inode(). It only matters for the 12 hour
  expiry, where the upper file system writes the timestamps on its own.
  (Sashiko)
- Rewrote my commit messages. (Christoph)

What it does now:

sync_filesystem() writes every inode on a dirty list, calls ->sync_fs()
and writes the block device. Only the first step is expensive, because
of other people's data. So overlayfs writes the upper inodes that were
modified through this overlay and does the other two steps as they are.
No file system has to promise anything for that.

Upper inodes with an overlay inode are tracked as in v1. The rest,
whiteouts, index entries, the work directories, are caught in the
ovl_do_*() helpers, which know what upper inodes they touch. If the VFS
considers such an inode dirty it gets a fake overlay inode, hashed by
the upper inode like a trap inode, and from there it's an ordinary dirty
overlay inode.

Things I know about:

 - An upper inode can have an overlay inode and a fake one. After
   creating 2000 files and a chown -R of another 2000 there are twice as
   many overlay inodes cached on ext4 and ext2, and 1.5 times as many on
   xfs and btrfs. The fake ones can be reclaimed once they're written.
 - With nobody else's data to skip syncfs() gets a bit slower, after the
   above it went from 49ms to 62ms on ext4 and from 96ms to 154ms on
   xfs.
 - ext2 and jfs can dirty the inode of the quota file from ->sync_fs(),
   that now waits for the flusher of the upper file system.
 - On a file system without a journal the neighbours' unwritten inodes
   show up as bitmap and count differences in fsck after a crash.

The testing is the same as for v1, now also on ext4 without a journal
and on ext2 with [2] applied. syncfs() goes from 226-268ms to 45-50ms
and unmount from 114-238ms to 37-43ms, on all five. fstests with
-overlay gives the same results with and without the series on ext4 and
on xfs.

[2] https://lore.kernel.org/r/20261027-b4-ext2-dirsync-v1-1-3dcd012d518d@toxicpanda.com

--- Original email (v1) ---

Hello,

This picks up Chengguang Xu's "implement containerized syncfs for
overlayfs", last posted as RFC V6 in November 2021 [1].

syncfs() on an overlay and unmounting one call sync_filesystem() on the
upper file system, so they write every dirty inode on it no matter who
dirtied it. We run a lot of containers on one upper file system, and
stopping one of them writes out the dirty data of all the others. That
can take a very long time, and s_umount of the upper file system is held
the whole time, so anything that wants it exclusively queues up and then
everybody else queues up behind that.

With this an overlay only writes the upper inodes that were changed
through it, and then has the upper file system commit its metadata.

The V6 discussion ended with a direction but no V7, so that's what I
tried to follow:

 - V6 kept every overlay inode with an upper inode dirty for good. Jan
   and Amir didn't like the cost of walking them. Now an overlay inode
   is marked dirty when the upper inode is changed, mostly from
   ovl_copyattr() as Amir and Miklos suggested.
 - It never looks at the dirty flags of the upper inode, Miklos pointed
   out that they're cleared when writeback starts.
 - ->write_inode() goes through write_inode_now() on the upper inode,
   and keeps the overlay inode dirty while the upper inode is open for
   writing, which covers shared mappings.
 - V6 marked the inodes DONTCACHE, which Jan was worried about. This
   goes back to the v5 patch that keeps dirty overlay inodes cached.

One thing is new. V6 called wait_sb_inodes() on the upper file system to
catch writeback that was started but not finished, which waits for
everybody's writeback. Instead ->write_inode() keeps the overlay inode
dirty while the upper inode has pages tagged dirty or under writeback.
The tags don't have the problem the inode flags have, the writeback tag
is set before the dirty tag is cleared. So a clean overlay inode means
its data is on disk, and syncfs() only waits for its own inodes.

That takes care of the data. Metadata is harder, a lot of what is
changed on the upper file system has no overlay inode that could be on
the dirty list: the root of the overlay isn't hashed, and whiteouts,
index entries, the work directories and their parent have none. File
systems that log their metadata don't care, ->sync_fs() commits it. ext2
does, there a whiteout was gone after a crash. I first had overlayfs
write all of those itself and kept finding more of them, the last one
was the link count of the shared whiteout when a whiteout is removed.

So patch 1 adds a superblock flag, SB_I_OVL_USE_SYNC_FS, by which a file
system says that ->sync_fs() commits all of its metadata, and patches 2
to 4 set it for ext4 with a journal, xfs and btrfs. Without the flag on
the upper file system nothing changes, no inode is marked dirty and
syncfs() and unmount call sync_filesystem() like they always did.

Patches 5, 7, 8 and 10 are Chengguang's with changes noted in them,
patches 6 and 9 are new. I dropped Jan's Reviewed-by tags from V6 since
the patches changed.

lazytime on the upper file system needs its own handling. The flusher
leaves the timestamps in memory for up to 12 hours, so the overlay inode
can't be clean, but it can't stay dirty and pinned for that long either.
Patch 9 gives it I_DIRTY_TIME like the upper inode has, and forwards
->sync_lazytime() to the upper inode.

Things I know about:

 - 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. Clean and unlinked ones are still freed right
   away.
 - Every overlay on such an upper file system gets a bdi.
 - I haven't measured syncfs() with a very large number of dirty inodes.

For testing I put several overlays on one upper file system and dirty
192M next to the overlay under test, through another overlay and
directly. Then syncfs() or unmount the one overlay, copy the block
device as it is at that moment, mount the copy, and check that
everything that was changed through the overlay is in it and that the
192M aren't. That passes with ext4, xfs and btrfs as the upper file
system. With KASAN and lockdep on, syncfs() goes from 239-284ms to
45-48ms and unmount from 126-234ms to 39-44ms. With ext2 and ext4
without a journal the 192M are written as before.

fstests with -overlay, the auto groups of generic and overlay, gives the
same results with and without the series on ext4 and on xfs.

[1] https://lore.kernel.org/r/20211122030038.1938875-1-cgxu519@mykernel.net

Thanks,

Josef

---
Changes in v2:
- Link to v1: https://patch.msgid.link/20261027-b4-ovl-containerized-sync-v1-0-802e512379a6@toxicpanda.com

---
Chengguang Xu (4):
      ovl: setup overlayfs' private bdi
      ovl: implement overlayfs' own ->write_inode operation
      ovl: cache dirty overlayfs' inode
      ovl: implement containerized syncfs for overlayfs

Josef Bacik (6):
      ovl: make the root inode appear hashed
      ovl: mark the overlay inode dirty on modification
      ovl: handle lazytime on the upper filesystem
      ovl: look up overlay inodes with a key structure
      ovl: pass the ovl_fs to ovl_do_rename_rd()
      ovl: put modified upper inodes on the dirty list

 fs/overlayfs/copy_up.c   |   9 ++-
 fs/overlayfs/dir.c       |  11 +--
 fs/overlayfs/file.c      |   7 +-
 fs/overlayfs/inode.c     | 185 ++++++++++++++++++++++++++++++++++++++++++++---
 fs/overlayfs/overlayfs.h | 147 ++++++++++++++++++++++++++++++++-----
 fs/overlayfs/ovl_entry.h |   2 +
 fs/overlayfs/params.c    |  14 +---
 fs/overlayfs/super.c     | 175 +++++++++++++++++++++++++++++++++++++++++---
 fs/overlayfs/util.c      | 108 ++++++++++++++++++++++++++-
 9 files changed, 598 insertions(+), 60 deletions(-)
---
base-commit: 2c8a968b488c134d05ccefff890270ac8ab899a8
change-id: 20261026-b4-ovl-containerized-sync-149bed1afe9a

[PATCH v2 01/10] ovl: setup overlayfs' private bdi

[Josef Bacik]

From: Chengguang Xu <charliecgxu@tencent.com>

Setup overlayfs' private bdi so that we can collect
overlayfs' own dirty inodes.

Signed-off-by: Chengguang Xu <charliecgxu@tencent.com>
[josef: only for an overlay that tracks dirty inodes, add
 ovl_tracks_dirty()]
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/overlayfs/overlayfs.h | 17 +++++++++++++++++
 fs/overlayfs/super.c     |  7 +++++++
 2 files changed, 24 insertions(+)

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 92da556598da..aa6c2d005a40 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -7,6 +7,7 @@
 #include <linux/kernel.h>
 #include <linux/uuid.h>
 #include <linux/fs.h>
+#include <linux/backing-dev.h>
 #include <linux/fsverity.h>
 #include <linux/namei.h>
 #include <linux/posix_acl.h>
@@ -663,6 +664,22 @@ static inline bool ovl_should_sync(struct ovl_fs *ofs)
 	return ofs->config.fsync_mode != OVL_FSYNC_VOLATILE;
 }
 
+/*
+ * Does this overlay track the upper inodes that are modified through it, so
+ * that syncfs() and unmount only have to sync those?
+ *
+ * There is nothing to track without an upper filesystem, and a volatile overlay
+ * never syncs.  sync_filesystem() does not write any inodes of a filesystem
+ * that has no writeback, like tmpfs, and nothing ever clears their dirty flags.
+ */
+static inline bool ovl_tracks_dirty(struct ovl_fs *ofs)
+{
+	if (!ovl_upper_mnt(ofs) || !ovl_should_sync(ofs))
+		return false;
+
+	return ovl_upper_mnt(ofs)->mnt_sb->s_bdi != &noop_backing_dev_info;
+}
+
 static inline bool ovl_should_sync_metadata(struct ovl_fs *ofs)
 {
 	return ofs->config.fsync_mode == OVL_FSYNC_STRICT;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 19b6d54c4500..ce3e30235ae9 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1455,6 +1455,13 @@ static int ovl_fill_super_creds(struct fs_context *fc, struct super_block *sb)
 		if (err)
 			return err;
 
+		/* Dirty overlay inodes go on the lists of this bdi */
+		if (ovl_tracks_dirty(ofs)) {
+			err = super_setup_bdi(sb);
+			if (err)
+				return err;
+		}
+
 		upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
 		if (!ovl_should_sync(ofs)) {
 			ofs->errseq = errseq_sample(&upper_sb->s_wb_err);

-- 
2.55.0

[PATCH v2 02/10] ovl: make the root inode appear hashed

[Josef Bacik]

__mark_inode_dirty() does not put an unhashed inode on a dirty list, and
the root inode of an overlay is never hashed.  Use inode_fake_hash() on it,
so that the following patches can track it like any other overlay inode.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/overlayfs/super.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index ce3e30235ae9..f0ca1539022d 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1366,6 +1366,11 @@ static struct dentry *ovl_get_root(struct super_block *sb,
 	ovl_dentry_set_flag(OVL_E_CONNECTED, root);
 	ovl_set_upperdata(d_inode(root));
 	ovl_inode_init(d_inode(root), &oip, ino, fsid);
+	/*
+	 * Nobody looks the root up by its hash, but __mark_inode_dirty() only
+	 * puts hashed inodes on a dirty list.
+	 */
+	inode_fake_hash(d_inode(root));
 	WARN_ON(!!IS_CASEFOLDED(d_inode(root)) != ofs->casefold);
 	ovl_dentry_init_flags(root, upperdentry, oe, DCACHE_OP_WEAK_REVALIDATE);
 	/* root keeps a reference of upperdentry */

-- 
2.55.0

[PATCH v2 03/10] ovl: mark the overlay inode dirty on modification

[Josef Bacik]

To sync only what was written through an overlay, we have to know which
upper inodes were modified through it.  Mark the overlay inode dirty when
its upper inode is modified, which puts it on the dirty list of the
overlay's bdi.

Nearly every modification ends in ovl_copyattr(), so do it there.  Copy up,
the data copy up of a metacopy inode, atime updates and shared writable
mappings do not, so they mark the inode themselves.

Do not mark an unhashed inode.  __mark_inode_dirty() would set its dirty
flags without queueing it, and every later call would return early.  The
inode of a lower hardlink is unhashed until ovl_inode_update() hashes it.

Based on work by Chengguang Xu.

Link: https://lore.kernel.org/r/20211122030038.1938875-1-cgxu519@mykernel.net
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Suggested-by: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/overlayfs/copy_up.c   |  5 +++++
 fs/overlayfs/file.c      |  7 ++++++-
 fs/overlayfs/inode.c     |  4 +++-
 fs/overlayfs/overlayfs.h | 20 ++++++++++++++++++++
 fs/overlayfs/util.c      | 14 +++++++++++++-
 5 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index e963701b4c87..40486d2a4b30 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -1122,6 +1122,11 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
 	ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
 	ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry));
 	ovl_set_upperdata(d_inode(c->dentry));
+	/*
+	 * The data was written to the upper inode, and there may be no later
+	 * write that would mark the overlay inode dirty.
+	 */
+	ovl_mark_inode_dirty(d_inode(c->dentry));
 out_free:
 	kfree(capability);
 out:
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 7433220d4ad6..53c5131a0482 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -472,8 +472,13 @@ static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
 		.cred = ovl_creds(file_inode(file)->i_sb),
 		.accessed = ovl_file_accessed,
 	};
+	int err;
 
-	return backing_file_mmap(of->realfile, vma, &ctx);
+	err = backing_file_mmap(of->realfile, vma, &ctx);
+	/* Stores through the mapping go straight to the upper inode */
+	if (!err && vma_is_shared_maywrite(vma))
+		ovl_mark_inode_dirty(file_inode(file));
+	return err;
 }
 
 static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 63906b3dafcb..9a4bf7c2502b 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -587,6 +587,7 @@ int ovl_update_time(struct inode *inode, enum fs_update_time type,
 			touch_atime(&upperpath);
 			inode_set_atime_to_ts(inode,
 					      inode_get_atime(d_inode(upperpath.dentry)));
+			ovl_mark_inode_dirty(inode);
 		}
 	}
 	return 0;
@@ -898,7 +899,8 @@ void ovl_inode_init(struct inode *inode, struct ovl_inode_params *oip,
 	oi->lowerdata_redirect = oip->lowerdata_redirect;
 
 	realinode = ovl_inode_real(inode);
-	ovl_copyattr(inode);
+	/* Nothing was changed, the inode is only being looked up */
+	__ovl_copyattr(inode);
 	ovl_copyflags(realinode, inode);
 	ovl_map_ino(inode, ino, fsid);
 }
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index aa6c2d005a40..101de0dd07d8 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -680,6 +680,25 @@ static inline bool ovl_tracks_dirty(struct ovl_fs *ofs)
 	return ovl_upper_mnt(ofs)->mnt_sb->s_bdi != &noop_backing_dev_info;
 }
 
+/*
+ * The upper inode was modified through this overlay.  Put the overlay inode on
+ * the dirty list of the overlay, where syncfs() and unmount find it.
+ *
+ * Do not look at the dirty flags of the upper inode here.  They are cleared
+ * when the writeback of the upper inode starts, not when it completes.
+ *
+ * Do not mark an inode that is not hashed.  __mark_inode_dirty() would set its
+ * dirty flags without putting it on a dirty list, and once the flags are set,
+ * later calls return early.  The overlay inode of a lower hardlink is unhashed
+ * until ovl_inode_update() hashes it, and ovl_inode_update() marks it after
+ * that.
+ */
+static inline void ovl_mark_inode_dirty(struct inode *inode)
+{
+	if (ovl_tracks_dirty(OVL_FS(inode->i_sb)) && !inode_unhashed(inode))
+		mark_inode_dirty(inode);
+}
+
 static inline bool ovl_should_sync_metadata(struct ovl_fs *ofs)
 {
 	return ofs->config.fsync_mode == OVL_FSYNC_STRICT;
@@ -877,6 +896,7 @@ bool ovl_lookup_trap_inode(struct super_block *sb, struct dentry *dir);
 struct inode *ovl_get_trap_inode(struct super_block *sb, struct dentry *dir);
 struct inode *ovl_get_inode(struct super_block *sb,
 			    struct ovl_inode_params *oip);
+void __ovl_copyattr(struct inode *to);
 void ovl_copyattr(struct inode *to);
 
 /* vfs fileattr flags read from overlay.protattr xattr to ovl inode */
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 521717209b2e..f32202500b3d 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -604,6 +604,8 @@ void ovl_inode_update(struct inode *inode, struct dentry *upperdentry)
 		inode->i_private = upperinode;
 		__insert_inode_hash(inode, (unsigned long) upperinode);
 	}
+	/* The copy up has written to the upper inode */
+	ovl_mark_inode_dirty(inode);
 }
 
 static void ovl_dir_version_inc(struct dentry *dentry, bool impurity)
@@ -1492,7 +1494,7 @@ int ovl_sync_status(struct ovl_fs *ofs)
  * map any lower or upper inode owned by id 1001 to id 1000. These mapping
  * helpers are nops when the relevant layer isn't idmapped.
  */
-void ovl_copyattr(struct inode *inode)
+void __ovl_copyattr(struct inode *inode)
 {
 	struct path realpath;
 	struct inode *realinode;
@@ -1516,3 +1518,13 @@ void ovl_copyattr(struct inode *inode)
 	i_size_write(inode, i_size_read(realinode));
 	spin_unlock(&inode->i_lock);
 }
+
+/*
+ * The attributes are copied because the real inode was modified through the
+ * overlay, so this is also where the overlay inode is marked dirty.
+ */
+void ovl_copyattr(struct inode *inode)
+{
+	__ovl_copyattr(inode);
+	ovl_mark_inode_dirty(inode);
+}

-- 
2.55.0

[PATCH v2 04/10] ovl: implement overlayfs' own ->write_inode operation

[Josef Bacik]

From: Chengguang Xu <charliecgxu@tencent.com>

Sync dirty data and meta of upper inode in overlayfs' ->write_inode()
and redirty overlayfs' inode.

The overlay inode is only redirtied while the upper inode is open for
writing, dirty or under writeback.  So a clean overlay inode means that
everything written through it has been written back.

The periodic pass of the flusher does not write the upper inode, the
flusher of the upper filesystem does that.  syncfs() waits for the data and
leaves the commit to ->sync_fs(), like sync_inodes_sb() does with for_sync.

The VFS ignores the return value of ->write_inode(), so record errors in
s_wb_err of the overlay for the next syncfs().

Signed-off-by: Chengguang Xu <charliecgxu@tencent.com>
[josef: everything after the first paragraph]
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/overlayfs/overlayfs.h | 13 ++++++++
 fs/overlayfs/super.c     | 80 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/overlayfs/util.c      | 52 +++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+)

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 101de0dd07d8..8f83325ec359 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -510,6 +510,19 @@ const struct ovl_layer *ovl_layer_lower(struct dentry *dentry);
 struct dentry *ovl_dentry_real(struct dentry *dentry);
 struct dentry *ovl_i_dentry_upper(struct inode *inode);
 struct inode *ovl_inode_upper(struct inode *inode);
+
+/* How ovl_write_upper_inode() writes an upper inode */
+enum ovl_write_mode {
+	/* Start the writeback and do not wait for it */
+	OVL_WRITE_START,
+	/* Wait for the data, ->sync_fs() is going to commit the metadata */
+	OVL_WRITE_WAIT,
+	/* A write_inode_now() that waits, for a caller that is not a sync */
+	OVL_WRITE_COMMIT,
+};
+
+int ovl_write_upper_inode(struct super_block *sb, struct inode *upper,
+			  enum ovl_write_mode mode);
 struct inode *ovl_inode_lower(struct inode *inode);
 struct inode *ovl_inode_lowerdata(struct inode *inode);
 struct inode *ovl_inode_real(struct inode *inode);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index f0ca1539022d..4e06b3c772fe 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -15,6 +15,8 @@
 #include <linux/seq_file.h>
 #include <linux/posix_acl_xattr.h>
 #include <linux/exportfs.h>
+#include <linux/pagemap.h>
+#include <linux/writeback.h>
 #include <linux/file.h>
 #include <linux/fs_context.h>
 #include <linux/fs_parser.h>
@@ -313,12 +315,90 @@ static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return err;
 }
 
+/*
+ * Could there be anything left to write or to wait for on @upper?
+ *
+ * A task that has @upper open for writing may have it mapped, and overlayfs
+ * does not see stores through a mapping.  The mapping holds the file open.
+ *
+ * Writeback that has been started but not completed, by us or by the flusher of
+ * the upper filesystem, shows in the page cache tags.  It does not show in the
+ * dirty flags of the inode, which are cleared when writeback starts.
+ *
+ * So dirty flags that are clear say nothing.  Dirty flags that are set do say
+ * that there is something left to write.  @upper may not have been written at
+ * all, see ovl_write_inode().  And a write_inode_now() that does not wait does
+ * nothing if the flusher of the upper filesystem is writing @upper.  A
+ * modification that was made during that pass is only recorded in the flags.
+ *
+ * i_writecount has to be read first.  If it is seen at zero, the tag of a page
+ * that was dirtied through a mapping must be seen as well.  The tag is set when
+ * the mapping goes away at the latest.  i_writecount drops after that, when the
+ * file is released, with the full barrier of the last fput() in between.
+ */
+static bool ovl_upper_unfinished(struct inode *upper)
+{
+	struct address_space *mapping = upper->i_mapping;
+
+	return atomic_read_acquire(&upper->i_writecount) > 0 ||
+	       mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) ||
+	       mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK) ||
+	       (inode_state_read_once(upper) & (I_DIRTY | I_SYNC));
+}
+
+static enum ovl_write_mode ovl_write_mode(struct inode *inode,
+					  struct inode *upper,
+					  struct writeback_control *wbc)
+{
+	if (wbc->sync_mode != WB_SYNC_ALL)
+		return OVL_WRITE_START;
+
+	/* sync_inodes_sb() does not wait for such a filesystem either */
+	if (upper->i_sb->s_iflags & SB_I_NO_DATA_INTEGRITY)
+		return OVL_WRITE_START;
+
+	/*
+	 * syncfs() and unmount call ovl_sync_fs() next.  While the overlay is
+	 * being mounted, iput() writes a dirty inode because it cannot keep it.
+	 * Nobody has asked for that inode to be durable yet.
+	 */
+	if (wbc->for_sync || !(inode->i_sb->s_flags & SB_ACTIVE))
+		return OVL_WRITE_WAIT;
+
+	return OVL_WRITE_COMMIT;
+}
+
+static int ovl_write_inode(struct inode *inode, struct writeback_control *wbc)
+{
+	struct inode *upper = ovl_inode_upper(inode);
+	int ret = 0;
+
+	if (!upper)
+		return 0;
+
+	/*
+	 * Leave periodic and background writeback to the flusher of the upper
+	 * filesystem.  @upper is on its dirty list, and it knows when the data
+	 * is old enough to be written.  All there is to do here is to find out
+	 * whether the overlay inode can be clean again.
+	 */
+	if (!wbc->for_kupdate && !wbc->for_background)
+		ret = ovl_write_upper_inode(inode->i_sb, upper,
+					    ovl_write_mode(inode, upper, wbc));
+
+	if (ovl_upper_unfinished(upper))
+		ovl_mark_inode_dirty(inode);
+
+	return ret;
+}
+
 static const struct super_operations ovl_super_operations = {
 	.alloc_inode	= ovl_alloc_inode,
 	.free_inode	= ovl_free_inode,
 	.destroy_inode	= ovl_destroy_inode,
 	.drop_inode	= inode_just_drop,
 	.put_super	= ovl_put_super,
+	.write_inode	= ovl_write_inode,
 	.sync_fs	= ovl_sync_fs,
 	.statfs		= ovl_statfs,
 	.show_options	= ovl_show_options,
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index f32202500b3d..76e090b2a0c5 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -14,6 +14,7 @@
 #include <linux/fileattr.h>
 #include <linux/uuid.h>
 #include <linux/namei.h>
+#include <linux/pagemap.h>
 #include <linux/ratelimit.h>
 #include <linux/overflow.h>
 #include "overlayfs.h"
@@ -1528,3 +1529,54 @@ void ovl_copyattr(struct inode *inode)
 	__ovl_copyattr(inode);
 	ovl_mark_inode_dirty(inode);
 }
+
+/*
+ * Write @upper the way sync_inodes_sb() writes an inode, with for_sync set in
+ * the writeback_control: wait for the data, hand the inode to the filesystem,
+ * and leave it to ->sync_fs() to commit it.  write_inode_now() cannot be told
+ * about for_sync.  If it waits, ext4 commits a transaction for every inode.
+ */
+static int ovl_sync_upper_inode(struct inode *upper)
+{
+	int err, err2;
+
+	err = filemap_write_and_wait(upper->i_mapping);
+
+	/* A sync also writes the timestamps that lazytime defers */
+	if (inode_state_read_once(upper) & I_DIRTY_TIME)
+		mark_inode_dirty_sync(upper);
+
+	err2 = sync_inode_metadata(upper, 0);
+	/*
+	 * That does nothing if the flusher of the upper filesystem is writing
+	 * @upper, and the flusher may have looked at the inode before it was
+	 * modified.  Wait for the flusher in that case.
+	 */
+	if (inode_state_read_once(upper) & (I_DIRTY_INODE | I_SYNC))
+		err2 = sync_inode_metadata(upper, 1);
+
+	return err ? err : err2;
+}
+
+/*
+ * Write @upper for the overlay @sb.
+ *
+ * Nobody may be there to take the error.  The VFS ignores what ->write_inode()
+ * returns when it walks the dirty list.  Record the error in the superblock of
+ * the overlay, like mapping_set_error() does for a page that could not be
+ * written, so that the next syncfs() on the overlay returns it.
+ */
+int ovl_write_upper_inode(struct super_block *sb, struct inode *upper,
+			  enum ovl_write_mode mode)
+{
+	int err;
+
+	if (mode == OVL_WRITE_WAIT)
+		err = ovl_sync_upper_inode(upper);
+	else
+		err = write_inode_now(upper, mode == OVL_WRITE_COMMIT);
+
+	if (err)
+		errseq_set(&sb->s_wb_err, err);
+	return err;
+}

-- 
2.55.0

[PATCH v2 05/10] ovl: cache dirty overlayfs' inode

[Josef Bacik]

From: Chengguang Xu <cgxu519@mykernel.net>

Now drop overlayfs' inode will sync dirty data,
so we change to only drop clean inode.

The purpose of doing this is to keep compatible
behavior with before because without this change
dropping overlayfs inode will not trigger syncing
of underlying dirty inode.

Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
[josef: also keep an inode that is under writeback, rebase]
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/overlayfs/super.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 4e06b3c772fe..a68eb78a9061 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -392,11 +392,27 @@ static int ovl_write_inode(struct inode *inode, struct writeback_control *wbc)
 	return ret;
 }
 
+/*
+ * In iput_final(), clean inode will drop directly and dirty inode will
+ * keep in the cache until write back to sync dirty data then add to lru
+ * list to wait reclaim.
+ *
+ * An inode under writeback has had its dirty flags cleared, and only I_SYNC
+ * pins it.  Such an inode is not clean.  ovl_write_inode() may be about to mark
+ * it dirty again, and marking an inode dirty that is being freed has no effect.
+ */
+static int ovl_drop_inode(struct inode *inode)
+{
+	if (!(inode_state_read(inode) & (I_DIRTY_ALL | I_SYNC)))
+		return 1;
+	return inode_generic_drop(inode);
+}
+
 static const struct super_operations ovl_super_operations = {
 	.alloc_inode	= ovl_alloc_inode,
 	.free_inode	= ovl_free_inode,
 	.destroy_inode	= ovl_destroy_inode,
-	.drop_inode	= inode_just_drop,
+	.drop_inode	= ovl_drop_inode,
 	.put_super	= ovl_put_super,
 	.write_inode	= ovl_write_inode,
 	.sync_fs	= ovl_sync_fs,

-- 
2.55.0

[PATCH v2 06/10] ovl: handle lazytime on the upper filesystem

[Josef Bacik]

With lazytime on the upper filesystem the flusher leaves timestamps in
memory for up to 12 hours.  The overlay inode must not be clean during that
time, or syncfs() would miss them.  It cannot stay dirty either, because
the flusher would visit it on every pass and it could not be reclaimed.

When deferred timestamps are all that is left to write, set I_DIRTY_TIME on
the overlay inode, like the VFS does on the upper inode.  Implement
->sync_lazytime() to pass the request on to the upper inode.

The VFS writes deferred timestamps on the last iput().  The upper inode
does not see that while the overlay inode holds it, so the last iput() of
the overlay inode stands in for it.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/overlayfs/dir.c       |  1 +
 fs/overlayfs/inode.c     | 19 +++++++++++++++++++
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/super.c     | 27 +++++++++++++++++++++++++++
 4 files changed, 48 insertions(+)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 1194ccf981c3..fc47beffa67e 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1489,6 +1489,7 @@ const struct inode_operations ovl_dir_inode_operations = {
 	.get_acl	= ovl_get_acl,
 	.set_acl	= ovl_set_acl,
 	.update_time	= ovl_update_time,
+	.sync_lazytime	= ovl_sync_lazytime,
 	.fileattr_get	= ovl_fileattr_get,
 	.fileattr_set	= ovl_fileattr_set,
 	.tmpfile	= ovl_tmpfile,
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 9a4bf7c2502b..5895ee5aa771 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -743,6 +743,22 @@ int ovl_fileattr_get(struct dentry *dentry, struct file_kattr *fa)
 	return err;
 }
 
+/*
+ * The overlay inode has I_DIRTY_TIME set because the upper inode has deferred
+ * timestamps, see ovl_park_inode().  The VFS calls this when the timestamps
+ * have to be written.  Mark the upper inode dirty, which hands the timestamps
+ * to the upper filesystem, and mark the overlay inode dirty, so that
+ * ovl_write_inode() writes the upper inode.
+ */
+void ovl_sync_lazytime(struct inode *inode)
+{
+	struct inode *upper = ovl_inode_upper(inode);
+
+	if (upper && (inode_state_read_once(upper) & I_DIRTY_TIME))
+		mark_inode_dirty_sync(upper);
+	ovl_mark_inode_dirty(inode);
+}
+
 static const struct inode_operations ovl_file_inode_operations = {
 	.setattr	= ovl_setattr,
 	.permission	= ovl_permission,
@@ -752,6 +768,7 @@ static const struct inode_operations ovl_file_inode_operations = {
 	.get_acl	= ovl_get_acl,
 	.set_acl	= ovl_set_acl,
 	.update_time	= ovl_update_time,
+	.sync_lazytime	= ovl_sync_lazytime,
 	.fiemap		= ovl_fiemap,
 	.fileattr_get	= ovl_fileattr_get,
 	.fileattr_set	= ovl_fileattr_set,
@@ -763,6 +780,7 @@ static const struct inode_operations ovl_symlink_inode_operations = {
 	.getattr	= ovl_getattr,
 	.listxattr	= ovl_listxattr,
 	.update_time	= ovl_update_time,
+	.sync_lazytime	= ovl_sync_lazytime,
 };
 
 static const struct inode_operations ovl_special_inode_operations = {
@@ -774,6 +792,7 @@ static const struct inode_operations ovl_special_inode_operations = {
 	.get_acl	= ovl_get_acl,
 	.set_acl	= ovl_set_acl,
 	.update_time	= ovl_update_time,
+	.sync_lazytime	= ovl_sync_lazytime,
 };
 
 static const struct address_space_operations ovl_aops = {
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 8f83325ec359..d3a897e46ccb 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -890,6 +890,7 @@ static inline struct posix_acl *ovl_get_acl_path(const struct path *path,
 
 int ovl_update_time(struct inode *inode, enum fs_update_time type,
 		unsigned int flags);
+void ovl_sync_lazytime(struct inode *inode);
 bool ovl_is_private_xattr(struct super_block *sb, const char *name);
 
 struct ovl_inode_params {
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index a68eb78a9061..de3e2d0b29ee 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -368,6 +368,31 @@ static enum ovl_write_mode ovl_write_mode(struct inode *inode,
 	return OVL_WRITE_COMMIT;
 }
 
+/*
+ * All that is left to write for @inode are timestamps that lazytime has
+ * deferred on the upper inode.  The flusher leaves those alone for up to 12
+ * hours.  Set I_DIRTY_TIME on the overlay inode, like the VFS did on the upper
+ * inode.  This takes the overlay inode off the list that the flusher walks,
+ * syncfs() still finds it, and the VFS calls ovl_sync_lazytime() when the
+ * timestamps have to be written.
+ *
+ * One of those occasions is when the last reference to an inode goes away.  The
+ * upper inode never loses its last reference while the overlay inode exists, so
+ * the overlay inode has to stand in for it.  The last reference to the overlay
+ * inode may already be gone at this point.  iput() checks for I_DIRTY_TIME
+ * under i_lock before it drops the last reference, so either iput() sees the
+ * flag, or the reference count is zero here.
+ */
+static void ovl_park_inode(struct inode *inode)
+{
+	if (!ovl_tracks_dirty(OVL_FS(inode->i_sb)))
+		return;
+
+	__mark_inode_dirty(inode, I_DIRTY_TIME);
+	if (!icount_read_once(inode))
+		mark_inode_dirty_sync(inode);
+}
+
 static int ovl_write_inode(struct inode *inode, struct writeback_control *wbc)
 {
 	struct inode *upper = ovl_inode_upper(inode);
@@ -388,6 +413,8 @@ static int ovl_write_inode(struct inode *inode, struct writeback_control *wbc)
 
 	if (ovl_upper_unfinished(upper))
 		ovl_mark_inode_dirty(inode);
+	else if (inode_state_read_once(upper) & I_DIRTY_TIME)
+		ovl_park_inode(inode);
 
 	return ret;
 }

-- 
2.55.0

[PATCH v2 07/10] ovl: look up overlay inodes with a key structure

[Josef Bacik]

A following patch adds fake overlay inodes, which are hashed by the same
real inode as an overlay inode or a trap inode.  Pass a key with the real
inode and the kind of inode to ovl_inode_test() and ovl_inode_set(), so
that all lookups share one test function.

No functional change intended.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/overlayfs/inode.c     | 57 ++++++++++++++++++++++++++++++++++++++----------
 fs/overlayfs/overlayfs.h |  5 +++++
 2 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 5895ee5aa771..88cec47d0bc8 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -1069,17 +1069,50 @@ struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, dev_t rdev)
 	return inode;
 }
 
+/*
+ * An overlay inode is hashed by a real inode.  A real inode can have a fake
+ * overlay inode next to its overlay inode or its trap inode.
+ */
+struct ovl_inode_key {
+	struct inode *real;
+	bool fake;
+};
+
 static int ovl_inode_test(struct inode *inode, void *data)
 {
-	return inode->i_private == data;
+	struct ovl_inode_key *key = data;
+
+	return inode->i_private == key->real &&
+	       ovl_test_flag(OVL_FAKE, inode) == key->fake;
 }
 
 static int ovl_inode_set(struct inode *inode, void *data)
 {
-	inode->i_private = data;
+	struct ovl_inode_key *key = data;
+
+	inode->i_private = key->real;
+	if (key->fake)
+		ovl_set_flag(OVL_FAKE, inode);
 	return 0;
 }
 
+static struct inode *ovl_ilookup(struct super_block *sb, struct inode *real,
+				 bool fake)
+{
+	struct ovl_inode_key key = { .real = real, .fake = fake };
+
+	return ilookup5(sb, (unsigned long)real, ovl_inode_test, &key);
+}
+
+static struct inode *ovl_iget_locked(struct super_block *sb, struct inode *real,
+				     bool fake)
+{
+	struct ovl_inode_key key = { .real = real, .fake = fake };
+
+	return iget5_locked(sb, (unsigned long)real, ovl_inode_test,
+			    ovl_inode_set, &key);
+}
+
 static bool ovl_verify_inode(struct inode *inode, struct dentry *lowerdentry,
 			     struct dentry *upperdentry, bool strict)
 {
@@ -1123,7 +1156,7 @@ struct inode *ovl_lookup_inode(struct super_block *sb, struct dentry *real,
 {
 	struct inode *inode, *key = d_inode(real);
 
-	inode = ilookup5(sb, (unsigned long) key, ovl_inode_test, key);
+	inode = ovl_ilookup(sb, key, false);
 	if (!inode)
 		return NULL;
 
@@ -1142,7 +1175,7 @@ bool ovl_lookup_trap_inode(struct super_block *sb, struct dentry *dir)
 	struct inode *trap;
 	bool res;
 
-	trap = ilookup5(sb, (unsigned long) key, ovl_inode_test, key);
+	trap = ovl_ilookup(sb, key, false);
 	if (!trap)
 		return false;
 
@@ -1166,8 +1199,7 @@ struct inode *ovl_get_trap_inode(struct super_block *sb, struct dentry *dir)
 	if (!d_is_dir(dir))
 		return ERR_PTR(-ENOTDIR);
 
-	trap = iget5_locked(sb, (unsigned long) key, ovl_inode_test,
-			    ovl_inode_set, key);
+	trap = ovl_iget_locked(sb, key, false);
 	if (!trap)
 		return ERR_PTR(-ENOMEM);
 
@@ -1218,12 +1250,15 @@ static bool ovl_hash_bylower(struct super_block *sb, struct dentry *upper,
 }
 
 static struct inode *ovl_iget5(struct super_block *sb, struct inode *newinode,
-			       struct inode *key)
+			       struct inode *real)
 {
-	return newinode ? inode_insert5(newinode, (unsigned long) key,
-					 ovl_inode_test, ovl_inode_set, key) :
-			  iget5_locked(sb, (unsigned long) key,
-				       ovl_inode_test, ovl_inode_set, key);
+	struct ovl_inode_key key = { .real = real };
+
+	if (!newinode)
+		return ovl_iget_locked(sb, real, false);
+
+	return inode_insert5(newinode, (unsigned long)real, ovl_inode_test,
+			     ovl_inode_set, &key);
 }
 
 struct inode *ovl_get_inode(struct super_block *sb,
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index d3a897e46ccb..f884e6695d2c 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -66,6 +66,11 @@ enum ovl_inode_flag {
 	OVL_CONST_INO,
 	OVL_HAS_DIGEST,
 	OVL_VERIFIED_DIGEST,
+	/*
+	 * A fake inode: not the overlay inode or the trap inode of the real
+	 * inode that it is hashed by.
+	 */
+	OVL_FAKE,
 };
 
 enum ovl_entry_flag {

-- 
2.55.0

[PATCH v2 08/10] ovl: pass the ovl_fs to ovl_do_rename_rd()

[Josef Bacik]

It is the only helper that modifies the upper filesystem without getting
the ovl_fs, and the next patch needs it there.

No functional change intended.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/overlayfs/copy_up.c   |  4 ++--
 fs/overlayfs/dir.c       | 10 +++++-----
 fs/overlayfs/overlayfs.h |  5 +++--
 fs/overlayfs/super.c     |  2 +-
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 40486d2a4b30..c1e00ceab8fb 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -563,7 +563,7 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
 	if (err)
 		goto out;
 
-	err = ovl_do_rename_rd(&rd);
+	err = ovl_do_rename_rd(ofs, &rd);
 	end_renaming(&rd);
 out:
 	if (err)
@@ -819,7 +819,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
 
 	err = ovl_copy_up_metadata(c, temp);
 	if (!err)
-		err = ovl_do_rename_rd(&rd);
+		err = ovl_do_rename_rd(ofs, &rd);
 	end_renaming(&rd);
 
 	if (err)
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index fc47beffa67e..6f7a513bb5a6 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -141,7 +141,7 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir,
 	rd.flags = flags;
 	err = start_renaming_two_dentries(&rd, whiteout, dentry);
 	if (!err) {
-		err = ovl_do_rename_rd(&rd);
+		err = ovl_do_rename_rd(ofs, &rd);
 		end_renaming(&rd);
 	}
 	if (err)
@@ -449,7 +449,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
 	if (err)
 		goto out_cleanup;
 
-	err = ovl_do_rename_rd(&rd);
+	err = ovl_do_rename_rd(ofs, &rd);
 	end_renaming(&rd);
 	if (err)
 		goto out_cleanup_unlocked;
@@ -560,14 +560,14 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
 			goto out_cleanup;
 
 		rd.flags = RENAME_EXCHANGE;
-		err = ovl_do_rename_rd(&rd);
+		err = ovl_do_rename_rd(ofs, &rd);
 		end_renaming(&rd);
 		if (err)
 			goto out_cleanup_unlocked;
 
 		ovl_cleanup(ofs, workdir, upper);
 	} else {
-		err = ovl_do_rename_rd(&rd);
+		err = ovl_do_rename_rd(ofs, &rd);
 		end_renaming(&rd);
 		if (err)
 			goto out_cleanup_unlocked;
@@ -1300,7 +1300,7 @@ static int ovl_rename_upper(struct ovl_renamedata *ovlrd, struct list_head *list
 	if (err)
 		goto out_unlock;
 
-	err = ovl_do_rename_rd(&rd);
+	err = ovl_do_rename_rd(ofs, &rd);
 
 	if (!err && ovlrd->cleanup_whiteout)
 		whiteout = dget(rd.new_dentry);
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index f884e6695d2c..83e25700c8a0 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -372,7 +372,8 @@ static inline int ovl_do_remove_acl(struct ovl_fs *ofs, struct dentry *dentry,
 	return vfs_remove_acl(ovl_upper_mnt_idmap(ofs), dentry, acl_name);
 }
 
-static inline int ovl_do_rename_rd(struct renamedata *rd)
+static inline int ovl_do_rename_rd(struct ovl_fs *ofs,
+				   struct renamedata *rd)
 {
 	int err;
 
@@ -399,7 +400,7 @@ static inline int ovl_do_rename(struct ovl_fs *ofs, struct dentry *olddir,
 		.flags		= flags,
 	};
 
-	return ovl_do_rename_rd(&rd);
+	return ovl_do_rename_rd(ofs, &rd);
 }
 
 static inline int ovl_do_whiteout(struct ovl_fs *ofs,
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index de3e2d0b29ee..c43f8fdd6230 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -732,7 +732,7 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
 
 	/* Name is inline and stable - using snapshot as a copy helper */
 	take_dentry_name_snapshot(&name, temp);
-	err = ovl_do_rename_rd(&rd);
+	err = ovl_do_rename_rd(ofs, &rd);
 	end_renaming(&rd);
 	if (err) {
 		if (err == -EINVAL)

-- 
2.55.0

[PATCH v2 09/10] ovl: put modified upper inodes on the dirty list

[Josef Bacik]

Many upper inodes that overlayfs modifies have no overlay inode that could
be on the dirty list: whiteouts, index entries, the work directories.

overlayfs modifies the namespace and the attributes of the upper filesystem
through the ovl_do_*() helpers, and each helper knows which upper inodes it
touches.  Have them call ovl_upper_modified(), which gives the upper inode
a fake overlay inode and marks that dirty.  A fake inode is hashed by the
real inode and has no dentry, like a trap inode.  From there on it is an
ordinary dirty overlay inode.

An upper inode only gets a fake inode if the VFS considers it dirty,
sync_filesystem() would not write it otherwise.  The flusher clears the
dirty flags when it starts, so also look at I_SYNC and the page cache tags.

If there is no memory for a fake inode, write the upper inode right away.
If that fails, or the lookup of the whiteout that RENAME_WHITEOUT made, the
next syncfs() returns the error.

Not covered: the atime of an upper directory that cannot be opened with
O_NOATIME, and inodes that the upper filesystem dirties on its own, like
the quota file of ext2 and jfs.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/overlayfs/inode.c     | 107 ++++++++++++++++++++++++++++++++++++++++++++++-
 fs/overlayfs/overlayfs.h |  90 +++++++++++++++++++++++++++++++--------
 fs/overlayfs/ovl_entry.h |   2 +
 fs/overlayfs/super.c     |  18 +++++---
 fs/overlayfs/util.c      |  42 +++++++++++++++++++
 5 files changed, 235 insertions(+), 24 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 88cec47d0bc8..85ccbb773534 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -13,6 +13,7 @@
 #include <linux/fileattr.h>
 #include <linux/security.h>
 #include <linux/namei.h>
+#include <linux/pagemap.h>
 #include <linux/posix_acl.h>
 #include <linux/posix_acl_xattr.h>
 #include "overlayfs.h"
@@ -752,7 +753,7 @@ int ovl_fileattr_get(struct dentry *dentry, struct file_kattr *fa)
  */
 void ovl_sync_lazytime(struct inode *inode)
 {
-	struct inode *upper = ovl_inode_upper(inode);
+	struct inode *upper = ovl_inode_upper_to_write(inode);
 
 	if (upper && (inode_state_read_once(upper) & I_DIRTY_TIME))
 		mark_inode_dirty_sync(upper);
@@ -1169,6 +1170,110 @@ struct inode *ovl_lookup_inode(struct super_block *sb, struct dentry *real,
 	return inode;
 }
 
+static const struct inode_operations ovl_fake_inode_operations = {
+	.sync_lazytime	= ovl_sync_lazytime,
+};
+
+/* The upper inode that ovl_write_inode() has to write for @inode */
+struct inode *ovl_inode_upper_to_write(struct inode *inode)
+{
+	if (ovl_test_flag(OVL_FAKE, inode))
+		return inode->i_private;
+
+	return ovl_inode_upper(inode);
+}
+
+/*
+ * Is there anything sync_filesystem() would write or wait for on @upper?
+ *
+ * The dirty flags alone cannot tell, because the flusher of the upper
+ * filesystem clears them when it starts to write the inode.  But it sets I_SYNC
+ * before it clears them, it clears I_SYNC after ->write_inode() has returned,
+ * and all of them are in one word.  The caller has modified @upper before it
+ * gets here.  So if neither a dirty flag nor I_SYNC is set, the modification
+ * has not dirtied the inode, or the flusher has handed all of it to the
+ * filesystem.  Pages whose writeback has been started but not completed show in
+ * the page cache tags.
+ */
+static bool ovl_upper_inode_dirty(struct inode *upper)
+{
+	struct address_space *mapping = upper->i_mapping;
+	unsigned int state = inode_state_read_once(upper);
+
+	/* The data of a regular file is left to its overlay inode */
+	if (S_ISREG(upper->i_mode))
+		return state & (I_DIRTY_INODE | I_DIRTY_TIME | I_SYNC);
+
+	return (state & (I_DIRTY_ALL | I_SYNC)) ||
+	       mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) ||
+	       mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK);
+}
+
+/*
+ * overlayfs has modified @upper on the upper filesystem.  Many of these inodes
+ * have no overlay inode that could be on the dirty list of the overlay:
+ * whiteouts, index entries and the work directories.  Give @upper a fake
+ * overlay inode and mark that dirty.  From there on it is treated like any
+ * other dirty overlay inode.  syncfs(), unmount and the flusher find it on the
+ * dirty list, ovl_write_inode() writes @upper, and ovl_drop_inode() keeps the
+ * fake inode for as long as it is dirty.
+ *
+ * Like a trap inode, a fake inode is hashed by the real inode and has no
+ * dentry. It holds a reference to @upper.
+ *
+ * sync_filesystem() on the upper filesystem would only write the inodes that
+ * are on a dirty list there.  So an inode that the VFS does not consider dirty
+ * after it was modified does not need a fake inode.  This is the normal case
+ * for a filesystem that logs its metadata.
+ */
+void ovl_upper_modified(struct ovl_fs *ofs, struct inode *upper)
+{
+	struct inode *fake;
+
+	if (!upper || !ovl_tracks_dirty(ofs))
+		return;
+
+	/*
+	 * Nobody is going to look for an inode that has no name left.  Drop its
+	 * fake inode, so that the space of @upper is freed.
+	 */
+	if (!upper->i_nlink) {
+		fake = ovl_ilookup(ofs->sb, upper, true);
+		if (fake) {
+			/* clear_nlink() counts, it must not run twice */
+			spin_lock(&fake->i_lock);
+			clear_nlink(fake);
+			spin_unlock(&fake->i_lock);
+			iput(fake);
+		}
+		return;
+	}
+
+	if (!ovl_upper_inode_dirty(upper))
+		return;
+
+	fake = ovl_iget_locked(ofs->sb, upper, true);
+	if (!fake) {
+		/*
+		 * There is no memory to remember @upper, so write it now.  The
+		 * modification itself has succeeded, so this is not an error of
+		 * the caller.  If the write fails, the next syncfs() says so.
+		 */
+		ovl_write_upper_inode(ofs->sb, upper, OVL_WRITE_WAIT);
+		return;
+	}
+
+	if (inode_state_read_once(fake) & I_NEW) {
+		ihold(upper);
+		fake->i_mode = upper->i_mode & S_IFMT;
+		fake->i_op = &ovl_fake_inode_operations;
+		unlock_new_inode(fake);
+	}
+
+	ovl_mark_inode_dirty(fake);
+	iput(fake);
+}
+
 bool ovl_lookup_trap_inode(struct super_block *sb, struct dentry *dir)
 {
 	struct inode *key = d_inode(dir);
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 83e25700c8a0..98a6946a96af 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -210,28 +210,58 @@ static inline const char *ovl_xattr(struct ovl_fs *ofs, enum ovl_xattr ox)
  * care to map 1000 to 1001.
  * The mnt idmapping helpers are nops if the upper layer isn't idmapped.
  */
+void ovl_upper_modified(struct ovl_fs *ofs, struct inode *upper);
+
+/*
+ * The ovl_do_*() helpers below are how overlayfs modifies the namespace and the
+ * attributes of the upper filesystem.  Each of them calls ovl_upper_modified()
+ * for all the upper inodes that it has modified, which puts them on the dirty
+ * list of the overlay.  Code that modifies the upper filesystem in another way
+ * has to mark the overlay inode of the file dirty.
+ */
 static inline int ovl_do_notify_change(struct ovl_fs *ofs,
 				       struct dentry *upperdentry,
 				       struct iattr *attr)
 {
-	return notify_change(ovl_upper_mnt_idmap(ofs), upperdentry, attr, NULL);
+	int err = notify_change(ovl_upper_mnt_idmap(ofs), upperdentry, attr,
+				NULL);
+
+	if (!err)
+		ovl_upper_modified(ofs, d_inode(upperdentry));
+	return err;
 }
 
 static inline int ovl_do_rmdir(struct ovl_fs *ofs,
 			       struct inode *dir, struct dentry *dentry)
 {
-	int err = vfs_rmdir(ovl_upper_mnt_idmap(ofs), dir, dentry, NULL);
+	struct inode *inode = d_inode(dentry);
+	int err;
 
+	ihold(inode);
+	err = vfs_rmdir(ovl_upper_mnt_idmap(ofs), dir, dentry, NULL);
 	pr_debug("rmdir(%pd2) = %i\n", dentry, err);
+	if (!err) {
+		ovl_upper_modified(ofs, dir);
+		ovl_upper_modified(ofs, inode);
+	}
+	iput(inode);
 	return err;
 }
 
 static inline int ovl_do_unlink(struct ovl_fs *ofs, struct inode *dir,
 				struct dentry *dentry)
 {
-	int err = vfs_unlink(ovl_upper_mnt_idmap(ofs), dir, dentry, NULL);
+	struct inode *inode = d_inode(dentry);
+	int err;
 
+	ihold(inode);
+	err = vfs_unlink(ovl_upper_mnt_idmap(ofs), dir, dentry, NULL);
 	pr_debug("unlink(%pd2) = %i\n", dentry, err);
+	if (!err) {
+		ovl_upper_modified(ofs, dir);
+		ovl_upper_modified(ofs, inode);
+	}
+	iput(inode);
 	return err;
 }
 
@@ -242,6 +272,10 @@ static inline int ovl_do_link(struct ovl_fs *ofs, struct dentry *old_dentry,
 			   new_dentry, NULL);
 
 	pr_debug("link(%pd2, %pd2) = %i\n", old_dentry, new_dentry, err);
+	if (!err) {
+		ovl_upper_modified(ofs, dir);
+		ovl_upper_modified(ofs, d_inode(old_dentry));
+	}
 	return err;
 }
 
@@ -252,6 +286,10 @@ static inline int ovl_do_create(struct ovl_fs *ofs,
 	int err = vfs_create(ovl_upper_mnt_idmap(ofs), dentry, mode, NULL);
 
 	pr_debug("create(%pd2, 0%o) = %i\n", dentry, mode, err);
+	if (!err) {
+		ovl_upper_modified(ofs, dir);
+		ovl_upper_modified(ofs, d_inode(dentry));
+	}
 	return err;
 }
 
@@ -266,6 +304,10 @@ static inline struct dentry *ovl_do_mkdir(struct ovl_fs *ofs,
 	pr_debug("mkdir(%pd2, 0%o)\n", dentry, mode);
 	ret = vfs_mkdir(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, NULL);
 	pr_debug("...mkdir = %i\n", PTR_ERR_OR_ZERO(ret));
+	if (!IS_ERR(ret)) {
+		ovl_upper_modified(ofs, dir);
+		ovl_upper_modified(ofs, d_inode(ret));
+	}
 	return ret;
 }
 
@@ -276,6 +318,10 @@ static inline int ovl_do_mknod(struct ovl_fs *ofs,
 	int err = vfs_mknod(ovl_upper_mnt_idmap(ofs), dir, dentry, mode, dev, NULL);
 
 	pr_debug("mknod(%pd2, 0%o, 0%o) = %i\n", dentry, mode, dev, err);
+	if (!err) {
+		ovl_upper_modified(ofs, dir);
+		ovl_upper_modified(ofs, d_inode(dentry));
+	}
 	return err;
 }
 
@@ -286,6 +332,10 @@ static inline int ovl_do_symlink(struct ovl_fs *ofs,
 	int err = vfs_symlink(ovl_upper_mnt_idmap(ofs), dir, dentry, oldname, NULL);
 
 	pr_debug("symlink(\"%s\", %pd2) = %i\n", oldname, dentry, err);
+	if (!err) {
+		ovl_upper_modified(ofs, dir);
+		ovl_upper_modified(ofs, d_inode(dentry));
+	}
 	return err;
 }
 
@@ -336,6 +386,8 @@ static inline int ovl_do_setxattr(struct ovl_fs *ofs, struct dentry *dentry,
 
 	pr_debug("setxattr(%pd2, \"%s\", \"%*pE\", %zu, %d) = %i\n",
 		 dentry, name, min((int)size, 48), value, size, flags, err);
+	if (!err)
+		ovl_upper_modified(ofs, d_inode(dentry));
 	return err;
 }
 
@@ -351,6 +403,8 @@ static inline int ovl_do_removexattr(struct ovl_fs *ofs, struct dentry *dentry,
 {
 	int err = vfs_removexattr(ovl_upper_mnt_idmap(ofs), dentry, name);
 	pr_debug("removexattr(%pd2, \"%s\") = %i\n", dentry, name, err);
+	if (!err)
+		ovl_upper_modified(ofs, d_inode(dentry));
 	return err;
 }
 
@@ -363,30 +417,25 @@ static inline int ovl_removexattr(struct ovl_fs *ofs, struct dentry *dentry,
 static inline int ovl_do_set_acl(struct ovl_fs *ofs, struct dentry *dentry,
 				 const char *acl_name, struct posix_acl *acl)
 {
-	return vfs_set_acl(ovl_upper_mnt_idmap(ofs), dentry, acl_name, acl);
+	int err = vfs_set_acl(ovl_upper_mnt_idmap(ofs), dentry, acl_name, acl);
+
+	if (!err)
+		ovl_upper_modified(ofs, d_inode(dentry));
+	return err;
 }
 
 static inline int ovl_do_remove_acl(struct ovl_fs *ofs, struct dentry *dentry,
 				    const char *acl_name)
 {
-	return vfs_remove_acl(ovl_upper_mnt_idmap(ofs), dentry, acl_name);
-}
+	int err = vfs_remove_acl(ovl_upper_mnt_idmap(ofs), dentry, acl_name);
 
-static inline int ovl_do_rename_rd(struct ovl_fs *ofs,
-				   struct renamedata *rd)
-{
-	int err;
-
-	pr_debug("rename(%pd2, %pd2, 0x%x)\n", rd->old_dentry, rd->new_dentry,
-		 rd->flags);
-	err = vfs_rename(rd);
-	if (err) {
-		pr_debug("...rename(%pd2, %pd2, ...) = %i\n",
-			 rd->old_dentry, rd->new_dentry, err);
-	}
+	if (!err)
+		ovl_upper_modified(ofs, d_inode(dentry));
 	return err;
 }
 
+int ovl_do_rename_rd(struct ovl_fs *ofs, struct renamedata *rd);
+
 static inline int ovl_do_rename(struct ovl_fs *ofs, struct dentry *olddir,
 				struct dentry *olddentry, struct dentry *newdir,
 				struct dentry *newdentry, unsigned int flags)
@@ -408,6 +457,10 @@ static inline int ovl_do_whiteout(struct ovl_fs *ofs,
 {
 	int err = vfs_whiteout(ovl_upper_mnt_idmap(ofs), dir, dentry);
 	pr_debug("whiteout(%pd2) = %i\n", dentry, err);
+	if (!err) {
+		ovl_upper_modified(ofs, dir);
+		ovl_upper_modified(ofs, d_inode(dentry));
+	}
 	return err;
 }
 
@@ -516,6 +569,7 @@ const struct ovl_layer *ovl_layer_lower(struct dentry *dentry);
 struct dentry *ovl_dentry_real(struct dentry *dentry);
 struct dentry *ovl_i_dentry_upper(struct inode *inode);
 struct inode *ovl_inode_upper(struct inode *inode);
+struct inode *ovl_inode_upper_to_write(struct inode *inode);
 
 /* How ovl_write_upper_inode() writes an upper inode */
 enum ovl_write_mode {
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index ac07e8769f9b..ed9a3a959fc0 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -85,6 +85,8 @@ struct ovl_fs {
 	int xino_mode;
 	/* For allocation of non-persistent inode numbers */
 	atomic_long_t last_ino;
+	/* For ovl_upper_modified(), which only gets to see upper inodes */
+	struct super_block *sb;
 	/* Shared whiteout cache */
 	struct dentry *whiteout;
 	bool no_shared_whiteout;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index c43f8fdd6230..84258edb5530 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -235,6 +235,8 @@ static void ovl_destroy_inode(struct inode *inode)
 {
 	struct ovl_inode *oi = OVL_I(inode);
 
+	if (ovl_test_flag(OVL_FAKE, inode))
+		iput(inode->i_private);
 	dput(oi->__upperdentry);
 	ovl_stack_put(ovl_lowerstack(oi->oe), ovl_numlower(oi->oe));
 	if (S_ISDIR(inode->i_mode))
@@ -336,12 +338,16 @@ static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
  * the mapping goes away at the latest.  i_writecount drops after that, when the
  * file is released, with the full barrier of the last fput() in between.
  */
-static bool ovl_upper_unfinished(struct inode *upper)
+static bool ovl_upper_unfinished(struct inode *inode, struct inode *upper)
 {
 	struct address_space *mapping = upper->i_mapping;
 
-	return atomic_read_acquire(&upper->i_writecount) > 0 ||
-	       mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) ||
+	/* A file that can be mapped has an overlay inode, which keeps track */
+	if (!ovl_test_flag(OVL_FAKE, inode) &&
+	    atomic_read_acquire(&upper->i_writecount) > 0)
+		return true;
+
+	return mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) ||
 	       mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK) ||
 	       (inode_state_read_once(upper) & (I_DIRTY | I_SYNC));
 }
@@ -395,7 +401,7 @@ static void ovl_park_inode(struct inode *inode)
 
 static int ovl_write_inode(struct inode *inode, struct writeback_control *wbc)
 {
-	struct inode *upper = ovl_inode_upper(inode);
+	struct inode *upper = ovl_inode_upper_to_write(inode);
 	int ret = 0;
 
 	if (!upper)
@@ -411,7 +417,7 @@ static int ovl_write_inode(struct inode *inode, struct writeback_control *wbc)
 		ret = ovl_write_upper_inode(inode->i_sb, upper,
 					    ovl_write_mode(inode, upper, wbc));
 
-	if (ovl_upper_unfinished(upper))
+	if (ovl_upper_unfinished(inode, upper))
 		ovl_mark_inode_dirty(inode);
 	else if (inode_state_read_once(upper) & I_DIRTY_TIME)
 		ovl_park_inode(inode);
@@ -1697,6 +1703,8 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 	struct ovl_fs *ofs = sb->s_fs_info;
 	int err;
 
+	ofs->sb = sb;
+
 	err = -EINVAL;
 	/* The fscontext fd may have been passed to another user namespace. */
 	if (fc->user_ns != current_user_ns())
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 76e090b2a0c5..ff43f8c39979 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1580,3 +1580,45 @@ int ovl_write_upper_inode(struct super_block *sb, struct inode *upper,
 		errseq_set(&sb->s_wb_err, err);
 	return err;
 }
+
+int ovl_do_rename_rd(struct ovl_fs *ofs, struct renamedata *rd)
+{
+	struct inode *target = d_inode(rd->new_dentry);
+	struct name_snapshot old_name;
+	struct dentry *whiteout;
+	int err;
+
+	pr_debug("rename(%pd2, %pd2, 0x%x)\n", rd->old_dentry, rd->new_dentry,
+		 rd->flags);
+
+	/* The old dentry is going to have the new name */
+	take_dentry_name_snapshot(&old_name, rd->old_dentry);
+
+	err = vfs_rename(rd);
+	if (err) {
+		pr_debug("...rename(%pd2, %pd2, ...) = %i\n",
+			 rd->old_dentry, rd->new_dentry, err);
+		goto out;
+	}
+
+	ovl_upper_modified(ofs, d_inode(rd->old_parent));
+	ovl_upper_modified(ofs, d_inode(rd->new_parent));
+	ovl_upper_modified(ofs, d_inode(rd->old_dentry));
+	ovl_upper_modified(ofs, target);
+
+	/* The upper filesystem has made this inode itself, so look it up */
+	if ((rd->flags & RENAME_WHITEOUT) && ovl_tracks_dirty(ofs)) {
+		whiteout = lookup_one(rd->mnt_idmap, &old_name.name,
+				      rd->old_parent);
+		if (!IS_ERR(whiteout)) {
+			ovl_upper_modified(ofs, d_inode(whiteout));
+			dput(whiteout);
+		} else {
+			/* The rename has succeeded, let syncfs() say it */
+			errseq_set(&ofs->sb->s_wb_err, PTR_ERR(whiteout));
+		}
+	}
+out:
+	release_dentry_name_snapshot(&old_name);
+	return err;
+}

-- 
2.55.0

[PATCH v2 10/10] ovl: implement containerized syncfs for overlayfs

[Josef Bacik]

From: Chengguang Xu <charliecgxu@tencent.com>

Now overlayfs can only sync own dirty inodes during syncfs,
so remove unnecessary sync_filesystem() on upper file system.

Call ->sync_fs() and write the block device of the upper filesystem
instead, which is what sync_filesystem() does after it has written the
inodes.  Do the same for a read-only remount.

With many containers on one upper filesystem, stopping one of them no
longer writes out the dirty data of all the others.

Signed-off-by: Chengguang Xu <charliecgxu@tencent.com>
[josef: everything after the first paragraph, no wait_sb_inodes() on the
 upper filesystem]
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/overlayfs/params.c | 14 +++-----------
 fs/overlayfs/super.c  | 28 ++++++++++++++++++++--------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
index 5a15e5478a6b..43fe0b1ef11f 100644
--- a/fs/overlayfs/params.c
+++ b/fs/overlayfs/params.c
@@ -764,22 +764,14 @@ static int ovl_reconfigure(struct fs_context *fc)
 {
 	struct super_block *sb = fc->root->d_sb;
 	struct ovl_fs *ofs = OVL_FS(sb);
-	struct super_block *upper_sb;
-	int ret = 0;
 
 	if (!(fc->sb_flags & SB_RDONLY) && ovl_force_readonly(ofs))
 		return -EROFS;
 
-	if (fc->sb_flags & SB_RDONLY && !sb_rdonly(sb)) {
-		upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
-		if (ovl_should_sync(ofs)) {
-			down_read(&upper_sb->s_umount);
-			ret = sync_filesystem(upper_sb);
-			up_read(&upper_sb->s_umount);
-		}
-	}
+	if (fc->sb_flags & SB_RDONLY && !sb_rdonly(sb) && ovl_should_sync(ofs))
+		return sync_filesystem(sb);
 
-	return ret;
+	return 0;
 }
 
 static const struct fs_context_operations ovl_context_ops = {
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 84258edb5530..361a3afa4ac8 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -15,6 +15,7 @@
 #include <linux/seq_file.h>
 #include <linux/posix_acl_xattr.h>
 #include <linux/exportfs.h>
+#include <linux/blkdev.h>
 #include <linux/pagemap.h>
 #include <linux/writeback.h>
 #include <linux/file.h>
@@ -271,18 +272,29 @@ static int ovl_sync_fs(struct super_block *sb, int wait)
 	/*
 	 * Not called for sync(2) call or an emergency sync (SB_I_SKIP_SYNC).
 	 * All the super blocks will be iterated, including upper_sb.
-	 *
-	 * If this is a syncfs(2) call, then we do need to call
-	 * sync_filesystem() on upper_sb, but enough if we do it when being
-	 * called with wait == 1.
 	 */
-	if (!wait)
-		return 0;
-
 	upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
 
+	/*
+	 * sync_filesystem() has already written the dirty inodes of this
+	 * overlay through ovl_write_inode(), including the fake ones, see
+	 * ovl_upper_modified().  What is left are the last two steps of
+	 * sync_filesystem() on the upper filesystem.  Do not write the inodes
+	 * that others have dirtied there.
+	 */
 	down_read(&upper_sb->s_umount);
-	ret = sync_filesystem(upper_sb);
+	/* Like sync_filesystem(), do not call ->sync_fs() on a read-only sb */
+	ret = 0;
+	if (sb_rdonly(upper_sb))
+		goto out;
+
+	if (upper_sb->s_op->sync_fs)
+		ret = upper_sb->s_op->sync_fs(upper_sb, wait);
+	if (!ret && wait)
+		ret = sync_blockdev(upper_sb->s_bdev);
+	else if (!ret)
+		ret = sync_blockdev_nowait(upper_sb->s_bdev);
+out:
 	up_read(&upper_sb->s_umount);
 
 	return ret;

-- 
2.55.0