[PATCH v10 00/18] overlayfs: Delayed copy up of data

[PATCH v10 00/18] overlayfs: Delayed copy up of data

[Vivek Goyal]

Hi,

Please find attached V10 of the metadata only copy up patches. I think
I have taken care of review comments and I will start writing automated
test cases now.

One thing which concerns me about these patches is the fact that during
file open d_real() will return lower inode which actually contains file
data and that inode will be installed in f->f_inode. So while f->f_inode
is perfectly fine for data operations, it is not fine for metadata
operations. Because metadata has been copied up and is represented by
a different inode.

I am concerned that this is a subtle behavior change and especially
security modules might be surprised by it. I will spend now more time
looking at what security modules are doing with f->f_inode. Thoughts?

Amir, some new patches have been added in series due to metacopy inode
support in midlayer. And some existing patches have changed. You
probably will have to look at existing Reviewd-by patches too.

Changes since V9:

- Added support for metacopy inode in midlayer.
- Added documentation for metadata only copy up.
- Stopped following metacopy origin if metacopy is not enabled. This is
  due to possible security concerns.
- Dropped a patch to do union of encryption and compression attributes
  of lower and upper. Doing union seemed wrong.
- Dropped a patch to get rid of -EOPNOTSUPP check in
  ovl_check_setxattr(). Looks like both Amir and Miklos wanted to retain
  that check and warn if unexpctedly upper returned -EOPNOTSUPP at a
  later time while checks were fine during mount time.

Any review comment/feedback is welcome.

Vivek

Amir Goldstein (1):
  ovl: disable redirect_dir and index when no xattr support

Vivek Goyal (17):
  ovl: Do not look for OVL_XATTR_NLINK if index is not there
  ovl: Create origin xattr on copy up for all files
  ovl: Provide a mount option metacopy=on/off for metadata copyup
  ovl: During copy up, first copy up metadata and then data
  ovl: Move the copy up helpers to copy_up.c
  ovl: Add mechanism to create a chain of origin dentries
  ovl: Copy up only metadata during copy up where it makes sense
  ovl: Add helper ovl_already_copied_up()
  ovl: A new xattr OVL_XATTR_METACOPY for file on upper
  ovl: Set OVL_UPPERDATA flag during ovl_lookup()
  ovl: Setup origin chain for lower regular files
  ovl: Check metacopy attributes on a chain of origin
  ovl: Do not mark a non dir as _OVL_PATH_MERGE in ovl_path_type()
  ovl: Copy up meta inode data from lowest data inode
  ovl: Fix ovl_getattr() to get number of blocks from lower
  ovl: Do not expose metacopy only upper dentry from d_real()
  ovl: Enable metadata only feature

 Documentation/filesystems/overlayfs.txt |  54 +++++++---
 fs/overlayfs/Kconfig                    |  15 +++
 fs/overlayfs/copy_up.c                  | 172 +++++++++++++++++++++---------
 fs/overlayfs/dir.c                      |   1 +
 fs/overlayfs/inode.c                    |  60 ++++-------
 fs/overlayfs/namei.c                    | 181 ++++++++++++++++++++++++++++----
 fs/overlayfs/overlayfs.h                |  24 ++++-
 fs/overlayfs/ovl_entry.h                |   2 +
 fs/overlayfs/super.c                    |  64 +++++++++--
 fs/overlayfs/util.c                     | 143 ++++++++++++++++++++++++-
 10 files changed, 583 insertions(+), 133 deletions(-)

-- 
2.13.6

[PATCH v10 01/18] ovl: Do not look for OVL_XATTR_NLINK if index is not there

[Vivek Goyal]

Soon, we will be creating OVL_TYPE_ORIGIN entries even for hardlinked
files with index=off. That means, it is possible that there is no
index and hence no OVL_XATTR_NLINK set on upperdentry but lowerdentry
is still there. In that case current implementation gets -ENODATA
from vfs_getxattr)() and it warns and returns fallback. I can get
following warning.

"overlayfs: failed to get index nlink ....."

Pass in index instead of upperdentry in ovl_get_nlink(). That way, if
index is not there, we know that OVL_XATTR_NLINK should not be
present and just return fallback.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/inode.c     | 13 ++++++-------
 fs/overlayfs/overlayfs.h |  2 +-
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 00b6b294272a..903548a6b825 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -543,7 +543,7 @@ int ovl_set_nlink_lower(struct dentry *dentry)
 }
 
 unsigned int ovl_get_nlink(struct dentry *lowerdentry,
-			   struct dentry *upperdentry,
+			   struct dentry *index,
 			   unsigned int fallback)
 {
 	int nlink_diff;
@@ -551,10 +551,10 @@ unsigned int ovl_get_nlink(struct dentry *lowerdentry,
 	char buf[13];
 	int err;
 
-	if (!lowerdentry || !upperdentry || d_inode(lowerdentry)->i_nlink == 1)
+	if (!lowerdentry || !index || d_inode(lowerdentry)->i_nlink == 1)
 		return fallback;
 
-	err = vfs_getxattr(upperdentry, OVL_XATTR_NLINK, &buf, sizeof(buf) - 1);
+	err = vfs_getxattr(index, OVL_XATTR_NLINK, &buf, sizeof(buf) - 1);
 	if (err < 0)
 		goto fail;
 
@@ -567,7 +567,7 @@ unsigned int ovl_get_nlink(struct dentry *lowerdentry,
 	if (err < 0)
 		goto fail;
 
-	nlink = d_inode(buf[0] == 'L' ? lowerdentry : upperdentry)->i_nlink;
+	nlink = d_inode(buf[0] == 'L' ? lowerdentry : index)->i_nlink;
 	nlink += nlink_diff;
 
 	if (nlink <= 0)
@@ -577,7 +577,7 @@ unsigned int ovl_get_nlink(struct dentry *lowerdentry,
 
 fail:
 	pr_warn_ratelimited("overlayfs: failed to get index nlink (%pd2, err=%i)\n",
-			    upperdentry, err);
+			    index, err);
 	return fallback;
 }
 
@@ -670,8 +670,7 @@ struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry,
 			goto out;
 		}
 
-		nlink = ovl_get_nlink(lowerdentry, upperdentry,
-				      realinode->i_nlink);
+		nlink = ovl_get_nlink(lowerdentry, index, realinode->i_nlink);
 		set_nlink(inode, nlink);
 	} else {
 		inode = new_inode(dentry->d_sb);
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index b489099ccd49..edb23f97bd90 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -274,7 +274,7 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
 int ovl_set_nlink_upper(struct dentry *dentry);
 int ovl_set_nlink_lower(struct dentry *dentry);
 unsigned int ovl_get_nlink(struct dentry *lowerdentry,
-			   struct dentry *upperdentry,
+			   struct dentry *index,
 			   unsigned int fallback);
 int ovl_setattr(struct dentry *dentry, struct iattr *attr);
 int ovl_getattr(const struct path *path, struct kstat *stat,
-- 
2.13.6

[PATCH v10 02/18] ovl: disable redirect_dir and index when no xattr support

[Vivek Goyal]

From: Amir Goldstein <amir73il@gmail.com>

Overlayfs falls back to index=off if lower/upper fs does not support
file handles. We should do the same if upper fs does not support xattr.

The redirect_dir feature is implicitly disabled when upper fs does not
support xattr via the check in ovl_redirect_dir(). Make the feature
explicitly disabled in this case by emitting a warning at mount time
and setting redirect_dir to off so its true state is visible in
/proc/mounts.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/super.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 76440feb79f6..e9419faaa156 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -967,7 +967,9 @@ static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
 	err = ovl_do_setxattr(ofs->workdir, OVL_XATTR_OPAQUE, "0", 1, 0);
 	if (err) {
 		ofs->noxattr = true;
-		pr_warn("overlayfs: upper fs does not support xattr.\n");
+		ofs->config.redirect_dir = false;
+		ofs->config.index = false;
+		pr_warn("overlayfs: upper fs does not support xattr, falling back to redirect_dir=off, index=off and no opaque dir.\n");
 	} else {
 		vfs_removexattr(ofs->workdir, OVL_XATTR_OPAQUE);
 	}
-- 
2.13.6

[PATCH v10 03/18] ovl: Create origin xattr on copy up for all files

[Vivek Goyal]

Right now my understanding is that origin xattr is created for all copied
up files if index=on. And if index=off, then we create it for all type
of files except hardlinks (nlink != 1).

With metadata only copy up, I will still require origin xattr to copy up
data later, so create it even for hardlinks even with index=off.

Given ->origin is always true now, get rid of this field from
"struct ovl_copy_up_ctx".

For broken hard links (wht index=off), we will already report inode number
of upper inode (and not lower). So we should not run into issues of two
objects in upper reporting same inode number. IOW, origin is present but
it is not being used for inode reporting purposes. This will still be useful
for metadata copy purposes.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/copy_up.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index eb3b8d39fb61..f55bece3688e 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -326,7 +326,6 @@ struct ovl_copy_up_ctx {
 	struct qstr destname;
 	struct dentry *workdir;
 	bool tmpfile;
-	bool origin;
 };
 
 static int ovl_link_up(struct ovl_copy_up_ctx *c)
@@ -469,15 +468,10 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
 	/*
 	 * Store identifier of lower inode in upper inode xattr to
 	 * allow lookup of the copy up origin inode.
-	 *
-	 * Don't set origin when we are breaking the association with a lower
-	 * hard link.
 	 */
-	if (c->origin) {
-		err = ovl_set_origin(c->dentry, c->lowerpath.dentry, temp);
-		if (err)
-			return err;
-	}
+	err = ovl_set_origin(c->dentry, c->lowerpath.dentry, temp);
+	if (err)
+		return err;
 
 	return 0;
 }
@@ -542,9 +536,6 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 	    c->stat.nlink > 1)
 		indexed = true;
 
-	if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || indexed)
-		c->origin = true;
-
 	if (indexed) {
 		c->destdir = ovl_indexdir(c->dentry->d_sb);
 		err = ovl_get_index_name(c->lowerpath.dentry, &c->destname);
-- 
2.13.6

[PATCH v10 04/18] ovl: Provide a mount option metacopy=on/off for metadata copyup

[Vivek Goyal]

By default metadata only copy up is disabled. Provide a mount option so
that users can choose one way or other.

Also provide a kernel config and module option to enable/disable
metacopy feature.

Like index feature, we verify on mount that upper root is not being
reused with a different lower root. This hopes to get the configuration
right and detect the copied layers use case. But this does only so
much as we don't verify all the lowers. So it is possible that a lower is
missing and later data copy up fails.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 Documentation/filesystems/overlayfs.txt | 54 ++++++++++++++++++++++++---------
 fs/overlayfs/Kconfig                    | 15 +++++++++
 fs/overlayfs/ovl_entry.h                |  1 +
 fs/overlayfs/super.c                    | 40 ++++++++++++++++++++++--
 4 files changed, 93 insertions(+), 17 deletions(-)

diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
index e6a5f4912b6d..43743d1121b8 100644
--- a/Documentation/filesystems/overlayfs.txt
+++ b/Documentation/filesystems/overlayfs.txt
@@ -235,6 +235,30 @@ rightmost one and going left.  In the above example lower1 will be the
 top, lower2 the middle and lower3 the bottom layer.
 
 
+Metadata only copyup
+--------------------
+
+When metadata only copy up feature is enabled, overlayfs will only copy
+up metadata (as opposed to whole file), when a metadata specific operation
+like chown/chmod is performed. Full file will be copied up later when
+file is opened for WRITE operation.
+
+IOW, this is delayed data copy up operation and data is copied up when
+there is a need to actually modify data.
+
+There are multiple ways to enable/disable this feature. A config option
+CONFIG_OVERLAY_FS_METACOPY can be set/unset to enable/disable this feature
+by default. Or one can enable/disable it at module load time with module
+parameter metacopy=on/off. Lastly, there is also a per mount option
+metacopy=on/off to enable/disable this feature per mount.
+
+Do not use metacopy=on with untrusted upper/lower directories. Otherwise
+it is possible that an attacker can create an handcrafted file with
+appropriate ORIGIN and METACOPY xattrs, and gain access to file on lower
+pointed by ORIGIN. This should not be possible on local system as setting
+"trusted." xattrs will require CAP_SYS_ADMIN. But it should be possible
+for untrusted layers like from a pen drive.
+
 Sharing and copying layers
 --------------------------
 
@@ -253,23 +277,25 @@ though it will not result in a crash or deadlock.
 Mounting an overlay using an upper layer path, where the upper layer path
 was previously used by another mounted overlay in combination with a
 different lower layer path, is allowed, unless the "inodes index" feature
-is enabled.
-
-With the "inodes index" feature, on the first time mount, an NFS file
-handle of the lower layer root directory, along with the UUID of the lower
-filesystem, are encoded and stored in the "trusted.overlay.origin" extended
-attribute on the upper layer root directory.  On subsequent mount attempts,
-the lower root directory file handle and lower filesystem UUID are compared
-to the stored origin in upper root directory.  On failure to verify the
-lower root origin, mount will fail with ESTALE.  An overlayfs mount with
-"inodes index" enabled will fail with EOPNOTSUPP if the lower filesystem
-does not support NFS export, lower filesystem does not have a valid UUID or
-if the upper filesystem does not support extended attributes.
+or "metadata only copyup" feature is enabled.
+
+With the "inodes index" or "metadata only copyup" feature, on the first time
+mount, an NFS file handle of the lower layer root directory, along with the
+UUID of the lower filesystem, are encoded and stored in the
+"trusted.overlay.origin" extended attribute on the upper layer root
+directory.  On subsequent mount attempts, the lower root directory file
+handle and lower filesystem UUID are compared to the stored origin in upper
+root directory.  On failure to verify the lower root origin, mount will fail
+with ESTALE.  An overlayfs mount with "inodes index" enabled will fail with
+EOPNOTSUPP if the lower filesystem does not support NFS export, lower
+filesystem does not have a valid UUID or if the upper filesystem does not
+support extended attributes.
 
 It is quite a common practice to copy overlay layers to a different
 directory tree on the same or different underlying filesystem, and even
-to a different machine.  With the "inodes index" feature, trying to mount
-the copied layers will fail the verification of the lower root file handle.
+to a different machine.  With the "inodes index" or "metadata only copyup"
+feature, trying to mount the copied layers will fail the verification of the
+lower root file handle.
 
 
 Non-standard behavior
diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
index 5ac415466861..176546dfc49a 100644
--- a/fs/overlayfs/Kconfig
+++ b/fs/overlayfs/Kconfig
@@ -53,3 +53,18 @@ config OVERLAY_FS_INDEX
 	  outcomes.  However, mounting the same overlay with an old kernel
 	  read-write and then mounting it again with a new kernel, will have
 	  unexpected results.
+
+config OVERLAY_FS_METACOPY
+	bool "Overlayfs: turn on metadata only copy up feature by default"
+	depends on OVERLAY_FS
+	help
+	  If this config option is enabled then overlay filesystems will
+	  copy up only metadata where appropriate and data copy up will
+	  happen when a file is opended for WRITE operation. It is still
+	  possible to turn off this feature globally with the "metacopy=off"
+	  module option or on a filesystem instance basis with the
+	  "metacopy=off" mount option.
+
+	  Note, that this feature is not backward compatible.  That is,
+	  mounting an overlay which has metacopy only inodes on a kernel
+	  that doesn't support this feature will have unexpected results.
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 9d0bc03bf6e4..92a4db0bd18c 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -17,6 +17,7 @@ struct ovl_config {
 	bool redirect_follow;
 	const char *redirect_mode;
 	bool index;
+	bool metacopy;
 };
 
 struct ovl_layer {
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index e9419faaa156..73cb90eeda0f 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -53,6 +53,11 @@ static void ovl_entry_stack_free(struct ovl_entry *oe)
 		dput(oe->lowerstack[i].dentry);
 }
 
+static bool ovl_metacopy_def = IS_ENABLED(CONFIG_OVERLAY_FS_METACOPY);
+module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
+MODULE_PARM_DESC(ovl_metacopy_def,
+		 "Default to on or off for the metadata only copy up feature");
+
 static void ovl_dentry_release(struct dentry *dentry)
 {
 	struct ovl_entry *oe = dentry->d_fsdata;
@@ -341,6 +346,9 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 		seq_printf(m, ",redirect_dir=%s", ofs->config.redirect_mode);
 	if (ofs->config.index != ovl_index_def)
 		seq_printf(m, ",index=%s", ofs->config.index ? "on" : "off");
+	if (ofs->config.metacopy != ovl_metacopy_def)
+		seq_printf(m, ",metacopy=%s",
+			   ofs->config.metacopy ? "on" : "off");
 	return 0;
 }
 
@@ -373,6 +381,8 @@ enum {
 	OPT_REDIRECT_DIR,
 	OPT_INDEX_ON,
 	OPT_INDEX_OFF,
+	OPT_METACOPY_ON,
+	OPT_METACOPY_OFF,
 	OPT_ERR,
 };
 
@@ -384,6 +394,8 @@ static const match_table_t ovl_tokens = {
 	{OPT_REDIRECT_DIR,		"redirect_dir=%s"},
 	{OPT_INDEX_ON,			"index=on"},
 	{OPT_INDEX_OFF,			"index=off"},
+	{OPT_METACOPY_ON,		"metacopy=on"},
+	{OPT_METACOPY_OFF,		"metacopy=off"},
 	{OPT_ERR,			NULL}
 };
 
@@ -490,6 +502,14 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 			config->index = false;
 			break;
 
+		case OPT_METACOPY_ON:
+			config->metacopy = true;
+			break;
+
+		case OPT_METACOPY_OFF:
+			config->metacopy = false;
+			break;
+
 		default:
 			pr_err("overlayfs: unrecognized mount option \"%s\" or missing value\n", p);
 			return -EINVAL;
@@ -703,9 +723,11 @@ static int ovl_lower_dir(const char *name, struct path *path,
 	 * The inodes index feature needs to encode and decode file
 	 * handles, so it requires that all layers support them.
 	 */
-	if (ofs->config.index && !ovl_can_decode_fh(path->dentry->d_sb)) {
+	if ((ofs->config.index || ofs->config.metacopy) &&
+	     !ovl_can_decode_fh(path->dentry->d_sb)) {
+		pr_warn("overlayfs: fs on '%s' does not support file handles, falling back to index=off and metacopy=off.\n", name);
 		ofs->config.index = false;
-		pr_warn("overlayfs: fs on '%s' does not support file handles, falling back to index=off.\n", name);
+		ofs->config.metacopy = false;
 	}
 
 	return 0;
@@ -969,7 +991,8 @@ static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
 		ofs->noxattr = true;
 		ofs->config.redirect_dir = false;
 		ofs->config.index = false;
-		pr_warn("overlayfs: upper fs does not support xattr, falling back to redirect_dir=off, index=off and no opaque dir.\n");
+		ofs->config.metacopy = false;
+		pr_warn("overlayfs: upper fs does not support xattr, falling back to redirect_dir=off, index=off, metacopy=off and no opaque dir.\n");
 	} else {
 		vfs_removexattr(ofs->workdir, OVL_XATTR_OPAQUE);
 	}
@@ -1209,6 +1232,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		goto out_err;
 
 	ofs->config.index = ovl_index_def;
+	ofs->config.metacopy = ovl_metacopy_def;
 	err = ovl_parse_opt((char *) data, &ofs->config);
 	if (err)
 		goto out_err;
@@ -1254,6 +1278,16 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	else if (ofs->upper_mnt->mnt_sb != ofs->same_sb)
 		ofs->same_sb = NULL;
 
+	if (ofs->upper_mnt && ofs->config.metacopy) {
+		/* Verify lower root is upper root origin */
+		err = ovl_verify_origin(upperpath.dentry,
+					oe->lowerstack[0].dentry, false, true);
+		if (err) {
+			pr_err("overlayfs: failed to verify upper root origin\n");
+			goto out_free_oe;
+		}
+	}
+
 	if (!(ovl_force_readonly(ofs)) && ofs->config.index) {
 		err = ovl_get_indexdir(ofs, oe, &upperpath);
 		if (err)
-- 
2.13.6

[PATCH v10 05/18] ovl: During copy up, first copy up metadata and then data

[Vivek Goyal]

Just a little re-ordering of code. This helps with next patch where after
copying up metadata, we skip data copying step, if needed.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/copy_up.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index f55bece3688e..303794bb9127 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -443,6 +443,19 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
 {
 	int err;
 
+	err = ovl_copy_xattr(c->lowerpath.dentry, temp);
+	if (err)
+		return err;
+
+	/*
+	 * Store identifier of lower inode in upper inode xattr to
+	 * allow lookup of the copy up origin inode.
+	 *
+	 */
+	err = ovl_set_origin(c->dentry, c->lowerpath.dentry, temp);
+	if (err)
+		return err;
+
 	if (S_ISREG(c->stat.mode)) {
 		struct path upperpath;
 
@@ -455,25 +468,11 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
 			return err;
 	}
 
-	err = ovl_copy_xattr(c->lowerpath.dentry, temp);
-	if (err)
-		return err;
-
 	inode_lock(temp->d_inode);
 	err = ovl_set_attr(temp, &c->stat);
 	inode_unlock(temp->d_inode);
-	if (err)
-		return err;
-
-	/*
-	 * Store identifier of lower inode in upper inode xattr to
-	 * allow lookup of the copy up origin inode.
-	 */
-	err = ovl_set_origin(c->dentry, c->lowerpath.dentry, temp);
-	if (err)
-		return err;
 
-	return 0;
+	return err;
 }
 
 static int ovl_copy_up_locked(struct ovl_copy_up_ctx *c)
-- 
2.13.6

[PATCH v10 06/18] ovl: Move the copy up helpers to copy_up.c

[Vivek Goyal]

Right now two copy up helpers are in inode.c. Amir suggested it might
be better to move these to copy_up.c.

There will one more related function which will come in later patch.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/copy_up.c   | 30 ++++++++++++++++++++++++++++++
 fs/overlayfs/inode.c     | 30 ------------------------------
 fs/overlayfs/overlayfs.h |  2 +-
 3 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 303794bb9127..31711edf5bde 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -686,6 +686,36 @@ int ovl_copy_up_flags(struct dentry *dentry, int flags)
 	return err;
 }
 
+static bool ovl_open_need_copy_up(struct dentry *dentry, int flags)
+{
+	if (ovl_dentry_upper(dentry) &&
+	    ovl_dentry_has_upper_alias(dentry))
+		return false;
+
+	if (special_file(d_inode(dentry)->i_mode))
+		return false;
+
+	if (!(OPEN_FMODE(flags) & FMODE_WRITE) && !(flags & O_TRUNC))
+		return false;
+
+	return true;
+}
+
+int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags)
+{
+	int err = 0;
+
+	if (ovl_open_need_copy_up(dentry, file_flags)) {
+		err = ovl_want_write(dentry);
+		if (!err) {
+			err = ovl_copy_up_flags(dentry, file_flags);
+			ovl_drop_write(dentry);
+		}
+	}
+
+	return err;
+}
+
 int ovl_copy_up(struct dentry *dentry)
 {
 	return ovl_copy_up_flags(dentry, 0);
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 903548a6b825..5239a1ff7211 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -341,36 +341,6 @@ struct posix_acl *ovl_get_acl(struct inode *inode, int type)
 	return acl;
 }
 
-static bool ovl_open_need_copy_up(struct dentry *dentry, int flags)
-{
-	if (ovl_dentry_upper(dentry) &&
-	    ovl_dentry_has_upper_alias(dentry))
-		return false;
-
-	if (special_file(d_inode(dentry)->i_mode))
-		return false;
-
-	if (!(OPEN_FMODE(flags) & FMODE_WRITE) && !(flags & O_TRUNC))
-		return false;
-
-	return true;
-}
-
-int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags)
-{
-	int err = 0;
-
-	if (ovl_open_need_copy_up(dentry, file_flags)) {
-		err = ovl_want_write(dentry);
-		if (!err) {
-			err = ovl_copy_up_flags(dentry, file_flags);
-			ovl_drop_write(dentry);
-		}
-	}
-
-	return err;
-}
-
 int ovl_update_time(struct inode *inode, struct timespec *ts, int flags)
 {
 	struct dentry *alias;
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index edb23f97bd90..31a212781a15 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -286,7 +286,6 @@ int ovl_xattr_get(struct dentry *dentry, struct inode *inode, const char *name,
 		  void *value, size_t size);
 ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size);
 struct posix_acl *ovl_get_acl(struct inode *inode, int type);
-int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags);
 int ovl_update_time(struct inode *inode, struct timespec *ts, int flags);
 bool ovl_is_private_xattr(const char *name);
 
@@ -319,6 +318,7 @@ int ovl_cleanup(struct inode *dir, struct dentry *dentry);
 /* copy_up.c */
 int ovl_copy_up(struct dentry *dentry);
 int ovl_copy_up_flags(struct dentry *dentry, int flags);
+int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags);
 int ovl_copy_xattr(struct dentry *old, struct dentry *new);
 int ovl_set_attr(struct dentry *upper, struct kstat *stat);
 struct ovl_fh *ovl_encode_fh(struct dentry *lower, bool is_upper);
-- 
2.13.6

[PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Vivek Goyal]

There is a need to support metacopy dentry in midlayer. That means there
could be a chain of metacopy dentries.

For example, upper could be metacopy, midlayer lower could be metacopy and
lowest layer could be actual data inode. This means when we copy up actual
data, we should be able to reach to lowest data inode and copy up data from
there. And that means we should keep track of all the dentries in origin
chain which lead to data inode.

Current ovl_check_origin() logic only looks for one origin dentry. This patch
enhances ovl_check_origin() to continue to follow origin chain and return
all the origin entries found. This is done only if caller of the function
set "follow_chain" argument.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/namei.c | 66 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 21 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index beb945e1963c..5b834abde37a 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -283,39 +283,61 @@ static int ovl_lookup_layer(struct dentry *base, struct ovl_lookup_data *d,
 	return 0;
 }
 
-
-static int ovl_check_origin(struct dentry *upperdentry,
+static int ovl_check_origin(struct dentry *dentry,
 			    struct ovl_path *lower, unsigned int numlower,
-			    struct ovl_path **stackp, unsigned int *ctrp)
+			    struct ovl_path **stackp, unsigned int *ctrp,
+			    bool follow_chain)
 {
 	struct vfsmount *mnt;
 	struct dentry *origin = NULL;
-	int i;
+	int i, nr_path = (follow_chain ? numlower : 1), err = 0;
+	int idx = 0;
+	bool mem_allocated = false;
+	struct ovl_path *origin_stack;
+
+	BUG_ON(*ctrp);
+
+	if (!*stackp) {
+		origin_stack = kmalloc(sizeof(struct ovl_path) * nr_path, GFP_KERNEL);
+		if (!origin_stack)
+			return -ENOMEM;
+		mem_allocated = true;
+	} else
+		origin_stack = *stackp;
 
 	for (i = 0; i < numlower; i++) {
 		mnt = lower[i].layer->mnt;
-		origin = ovl_get_origin(upperdentry, mnt);
-		if (IS_ERR(origin))
-			return PTR_ERR(origin);
+		origin = ovl_get_origin(dentry, mnt);
+		if (IS_ERR(origin)) {
+			err = PTR_ERR(origin);
+			goto out_err;
+		}
 
-		if (origin)
-			break;
+		if (origin) {
+			origin_stack[idx++] = (struct ovl_path){.dentry = origin, .layer = lower[i].layer};
+			if (!follow_chain)
+				break;
+			else
+				dentry = origin;
+		}
 	}
 
-	if (!origin)
+	if (mem_allocated && !idx) {
+		kfree(origin_stack);
 		return 0;
-
-	BUG_ON(*ctrp);
-	if (!*stackp)
-		*stackp = kmalloc(sizeof(struct ovl_path), GFP_KERNEL);
-	if (!*stackp) {
-		dput(origin);
-		return -ENOMEM;
 	}
-	**stackp = (struct ovl_path){.dentry = origin, .layer = lower[i].layer};
-	*ctrp = 1;
 
+	*stackp = origin_stack;
+	*ctrp = idx;
 	return 0;
+out_err:
+	for (i = 0; i < idx; i++)
+		dput(origin_stack[i].dentry);
+
+	if (mem_allocated)
+		kfree(origin_stack);
+
+	return err;
 }
 
 /*
@@ -427,7 +449,7 @@ int ovl_verify_index(struct dentry *index, struct ovl_path *lower,
 	if (err)
 		goto fail;
 
-	err = ovl_check_origin(index, lower, numlower, &stack, &ctr);
+	err = ovl_check_origin(index, lower, numlower, &stack, &ctr, false);
 	if (!err && !ctr)
 		err = -ESTALE;
 	if (err)
@@ -602,6 +624,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	struct dentry *this;
 	unsigned int i;
 	int err;
+	bool origin_chain = ofs->config.metacopy ? true : false;
 	struct ovl_lookup_data d = {
 		.name = dentry->d_name,
 		.is_dir = false,
@@ -639,7 +662,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			 * to a dentry in lower layer that was moved under us.
 			 */
 			err = ovl_check_origin(upperdentry, roe->lowerstack,
-					       roe->numlower, &stack, &ctr);
+					       roe->numlower, &stack, &ctr,
+					       origin_chain);
 			if (err)
 				goto out_put_upper;
 		}
-- 
2.13.6

[PATCH v10 08/18] ovl: Copy up only metadata during copy up where it makes sense

[Vivek Goyal]

If it makes sense to copy up only metadata during copy up, do it. This
is done for regular files which are not opened for WRITE and have origin
being saved.

Right now ->metacopy is set to 0 always. Last patch in the series will
remove the hard coded statement and enable metacopy feature.

Currently ovl_set_origin() returns success even if specified xattr could
not be set due to -EOPNOTSUPP returned by upper. IOW, ovl_set_origin()
operation is optional and copy up operation continues.

With metadata copy up, ovl_set_origin() can't be optional. We need to know
if origin could be set or not. If it could not be set, then either disable
metacopy or abort copy up operation. I have take then later approach and
disable metacopy for this inode.

Normally we should not run into this path as metacopy will not be enabled
if upper does not support xattr. This check is done during mount. This
path can only hit if checks during mount pass but at some point of time
later upper still returns -EOPNOTSUPP. So its a saftey mechanism to handle
that unexpected -EOPNOTSUPP.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/copy_up.c | 38 +++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 31711edf5bde..c5804b87f3c7 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -232,6 +232,26 @@ int ovl_set_attr(struct dentry *upperdentry, struct kstat *stat)
 	return err;
 }
 
+static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode,
+				  int flags)
+{
+	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+
+	/* TODO: Will enable metacopy in last patch of series */
+	return false;
+
+	if (!ofs->config.metacopy)
+		return false;
+
+	if (!S_ISREG(mode))
+		return false;
+
+	if (flags && ((OPEN_FMODE(flags) & FMODE_WRITE) || (flags & O_TRUNC)))
+		return false;
+
+	return true;
+}
+
 struct ovl_fh *ovl_encode_fh(struct dentry *lower, bool is_upper)
 {
 	struct ovl_fh *fh;
@@ -305,11 +325,8 @@ static int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
 			return PTR_ERR(fh);
 	}
 
-	/*
-	 * Do not fail when upper doesn't support xattrs.
-	 */
 	err = ovl_check_setxattr(dentry, upper, OVL_XATTR_ORIGIN, fh,
-				 fh ? fh->len : 0, 0);
+				 fh ? fh->len : 0, -EOPNOTSUPP);
 	kfree(fh);
 
 	return err;
@@ -326,6 +343,7 @@ struct ovl_copy_up_ctx {
 	struct qstr destname;
 	struct dentry *workdir;
 	bool tmpfile;
+	bool metacopy;
 };
 
 static int ovl_link_up(struct ovl_copy_up_ctx *c)
@@ -453,10 +471,14 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
 	 *
 	 */
 	err = ovl_set_origin(c->dentry, c->lowerpath.dentry, temp);
-	if (err)
-		return err;
+	if (err) {
+		if (err != -EOPNOTSUPP)
+			return err;
+		/* Upper does not support xattr. Copy up data as well */
+		c->metacopy = false;
+	}
 
-	if (S_ISREG(c->stat.mode)) {
+	if (S_ISREG(c->stat.mode) && !c->metacopy) {
 		struct path upperpath;
 
 		ovl_path_upper(c->dentry, &upperpath);
@@ -601,6 +623,8 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 	if (err)
 		return err;
 
+	ctx.metacopy = ovl_need_meta_copy_up(dentry, ctx.stat.mode, flags);
+
 	ovl_path_upper(parent, &parentpath);
 	ctx.destdir = parentpath.dentry;
 	ctx.destname = dentry->d_name;
-- 
2.13.6

[PATCH v10 09/18] ovl: Add helper ovl_already_copied_up()

[Vivek Goyal]

There are couple of places where we need to know if file is already copied
up (in lockless manner). Right now its open coded and there are only
two conditions to check. Soon this patch series will introduce another
condition to check and Amir wants to introduce one more. So introduce
a helper instead to check this so that code is easier to read.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/copy_up.c   | 19 ++-----------------
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/util.c      | 24 +++++++++++++++++++++++-
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index c5804b87f3c7..65e993e931ba 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -671,21 +671,7 @@ int ovl_copy_up_flags(struct dentry *dentry, int flags)
 		struct dentry *next;
 		struct dentry *parent;
 
-		/*
-		 * Check if copy-up has happened as well as for upper alias (in
-		 * case of hard links) is there.
-		 *
-		 * Both checks are lockless:
-		 *  - false negatives: will recheck under oi->lock
-		 *  - false positives:
-		 *    + ovl_dentry_upper() uses memory barriers to ensure the
-		 *      upper dentry is up-to-date
-		 *    + ovl_dentry_has_upper_alias() relies on locking of
-		 *      upper parent i_rwsem to prevent reordering copy-up
-		 *      with rename.
-		 */
-		if (ovl_dentry_upper(dentry) &&
-		    ovl_dentry_has_upper_alias(dentry))
+		if (ovl_already_copied_up(dentry))
 			break;
 
 		next = dget(dentry);
@@ -712,8 +698,7 @@ int ovl_copy_up_flags(struct dentry *dentry, int flags)
 
 static bool ovl_open_need_copy_up(struct dentry *dentry, int flags)
 {
-	if (ovl_dentry_upper(dentry) &&
-	    ovl_dentry_has_upper_alias(dentry))
+	if (ovl_already_copied_up(dentry))
 		return false;
 
 	if (special_file(d_inode(dentry)->i_mode))
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 31a212781a15..ac5bc3b7ce2b 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -227,6 +227,7 @@ bool ovl_is_whiteout(struct dentry *dentry);
 struct file *ovl_path_open(struct path *path, int flags);
 int ovl_copy_up_start(struct dentry *dentry);
 void ovl_copy_up_end(struct dentry *dentry);
+bool ovl_already_copied_up(struct dentry *dentry);
 bool ovl_check_origin_xattr(struct dentry *dentry);
 bool ovl_check_dir_xattr(struct dentry *dentry, const char *name);
 int ovl_check_setxattr(struct dentry *dentry, struct dentry *upperdentry,
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index d6bb1c9f5e7a..39cd00723918 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -314,13 +314,35 @@ struct file *ovl_path_open(struct path *path, int flags)
 	return dentry_open(path, flags | O_NOATIME, current_cred());
 }
 
+bool ovl_already_copied_up(struct dentry *dentry)
+{
+	/*
+	 * Check if copy-up has happened as well as for upper alias (in
+	 * case of hard links) is there.
+	 *
+	 * Both checks are lockless:
+	 *  - false negatives: will recheck under oi->lock
+	 *  - false positives:
+	 *    + ovl_dentry_upper() uses memory barriers to ensure the
+	 *      upper dentry is up-to-date
+	 *    + ovl_dentry_has_upper_alias() relies on locking of
+	 *      upper parent i_rwsem to prevent reordering copy-up
+	 *      with rename.
+	 */
+	if (ovl_dentry_upper(dentry) &&
+	    ovl_dentry_has_upper_alias(dentry))
+		return true;
+
+	return false;
+}
+
 int ovl_copy_up_start(struct dentry *dentry)
 {
 	struct ovl_inode *oi = OVL_I(d_inode(dentry));
 	int err;
 
 	err = mutex_lock_interruptible(&oi->lock);
-	if (!err && ovl_dentry_has_upper_alias(dentry)) {
+	if (!err && ovl_already_copied_up(dentry)) {
 		err = 1; /* Already copied up */
 		mutex_unlock(&oi->lock);
 	}
-- 
2.13.6

[PATCH v10 10/18] ovl: A new xattr OVL_XATTR_METACOPY for file on upper

[Vivek Goyal]

Now we will have the capability to have upper inodes which might be only
metadata copy up and data is still on lower inode. So add a new xattr
OVL_XATTR_METACOPY to distinguish between two cases.

Presence of OVL_XATTR_METACOPY reflects that file has been copied up
metadata only and and data will be copied up later from lower origin.
So this xattr is set when a metadata copy takes place and cleared when
data copy takes place.

We also use a bit in ovl_inode->flags to cache OVL_UPPERDATA which reflects
whether ovl inode has data or not (as opposed to metadata only copy up).

If a file is copied up metadata only and later when same file is opened
for WRITE, then data copy up takes place. We copy up data, remove METACOPY
xattr and then set the UPPERDATA flag in ovl_inode->flags. While all
these operations happen with oi->lock held, read side of oi->flags can be
lockless. That is another thread on another cpu can check if UPPERDATA
flag is set or not.

So this gives us an ordering requirement w.r.t UPPERDATA flag. That is, if
another cpu sees UPPERDATA flag set, then it should be guaranteed that
effects of data copy up and remove xattr operations are also visible.

For example.

	CPU1				CPU2
ovl_d_real()				acquire(oi->lock)
 ovl_open_maybe_copy_up()                ovl_copy_up_data()
  open_open_need_copy_up()		 vfs_removexattr()
   ovl_already_copied_up()
    ovl_dentry_needs_data_copy_up()	 ovl_set_flag(OVL_UPPERDATA)
     ovl_test_flag(OVL_UPPERDATA)       release(oi->lock)

Say CPU2 is copying up data and in the end sets UPPERDATA flag. But if
CPU1 perceives the effects of setting UPPERDATA flag but not the effects
of preceeding operations (ex. upper that is not fully copied up), it will be
a problem.

Hence this patch introduces smp_wmb() on setting UPPERDATA flag operation
and smp_rmb() on UPPERDATA flag test operation.

May be some other lock or barrier is already covering it. But I am not sure
what that is and is it obvious enough that we will not break it in future.

So hence trying to be safe here and introducing barriers explicitly for
UPPERDATA flag/bit.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/copy_up.c   | 56 +++++++++++++++++++++++++++++++----
 fs/overlayfs/dir.c       |  1 +
 fs/overlayfs/overlayfs.h | 18 ++++++++++--
 fs/overlayfs/super.c     |  1 +
 fs/overlayfs/util.c      | 76 +++++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 141 insertions(+), 11 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 65e993e931ba..0bfc1df13b79 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -195,6 +195,16 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
 	return error;
 }
 
+static int ovl_set_size(struct dentry *upperdentry, struct kstat *stat)
+{
+	struct iattr attr = {
+		.ia_valid = ATTR_SIZE,
+		.ia_size = stat->size,
+	};
+
+	return notify_change(upperdentry, &attr, NULL);
+}
+
 static int ovl_set_timestamps(struct dentry *upperdentry, struct kstat *stat)
 {
 	struct iattr attr = {
@@ -490,8 +500,18 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
 			return err;
 	}
 
+	if (c->metacopy) {
+		err = ovl_check_setxattr(c->dentry, temp, OVL_XATTR_METACOPY,
+					 NULL, 0, -EOPNOTSUPP);
+		if (err)
+			return err;
+	}
+
 	inode_lock(temp->d_inode);
-	err = ovl_set_attr(temp, &c->stat);
+	if (c->metacopy)
+		err = ovl_set_size(temp, &c->stat);
+	if (!err)
+		err = ovl_set_attr(temp, &c->stat);
 	inode_unlock(temp->d_inode);
 
 	return err;
@@ -523,6 +543,8 @@ static int ovl_copy_up_locked(struct ovl_copy_up_ctx *c)
 	if (err)
 		goto out_cleanup;
 
+	if (!c->metacopy)
+		ovl_set_upperdata(d_inode(c->dentry));
 	inode = d_inode(c->dentry);
 	ovl_inode_update(inode, newdentry);
 	if (S_ISDIR(inode->i_mode))
@@ -602,6 +624,28 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 	return err;
 }
 
+/* Copy up data of an inode which was copied up metadata only in the past. */
+static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
+{
+	struct path upperpath;
+	int err;
+
+	ovl_path_upper(c->dentry, &upperpath);
+	if (WARN_ON(upperpath.dentry == NULL))
+		return -EIO;
+
+	err = ovl_copy_up_data(&c->lowerpath, &upperpath, c->stat.size);
+	if (err)
+		return err;
+
+	err = vfs_removexattr(upperpath.dentry, OVL_XATTR_METACOPY);
+	if (err)
+		return err;
+
+	ovl_set_upperdata(d_inode(c->dentry));
+	return err;
+}
+
 static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 			   int flags)
 {
@@ -645,7 +689,7 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 	}
 	ovl_do_check_copy_up(ctx.lowerpath.dentry);
 
-	err = ovl_copy_up_start(dentry);
+	err = ovl_copy_up_start(dentry, flags);
 	/* err < 0: interrupted, err > 0: raced with another copy-up */
 	if (unlikely(err)) {
 		if (err > 0)
@@ -655,6 +699,8 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 			err = ovl_do_copy_up(&ctx);
 		if (!err && !ovl_dentry_has_upper_alias(dentry))
 			err = ovl_link_up(&ctx);
+		if (!err && ovl_dentry_needs_data_copy_up_locked(dentry, flags))
+			err = ovl_copy_up_meta_inode_data(&ctx);
 		ovl_copy_up_end(dentry);
 	}
 	do_delayed_call(&done);
@@ -671,7 +717,7 @@ int ovl_copy_up_flags(struct dentry *dentry, int flags)
 		struct dentry *next;
 		struct dentry *parent;
 
-		if (ovl_already_copied_up(dentry))
+		if (ovl_already_copied_up(dentry, flags))
 			break;
 
 		next = dget(dentry);
@@ -698,13 +744,13 @@ int ovl_copy_up_flags(struct dentry *dentry, int flags)
 
 static bool ovl_open_need_copy_up(struct dentry *dentry, int flags)
 {
-	if (ovl_already_copied_up(dentry))
+	if (ovl_already_copied_up(dentry, flags))
 		return false;
 
 	if (special_file(d_inode(dentry)->i_mode))
 		return false;
 
-	if (!(OPEN_FMODE(flags) & FMODE_WRITE) && !(flags & O_TRUNC))
+	if (!ovl_open_flags_need_copy_up(flags))
 		return false;
 
 	return true;
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index f9788bc116a8..99378071070f 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -158,6 +158,7 @@ static void ovl_instantiate(struct dentry *dentry, struct inode *inode,
 	ovl_dentry_version_inc(dentry->d_parent, false);
 	ovl_dentry_set_upper_alias(dentry);
 	if (!hardlink) {
+		ovl_set_upperdata(inode);
 		ovl_inode_update(inode, newdentry);
 		ovl_copyattr(newdentry->d_inode, inode);
 	} else {
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index ac5bc3b7ce2b..de400606df0c 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -27,6 +27,7 @@ enum ovl_path_type {
 #define OVL_XATTR_ORIGIN OVL_XATTR_PREFIX "origin"
 #define OVL_XATTR_IMPURE OVL_XATTR_PREFIX "impure"
 #define OVL_XATTR_NLINK OVL_XATTR_PREFIX "nlink"
+#define OVL_XATTR_METACOPY OVL_XATTR_PREFIX "metacopy"
 
 enum ovl_flag {
 	/* Pure upper dir that may contain non pure upper entries */
@@ -34,6 +35,7 @@ enum ovl_flag {
 	/* Non-merge dir that may contain whiteout entries */
 	OVL_WHITEOUTS,
 	OVL_INDEX,
+	OVL_UPPERDATA,
 };
 
 /*
@@ -186,6 +188,14 @@ static inline struct dentry *ovl_do_tmpfile(struct dentry *dentry, umode_t mode)
 	return ret;
 }
 
+static inline bool ovl_open_flags_need_copy_up(int flags)
+{
+	if (!flags)
+		return false;
+
+	return ((OPEN_FMODE(flags) & FMODE_WRITE) || (flags & O_TRUNC));
+}
+
 /* util.c */
 int ovl_want_write(struct dentry *dentry);
 void ovl_drop_write(struct dentry *dentry);
@@ -215,6 +225,10 @@ bool ovl_dentry_is_whiteout(struct dentry *dentry);
 void ovl_dentry_set_opaque(struct dentry *dentry);
 bool ovl_dentry_has_upper_alias(struct dentry *dentry);
 void ovl_dentry_set_upper_alias(struct dentry *dentry);
+bool ovl_dentry_needs_data_copy_up(struct dentry *dentry, int flags);
+bool ovl_dentry_needs_data_copy_up_locked(struct dentry *dentry, int flags);
+bool ovl_has_upperdata(struct dentry *dentry);
+void ovl_set_upperdata(struct inode *inode);
 bool ovl_redirect_dir(struct super_block *sb);
 const char *ovl_dentry_get_redirect(struct dentry *dentry);
 void ovl_dentry_set_redirect(struct dentry *dentry, const char *redirect);
@@ -225,9 +239,9 @@ void ovl_dentry_version_inc(struct dentry *dentry, bool impurity);
 u64 ovl_dentry_version_get(struct dentry *dentry);
 bool ovl_is_whiteout(struct dentry *dentry);
 struct file *ovl_path_open(struct path *path, int flags);
-int ovl_copy_up_start(struct dentry *dentry);
+int ovl_copy_up_start(struct dentry *dentry, int flags);
 void ovl_copy_up_end(struct dentry *dentry);
-bool ovl_already_copied_up(struct dentry *dentry);
+bool ovl_already_copied_up(struct dentry *dentry, int flags);
 bool ovl_check_origin_xattr(struct dentry *dentry);
 bool ovl_check_dir_xattr(struct dentry *dentry, const char *name);
 int ovl_check_setxattr(struct dentry *dentry, struct dentry *upperdentry,
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 73cb90eeda0f..a6e3719a599b 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1326,6 +1326,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 
 	/* Root is always merge -> can have whiteouts */
 	ovl_set_flag(OVL_WHITEOUTS, d_inode(root_dentry));
+	ovl_set_upperdata(d_inode(root_dentry));
 	ovl_inode_init(d_inode(root_dentry), upperpath.dentry,
 		       ovl_dentry_lower(root_dentry));
 
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 39cd00723918..3b509b691032 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -231,6 +231,62 @@ void ovl_dentry_set_upper_alias(struct dentry *dentry)
 	oe->has_upper = true;
 }
 
+static bool ovl_should_check_upperdata(struct dentry *dentry)
+{
+	if (!S_ISREG(d_inode(dentry)->i_mode))
+		return false;
+
+	if (!ovl_dentry_lower(dentry))
+		return false;
+
+	return true;
+}
+
+bool ovl_has_upperdata(struct dentry *dentry)
+{
+	if (!ovl_should_check_upperdata(dentry))
+		return true;
+
+	if (!ovl_test_flag(OVL_UPPERDATA, d_inode(dentry)))
+		return false;
+	/*
+	 * Pairs with smp_wmb() in ovl_set_upperdata(). Main user of
+	 * ovl_has_upperdata() is ovl_copy_up_meta_inode_data(). Make sure
+	 * if setting of OVL_UPPERDATA is visible, then effects of writes
+	 * before that are visible too.
+	 */
+	smp_rmb();
+	return true;
+}
+
+void ovl_set_upperdata(struct inode *inode)
+{
+	/*
+	 * Pairs with smp_rmb() in ovl_has_upperdata(). Make sure
+	 * if OVL_UPPERDATA flag is visible, then effects of write operations
+	 * before it are visible as well.
+	 */
+	smp_wmb();
+	ovl_set_flag(OVL_UPPERDATA, inode);
+}
+
+/* Caller should hold ovl_inode->lock */
+bool ovl_dentry_needs_data_copy_up_locked(struct dentry *dentry, int flags)
+{
+	if (!ovl_open_flags_need_copy_up(flags))
+		return false;
+
+	return !ovl_test_flag(OVL_UPPERDATA, d_inode(dentry));
+}
+
+bool ovl_dentry_needs_data_copy_up(struct dentry *dentry, int flags)
+{
+	if (!ovl_open_flags_need_copy_up(flags))
+		return false;
+
+	return !ovl_has_upperdata(dentry);
+}
+
 bool ovl_redirect_dir(struct super_block *sb)
 {
 	struct ovl_fs *ofs = sb->s_fs_info;
@@ -314,7 +370,18 @@ struct file *ovl_path_open(struct path *path, int flags)
 	return dentry_open(path, flags | O_NOATIME, current_cred());
 }
 
-bool ovl_already_copied_up(struct dentry *dentry)
+/* Caller should hold ovl_inode->lock */
+static bool ovl_already_copied_up_locked(struct dentry *dentry, int flags)
+{
+	if (ovl_dentry_upper(dentry) &&
+	    ovl_dentry_has_upper_alias(dentry) &&
+	    !ovl_dentry_needs_data_copy_up_locked(dentry, flags))
+		return true;
+
+	return false;
+}
+
+bool ovl_already_copied_up(struct dentry *dentry, int flags)
 {
 	/*
 	 * Check if copy-up has happened as well as for upper alias (in
@@ -330,19 +397,20 @@ bool ovl_already_copied_up(struct dentry *dentry)
 	 *      with rename.
 	 */
 	if (ovl_dentry_upper(dentry) &&
-	    ovl_dentry_has_upper_alias(dentry))
+	    ovl_dentry_has_upper_alias(dentry) &&
+	    !ovl_dentry_needs_data_copy_up(dentry, flags))
 		return true;
 
 	return false;
 }
 
-int ovl_copy_up_start(struct dentry *dentry)
+int ovl_copy_up_start(struct dentry *dentry, int flags)
 {
 	struct ovl_inode *oi = OVL_I(d_inode(dentry));
 	int err;
 
 	err = mutex_lock_interruptible(&oi->lock);
-	if (!err && ovl_already_copied_up(dentry)) {
+	if (!err && ovl_already_copied_up_locked(dentry, flags)) {
 		err = 1; /* Already copied up */
 		mutex_unlock(&oi->lock);
 	}
-- 
2.13.6

[PATCH v10 11/18] ovl: Set OVL_UPPERDATA flag during ovl_lookup()

[Vivek Goyal]

During lookup, check for presence of OVL_XATTR_METACOPY and if not present,
set OVL_UPPERDATA bit in flags.

OVL_UPPERDATA flag is set unconditionally if upper inode exists.

Do not follow metacopy origin if we find a metacopy only inode and metacopy
feature is not enabled for that mount. Like redirect, this can have security
implications where an attacker could hand craft upper and try to gain
access to file on lower which it should not have to begin with.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/namei.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 5b834abde37a..d4ad9408749f 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -25,6 +25,59 @@ struct ovl_lookup_data {
 	char *redirect;
 };
 
+/* err < 0, 0 if no metacopy xattr, 1 if metacopy xattr found */
+static int ovl_check_metacopy_xattr(struct dentry *dentry)
+{
+	int res;
+
+	/* Only regular files can have metacopy xattr */
+	if (!S_ISREG(d_inode(dentry)->i_mode))
+		return 0;
+
+	res = vfs_getxattr(dentry, OVL_XATTR_METACOPY, NULL, 0);
+	if (res < 0) {
+		if (res == -ENODATA || res == -EOPNOTSUPP)
+			return 0;
+		goto out;
+	}
+
+	return 1;
+out:
+	pr_warn_ratelimited("overlayfs: failed to get metacopy (%i)\n", res);
+	return res;
+}
+
+/* err < 0, 0 if upper does not have metacopy, 1 if metacopy found */
+static int ovl_check_metacopy(struct ovl_fs *ofs, struct dentry *dentry,
+			      unsigned int ctr)
+{
+	int metacopy;
+
+	metacopy = ovl_check_metacopy_xattr(dentry);
+	if (metacopy <= 0 )
+		return metacopy;
+
+	if (!ctr) {
+		/*
+		 * Found a dentry with metacopy set but at the same time
+		 * there is no corresponding lower data dentry. Something is
+		 * not right.
+		 */
+		return -ESTALE;
+	}
+
+	if (!ofs->config.metacopy) {
+		/*
+		 * Do not follow metacopy origin if metacopy feature is not
+		 * enabled. This can be a security issue (Like redirect).
+		 */
+		pr_warn_ratelimited("overlay: refusing to follow metacopy origin for (%pd2)\n", dentry);
+		return -EPERM;
+	}
+
+	return metacopy;
+}
+
 static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
 			      size_t prelen, const char *post)
 {
@@ -624,6 +677,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	struct dentry *this;
 	unsigned int i;
 	int err;
+	bool metacopy = false;
 	bool origin_chain = ofs->config.metacopy ? true : false;
 	struct ovl_lookup_data d = {
 		.name = dentry->d_name,
@@ -666,6 +720,11 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 					       origin_chain);
 			if (err)
 				goto out_put_upper;
+
+			err = ovl_check_metacopy(ofs, upperdentry, ctr);
+			metacopy = err;
+			if (err < 0)
+				goto out_put_upper;
 		}
 
 		if (d.redirect) {
@@ -766,6 +825,9 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		OVL_I(inode)->redirect = upperredirect;
 		if (index)
 			ovl_set_flag(OVL_INDEX, inode);
+
+		if (upperdentry && !metacopy)
+			ovl_set_flag(OVL_UPPERDATA, inode);
 	}
 
 	revert_creds(old_cred);
-- 
2.13.6

[PATCH v10 12/18] ovl: Setup origin chain for lower regular files

[Vivek Goyal]

So far we look for origin only if upper dentry is found. And we look for
origin chain if metacopy feature is enabled.

But with midlayer metacopy support it is possible that a no upper dentry
is there and lower itself is metacopy inode. In that case also we need to
setup origin chain and keep track of which inode is data inode so that
later we can copy up the data from data inode.

So this patch, introduces logic to setup origin chain when upper dentry
is not present. As of now only metacopy feature needs it so it is done
only if metacopy feature is enabled.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/namei.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index d4ad9408749f..99f896e7bcf2 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -802,6 +802,24 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		}
 	}
 
+	if (origin_chain && !upperdentry && ctr && !d.is_dir) {
+		int nr_origins = 0;
+		struct ovl_path *stackptr = &stack[1];
+		/*
+		 * If upper is present, we already followed origin. If not,
+		 * follow origin now.
+		 */
+		i = ovl_find_layer(ofs, &stack[0]);
+		i++;
+		err = ovl_check_origin(stack[0].dentry, &roe->lowerstack[i],
+				       ofs->numlower - i, &stackptr,
+				       &nr_origins, true);
+		if (err)
+			goto out_put;
+
+		ctr += nr_origins;
+	}
+
 	oe = ovl_alloc_entry(ctr);
 	err = -ENOMEM;
 	if (!oe)
-- 
2.13.6

[PATCH v10 13/18] ovl: Check metacopy attributes on a chain of origin

[Vivek Goyal]

So far our check for metacopy is simple. That is we check for presence
of OVL_XATTR_METACOPY only on upper inode. But with the possibility of
metacopy inode being present on lower layer also, we should check it
on the whole origin chain and keep track of which lower dentries are
metacopy.

So this patch adds enahnces ovl_check_metacopy() so that it checks
for metacopy not only on dentry being passed, but also travels through
the chain of origins for that dentry and checks metacopy xattr.

We also need to remember metacopy state of each dentry/inode so that
we don't have to look it up all the time. Hence I added a field
"metacopy" in "struct ovl_path" and set this when a particular dentry
is metacopy only. This only applies to lower dentries as we keep
ovl_path only for lower dentries. Upper dentry continues to rely on
ovl inode flag OVL_UPPERDATA to figure out if upper is metacopy
only inode or not.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/namei.c     | 45 ++++++++++++++++++++++++++++++++++++++++-----
 fs/overlayfs/ovl_entry.h |  1 +
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 99f896e7bcf2..f6f0d97bd0ca 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -47,11 +47,36 @@ static int ovl_check_metacopy_xattr(struct dentry *dentry)
 	return res;
 }
 
+static int ovl_check_origin_metacopy(struct ovl_path *stackp, unsigned int ctr)
+{
+	int i, metacopy;
+
+	if (!ctr)
+		return 0;
+
+	for (i = 0; i < ctr; i++) {
+		metacopy = ovl_check_metacopy_xattr(stackp[i].dentry);
+		if (metacopy <= 0)
+			return metacopy;
+		stackp[i].metacopy = true;
+	}
+
+	if (stackp[i - 1].metacopy) {
+		/*
+		 * Last origin should not have metacopy set. It should be
+		 * the source of metacopy. Something is wrong.
+		 */
+		return -EIO;
+	}
+
+	return 0;
+}
+
 /* err < 0, 0 if upper does not have metacopy, 1 if metacopy found */
 static int ovl_check_metacopy(struct ovl_fs *ofs, struct dentry *dentry,
-			      unsigned int ctr)
+			      struct ovl_path *stackp, unsigned int ctr)
 {
-	int metacopy;
+	int metacopy, err;
 
 	metacopy = ovl_check_metacopy_xattr(dentry);
 	if (metacopy <= 0 )
@@ -75,6 +100,9 @@ static int ovl_check_metacopy(struct ovl_fs *ofs, struct dentry *dentry,
 		return -EPERM;
 	}
 
+	err = ovl_check_origin_metacopy(stackp, ctr);
+	if (err)
+		return err;
 	return metacopy;
 }
 
@@ -348,8 +376,6 @@ static int ovl_check_origin(struct dentry *dentry,
 	bool mem_allocated = false;
 	struct ovl_path *origin_stack;
 
-	BUG_ON(*ctrp);
-
 	if (!*stackp) {
 		origin_stack = kmalloc(sizeof(struct ovl_path) * nr_path, GFP_KERNEL);
 		if (!origin_stack)
@@ -721,7 +747,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			if (err)
 				goto out_put_upper;
 
-			err = ovl_check_metacopy(ofs, upperdentry, ctr);
+			err = ovl_check_metacopy(ofs, upperdentry, stack, ctr);
 			metacopy = err;
 			if (err < 0)
 				goto out_put_upper;
@@ -818,6 +844,15 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			goto out_put;
 
 		ctr += nr_origins;
+
+		err = ovl_check_metacopy(ofs, stack[0].dentry, &stack[1],
+					 nr_origins);
+		metacopy = err;
+		if (err < 0)
+			goto out_put;
+
+		if (metacopy)
+			stack[0].metacopy = true;
 	}
 
 	oe = ovl_alloc_entry(ctr);
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 92a4db0bd18c..a8cfdae4df89 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -28,6 +28,7 @@ struct ovl_layer {
 struct ovl_path {
 	struct ovl_layer *layer;
 	struct dentry *dentry;
+	bool metacopy;
 };
 
 /* private information held for overlayfs's superblock */
-- 
2.13.6

[PATCH v10 14/18] ovl: Do not mark a non dir as _OVL_PATH_MERGE in ovl_path_type()

[Vivek Goyal]

With the addition of an origin chain for regular files, it is possible
that we don't have an upperdentry and oe->numlower > 1 for non-dir. So
mark a path __OVL_TYPE_MERGE only if it is a directory.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 3b509b691032..e717d561019d 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -106,7 +106,7 @@ enum ovl_path_type ovl_path_type(struct dentry *dentry)
 				type |= __OVL_PATH_MERGE;
 		}
 	} else {
-		if (oe->numlower > 1)
+		if (oe->numlower > 1 && d_is_dir(dentry))
 			type |= __OVL_PATH_MERGE;
 	}
 	return type;
-- 
2.13.6

[PATCH v10 15/18] ovl: Copy up meta inode data from lowest data inode

[Vivek Goyal]

So far lower could not be a meta inode. So whenever it was time to copy
up data of a meta inode, we could copy it up from top most lower dentry.

But now lower itself can be a metacopy inode. That means data copy up
needs to take place from a data inode in metacopy inode chain. Find
lower data inode in the chain and use that for data copy up.

Introduced a helper called ovl_path_lowerdata() to find the lower
data inode chain.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/copy_up.c   | 14 ++++++++++----
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/util.c      | 19 +++++++++++++++++++
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 0bfc1df13b79..f4ece4754725 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -489,13 +489,15 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
 	}
 
 	if (S_ISREG(c->stat.mode) && !c->metacopy) {
-		struct path upperpath;
+		struct path upperpath, datapath;
 
 		ovl_path_upper(c->dentry, &upperpath);
 		BUG_ON(upperpath.dentry != NULL);
 		upperpath.dentry = temp;
 
-		err = ovl_copy_up_data(&c->lowerpath, &upperpath, c->stat.size);
+		ovl_path_lowerdata(c->dentry, &datapath);
+		BUG_ON(datapath.dentry == NULL);
+		err = ovl_copy_up_data(&datapath, &upperpath, c->stat.size);
 		if (err)
 			return err;
 	}
@@ -627,14 +629,18 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 /* Copy up data of an inode which was copied up metadata only in the past. */
 static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
 {
-	struct path upperpath;
+	struct path upperpath, datapath;
 	int err;
 
 	ovl_path_upper(c->dentry, &upperpath);
 	if (WARN_ON(upperpath.dentry == NULL))
 		return -EIO;
 
-	err = ovl_copy_up_data(&c->lowerpath, &upperpath, c->stat.size);
+	ovl_path_lowerdata(c->dentry, &datapath);
+	if (WARN_ON(datapath.dentry == NULL))
+		return -EIO;
+
+	err = ovl_copy_up_data(&datapath, &upperpath, c->stat.size);
 	if (err)
 		return err;
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index de400606df0c..9ec65f0b5384 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -210,6 +210,7 @@ bool ovl_dentry_weird(struct dentry *dentry);
 enum ovl_path_type ovl_path_type(struct dentry *dentry);
 void ovl_path_upper(struct dentry *dentry, struct path *path);
 void ovl_path_lower(struct dentry *dentry, struct path *path);
+void ovl_path_lowerdata(struct dentry *dentry, struct path *path);
 enum ovl_path_type ovl_path_real(struct dentry *dentry, struct path *path);
 struct dentry *ovl_dentry_upper(struct dentry *dentry);
 struct dentry *ovl_dentry_lower(struct dentry *dentry);
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index e717d561019d..9cd4187cb043 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -132,6 +132,25 @@ void ovl_path_lower(struct dentry *dentry, struct path *path)
 	}
 }
 
+void ovl_path_lowerdata(struct dentry *dentry, struct path *path)
+{
+	struct ovl_entry *oe = dentry->d_fsdata;
+	int i;
+
+	if (!oe->numlower) {
+		*path = (struct path) { };
+		return;
+	}
+
+	for (i = 0 ; i < oe->numlower; i++) {
+		if (oe->lowerstack[i].metacopy)
+			continue;
+		path->mnt = oe->lowerstack[i].layer->mnt;
+		path->dentry = oe->lowerstack[i].dentry;
+		return;
+	}
+}
+
 enum ovl_path_type ovl_path_real(struct dentry *dentry, struct path *path)
 {
 	enum ovl_path_type type = ovl_path_type(dentry);
-- 
2.13.6

[PATCH v10 16/18] ovl: Fix ovl_getattr() to get number of blocks from lower

[Vivek Goyal]

If an inode has been copied up metadata only, then we need to query the
number of blocks from lower and fill up the stat->st_blocks.

We need to be careful about races where we are doing stat on one cpu and
data copy up is taking place on other cpu. We want to return
stat->st_blocks either from lower or stable upper and not something in
between. Hence, ovl_has_upperdata() is called first to figure out whether
block reporting will take place from lower or upper.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/inode.c     | 17 ++++++++++++++++-
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/util.c      | 13 +++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 5239a1ff7211..801bfcce2ad8 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -76,6 +76,9 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
 	bool is_dir = S_ISDIR(dentry->d_inode->i_mode);
 	bool samefs = ovl_same_sb(dentry->d_sb);
 	int err;
+	bool metacopy = false;
+
+	metacopy = ovl_is_metacopy_dentry(dentry);
 
 	type = ovl_path_real(dentry, &realpath);
 	old_cred = ovl_override_creds(dentry->d_sb);
@@ -93,7 +96,8 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
 	if (!is_dir || samefs) {
 		if (OVL_TYPE_ORIGIN(type)) {
 			struct kstat lowerstat;
-			u32 lowermask = STATX_INO | (!is_dir ? STATX_NLINK : 0);
+			u32 lowermask = STATX_INO | STATX_BLOCKS |
+					(!is_dir ? STATX_NLINK : 0);
 
 			ovl_path_lower(dentry, &realpath);
 			err = vfs_getattr(&realpath, &lowerstat,
@@ -118,6 +122,17 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
 			else
 				stat->dev = ovl_get_pseudo_dev(dentry);
 		}
+		if (metacopy) {
+			struct kstat lowerdatastat;
+			u32 lowermask = STATX_BLOCKS;
+
+			ovl_path_lowerdata(dentry, &realpath);
+			err = vfs_getattr(&realpath, &lowerdatastat,
+					  lowermask, flags);
+			if (err)
+				goto out;
+			stat->blocks = lowerdatastat.blocks;
+		}
 		if (samefs) {
 			/*
 			 * When all layers are on the same fs, all real inode
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 9ec65f0b5384..b663a65826a7 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -257,6 +257,7 @@ void ovl_inuse_unlock(struct dentry *dentry);
 int ovl_nlink_start(struct dentry *dentry, bool *locked);
 void ovl_nlink_end(struct dentry *dentry, bool locked);
 int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir);
+bool ovl_is_metacopy_dentry(struct dentry *dentry);
 
 static inline bool ovl_is_impuredir(struct dentry *dentry)
 {
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 9cd4187cb043..8d6cf8ceb6ff 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -705,3 +705,16 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir)
 	pr_err("overlayfs: failed to lock workdir+upperdir\n");
 	return -EIO;
 }
+
+bool ovl_is_metacopy_dentry(struct dentry *dentry)
+{
+	struct ovl_entry *oe = dentry->d_fsdata;
+
+	if (ovl_dentry_upper(dentry)) {
+		if (!ovl_has_upperdata(dentry))
+			return true;
+		return false;
+	}
+
+	return oe->lowerstack[0].metacopy;
+}
-- 
2.13.6

[PATCH v10 17/18] ovl: Do not expose metacopy only upper dentry from d_real()

[Vivek Goyal]

d_real() can make a upper metacopy dentry/inode visible to the vfs layer.
This is something new and vfs layer does not know that this inode contains
only metadata and not data. And this could break things.

So to be safe, do not expose metacopy only dentry/inode to vfs using
d_real().

IOW, d_real() will not reuturn metacopy dentry. Instead, it will return
dentry corresponding lower dentry/inode which has file data.

For regular d_real() call (inode == NULL, D_REAL_UPPER not set), if upper
dentry inode is metacopy only and does not have data, return lower dentry.

If d_real() is called with flag D_REAL_UPPER, return upper dentry only if
it has data (flag OVL_UPPERDATA is set).

Similiarly, if d_real(inode=X) is called, a warning is emitted if returned
dentry/inode does not have OVL_UPPERDATA set. This should not happen as
we never made this metacopy inode visible to vfs so nobody should be
calling overlayfs back with inode=metacopy_inode.

I scanned the code and I don't think it breaks any of the existing code.
There are two users of D_REAL_UPPER. may_write_real() and
update_ovl_inode_times().

may_write_real(), will get an NULL dentry if upper inode is metacopy only
and it will return -EPERM. Effectively, we are disallowing modifications
to metacopy only inode from this interface. Though there is opportunity
to improve it. (Allow chattr on metacopy inodes).

update_ovl_inode_times() gets inode mtime and ctime from real inode. It
should not be broken for metacopy inode as well for following reasons.

- For any metadata operations (setattr, acl etc), overlay always calls
  ovl_copyattr() and updates ovl inode mtime and ctime. So there is no
  need to update mtime and ctime in this case. Its already updated, hence
  even if d_real(D_REAL_UPPER) returns nil, it should be fine.

- For metadata inode, mtime should be same as lower and not change. (data
  can't be modified on metadata inode without copyup). IOW, mtime of
  ovl dentry should be same as mtime of underlying metadata inode on upper
  always. So there is no need to update it.

- For file writes, ctime and mtime will be updated. But in that case
  first data will be copied up and this will not be a metadata inode
  anymore. And furthr call to d_real(D_REAL_UPPER) will return upper
  inode and new mtime and ctime will be obtainable.

So atime updates should work just fine for metacopy inodes. I think only
corner case is if somehow underlying filesystem changes ctime of upper
metadata inode without overlay knowing about it. Not sure how that
can happen. If somehow is affected by that, then we probably can implement
another flag which will allow caller to get metacopy inode as well.
Something like d_real(D_REAL_UPPER | D_METACOPY). And that should solve
this issue.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/super.c     | 21 +++++++++++++++++----
 fs/overlayfs/util.c      | 15 +++++++++++++++
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index b663a65826a7..c712ab7274f9 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -214,6 +214,7 @@ void ovl_path_lowerdata(struct dentry *dentry, struct path *path);
 enum ovl_path_type ovl_path_real(struct dentry *dentry, struct path *path);
 struct dentry *ovl_dentry_upper(struct dentry *dentry);
 struct dentry *ovl_dentry_lower(struct dentry *dentry);
+struct dentry *ovl_dentry_lowerdata(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);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index a6e3719a599b..f5cacae8269a 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -91,8 +91,14 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
 	struct dentry *real;
 	int err;
 
-	if (flags & D_REAL_UPPER)
-		return ovl_dentry_upper(dentry);
+	if (flags & D_REAL_UPPER) {
+		real = ovl_dentry_upper(dentry);
+		if (!real)
+			return NULL;
+		if (!ovl_has_upperdata(dentry))
+			return NULL;
+		return real;
+	}
 
 	if (!d_is_reg(dentry)) {
 		if (!inode || inode == d_inode(dentry))
@@ -108,15 +114,22 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
 
 	real = ovl_dentry_upper(dentry);
 	if (real && (!inode || inode == d_inode(real))) {
+		bool metacopy = !ovl_has_upperdata(dentry);
 		if (!inode) {
 			err = ovl_check_append_only(d_inode(real), open_flags);
 			if (err)
 				return ERR_PTR(err);
-		}
+
+			if (unlikely(metacopy))
+				goto lower;
+		} else if (unlikely(metacopy))
+			goto bug;
+
 		return real;
 	}
 
-	real = ovl_dentry_lower(dentry);
+lower:
+	real = ovl_dentry_lowerdata(dentry);
 	if (!real)
 		goto bug;
 
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 8d6cf8ceb6ff..f57f2cfccbd4 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -175,6 +175,21 @@ struct dentry *ovl_dentry_lower(struct dentry *dentry)
 	return oe->numlower ? oe->lowerstack[0].dentry : NULL;
 }
 
+struct dentry *ovl_dentry_lowerdata(struct dentry *dentry)
+{
+	struct ovl_entry *oe = dentry->d_fsdata;
+	int i;
+
+	for(i = 0 ; i < oe->numlower; i++) {
+		if (oe->lowerstack[i].metacopy)
+			continue;
+
+		return oe->lowerstack[i].dentry;
+	}
+
+	return NULL;
+}
+
 struct dentry *ovl_dentry_real(struct dentry *dentry)
 {
 	return ovl_dentry_upper(dentry) ?: ovl_dentry_lower(dentry);
-- 
2.13.6

[PATCH v10 18/18] ovl: Enable metadata only feature

[Vivek Goyal]

All the bits are in patches before this. So it is time to enable the
metadata only copy up feature.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/copy_up.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index f4ece4754725..c2ce2b9c4ebe 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -247,9 +247,6 @@ static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode,
 {
 	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
 
-	/* TODO: Will enable metacopy in last patch of series */
-	return false;
-
 	if (!ofs->config.metacopy)
 		return false;
 
-- 
2.13.6

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Amir Goldstein]

On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> There is a need to support metacopy dentry in midlayer. That means there
> could be a chain of metacopy dentries.
>
> For example, upper could be metacopy, midlayer lower could be metacopy and
> lowest layer could be actual data inode. This means when we copy up actual
> data, we should be able to reach to lowest data inode and copy up data from
> there. And that means we should keep track of all the dentries in origin
> chain which lead to data inode.
>
> Current ovl_check_origin() logic only looks for one origin dentry. This patch
> enhances ovl_check_origin() to continue to follow origin chain and return
> all the origin entries found. This is done only if caller of the function
> set "follow_chain" argument.
>

We don't really need to keep the entire chain do we?
We can follow chain but keep only the one inode that is not a metacopy inode.
All the rest are useless, no?
Then we don't create a new type of object - non-dir with numlower > 1.

Amir.

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Amir Goldstein]

On Mon, Jan 22, 2018 at 9:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> There is a need to support metacopy dentry in midlayer. That means there
>> could be a chain of metacopy dentries.
>>
>> For example, upper could be metacopy, midlayer lower could be metacopy and
>> lowest layer could be actual data inode. This means when we copy up actual
>> data, we should be able to reach to lowest data inode and copy up data from
>> there. And that means we should keep track of all the dentries in origin
>> chain which lead to data inode.
>>
>> Current ovl_check_origin() logic only looks for one origin dentry. This patch
>> enhances ovl_check_origin() to continue to follow origin chain and return
>> all the origin entries found. This is done only if caller of the function
>> set "follow_chain" argument.
>>
>
> We don't really need to keep the entire chain do we?
> We can follow chain but keep only the one inode that is not a metacopy inode.
> All the rest are useless, no?
> Then we don't create a new type of object - non-dir with numlower > 1.
>

Seems like if you don't keep the entire chain, then no need for
patches 14 and 15.
Also with upper metacopy, you can fix upper origin xattr after
following to the data
origin and forget about middle layer metacopies forever.

Am I missing something?

Thanks,
Amir.

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Vivek Goyal]

On Mon, Jan 22, 2018 at 09:34:20PM +0200, Amir Goldstein wrote:
> On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > There is a need to support metacopy dentry in midlayer. That means there
> > could be a chain of metacopy dentries.
> >
> > For example, upper could be metacopy, midlayer lower could be metacopy and
> > lowest layer could be actual data inode. This means when we copy up actual
> > data, we should be able to reach to lowest data inode and copy up data from
> > there. And that means we should keep track of all the dentries in origin
> > chain which lead to data inode.
> >
> > Current ovl_check_origin() logic only looks for one origin dentry. This patch
> > enhances ovl_check_origin() to continue to follow origin chain and return
> > all the origin entries found. This is done only if caller of the function
> > set "follow_chain" argument.
> >
> 
> We don't really need to keep the entire chain do we?
> We can follow chain but keep only the one inode that is not a metacopy inode.
> All the rest are useless, no?
> Then we don't create a new type of object - non-dir with numlower > 1.

Hi Amir,

I think we will have to keep atleast 2. One is actual data inode and
other is top most lower metacopy inode. This top most metacopy inode
will be used for copy up operation and upper file will use this metadata.
We can't use metadata of data inode.

Vivek

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Vivek Goyal]

On Mon, Jan 22, 2018 at 09:46:33PM +0200, Amir Goldstein wrote:
> On Mon, Jan 22, 2018 at 9:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >> There is a need to support metacopy dentry in midlayer. That means there
> >> could be a chain of metacopy dentries.
> >>
> >> For example, upper could be metacopy, midlayer lower could be metacopy and
> >> lowest layer could be actual data inode. This means when we copy up actual
> >> data, we should be able to reach to lowest data inode and copy up data from
> >> there. And that means we should keep track of all the dentries in origin
> >> chain which lead to data inode.
> >>
> >> Current ovl_check_origin() logic only looks for one origin dentry. This patch
> >> enhances ovl_check_origin() to continue to follow origin chain and return
> >> all the origin entries found. This is done only if caller of the function
> >> set "follow_chain" argument.
> >>
> >
> > We don't really need to keep the entire chain do we?
> > We can follow chain but keep only the one inode that is not a metacopy inode.
> > All the rest are useless, no?
> > Then we don't create a new type of object - non-dir with numlower > 1.
> >
> 
> Seems like if you don't keep the entire chain, then no need for
> patches 14 and 15.

I will need to have atleast 2 lower dentries. One will be top most
metadata copy and other lower most data dentry. IOW, both the ends of
the chain need to be there. 

> Also with upper metacopy, you can fix upper origin xattr after
> following to the data
> origin and forget about middle layer metacopies forever.

If upper metacopy is alreday there, then I agree that lower top most
becomes inner node of chain and we can get rid of it.

> 
> Am I missing something?

I think you are missing the case when there is no upper and lower has
a metacopy chain. In that case we need to retain two dentries. One for
data copy up and one for metadata copy up.

Vivek

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Amir Goldstein]

On Tue, Jan 23, 2018 at 3:45 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Mon, Jan 22, 2018 at 09:46:33PM +0200, Amir Goldstein wrote:
>> On Mon, Jan 22, 2018 at 9:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> > On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> >> There is a need to support metacopy dentry in midlayer. That means there
>> >> could be a chain of metacopy dentries.
>> >>
>> >> For example, upper could be metacopy, midlayer lower could be metacopy and
>> >> lowest layer could be actual data inode. This means when we copy up actual
>> >> data, we should be able to reach to lowest data inode and copy up data from
>> >> there. And that means we should keep track of all the dentries in origin
>> >> chain which lead to data inode.
>> >>
>> >> Current ovl_check_origin() logic only looks for one origin dentry. This patch
>> >> enhances ovl_check_origin() to continue to follow origin chain and return
>> >> all the origin entries found. This is done only if caller of the function
>> >> set "follow_chain" argument.
>> >>
>> >
>> > We don't really need to keep the entire chain do we?
>> > We can follow chain but keep only the one inode that is not a metacopy inode.
>> > All the rest are useless, no?
>> > Then we don't create a new type of object - non-dir with numlower > 1.
>> >
>>
>> Seems like if you don't keep the entire chain, then no need for
>> patches 14 and 15.
>
> I will need to have atleast 2 lower dentries. One will be top most
> metadata copy and other lower most data dentry. IOW, both the ends of
> the chain need to be there.
>
>> Also with upper metacopy, you can fix upper origin xattr after
>> following to the data
>> origin and forget about middle layer metacopies forever.
>
> If upper metacopy is alreday there, then I agree that lower top most
> becomes inner node of chain and we can get rid of it.
>
>>
>> Am I missing something?
>
> I think you are missing the case when there is no upper and lower has
> a metacopy chain. In that case we need to retain two dentries. One for
> data copy up and one for metadata copy up.
>

OK. so you can get rid of all the middle metacopies while following
the origin chain. no reason to keep those.

Thanks,
Amir.

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Vivek Goyal]

On Tue, Jan 23, 2018 at 03:47:29PM +0200, Amir Goldstein wrote:
> On Tue, Jan 23, 2018 at 3:45 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Mon, Jan 22, 2018 at 09:46:33PM +0200, Amir Goldstein wrote:
> >> On Mon, Jan 22, 2018 at 9:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> >> > On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >> >> There is a need to support metacopy dentry in midlayer. That means there
> >> >> could be a chain of metacopy dentries.
> >> >>
> >> >> For example, upper could be metacopy, midlayer lower could be metacopy and
> >> >> lowest layer could be actual data inode. This means when we copy up actual
> >> >> data, we should be able to reach to lowest data inode and copy up data from
> >> >> there. And that means we should keep track of all the dentries in origin
> >> >> chain which lead to data inode.
> >> >>
> >> >> Current ovl_check_origin() logic only looks for one origin dentry. This patch
> >> >> enhances ovl_check_origin() to continue to follow origin chain and return
> >> >> all the origin entries found. This is done only if caller of the function
> >> >> set "follow_chain" argument.
> >> >>
> >> >
> >> > We don't really need to keep the entire chain do we?
> >> > We can follow chain but keep only the one inode that is not a metacopy inode.
> >> > All the rest are useless, no?
> >> > Then we don't create a new type of object - non-dir with numlower > 1.
> >> >
> >>
> >> Seems like if you don't keep the entire chain, then no need for
> >> patches 14 and 15.
> >
> > I will need to have atleast 2 lower dentries. One will be top most
> > metadata copy and other lower most data dentry. IOW, both the ends of
> > the chain need to be there.
> >
> >> Also with upper metacopy, you can fix upper origin xattr after
> >> following to the data
> >> origin and forget about middle layer metacopies forever.
> >
> > If upper metacopy is alreday there, then I agree that lower top most
> > becomes inner node of chain and we can get rid of it.
> >
> >>
> >> Am I missing something?
> >
> > I think you are missing the case when there is no upper and lower has
> > a metacopy chain. In that case we need to retain two dentries. One for
> > data copy up and one for metadata copy up.
> >
> 
> OK. so you can get rid of all the middle metacopies while following
> the origin chain. no reason to keep those.

Sure, will do that. As of now there are no users of intermediate nodes.
Once somebody needs these, code can be modified.

Vivek

Re: [PATCH v10 00/18] overlayfs: Delayed copy up of data

[Miklos Szeredi]

On Mon, Jan 22, 2018 at 7:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> Hi,
>
> Please find attached V10 of the metadata only copy up patches. I think
> I have taken care of review comments and I will start writing automated
> test cases now.
>
> One thing which concerns me about these patches is the fact that during
> file open d_real() will return lower inode which actually contains file
> data and that inode will be installed in f->f_inode. So while f->f_inode
> is perfectly fine for data operations, it is not fine for metadata
> operations. Because metadata has been copied up and is represented by
> a different inode.
>
> I am concerned that this is a subtle behavior change and especially
> security modules might be surprised by it. I will spend now more time
> looking at what security modules are doing with f->f_inode. Thoughts?

This is exactly the same situation where there is an r/o open and
copy-up later.  Except this makes it more widespread.  So I don't
think any new issues are introduced (doesn't mean there aren't any old
ones).

The only risk I see with this feature is exactly the fact that the
ro/rw inconsistency can happen more often, which might break more
cases.  I'd really like to work on getting rid of this issue.
Hopefully next cycle...

Thanks,
Miklos
>

Re: [PATCH v10 00/18] overlayfs: Delayed copy up of data

[Amir Goldstein]

On Wed, Jan 24, 2018 at 11:05 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Mon, Jan 22, 2018 at 7:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> Hi,
>>
>> Please find attached V10 of the metadata only copy up patches. I think
>> I have taken care of review comments and I will start writing automated
>> test cases now.
>>
>> One thing which concerns me about these patches is the fact that during
>> file open d_real() will return lower inode which actually contains file
>> data and that inode will be installed in f->f_inode. So while f->f_inode
>> is perfectly fine for data operations, it is not fine for metadata
>> operations. Because metadata has been copied up and is represented by
>> a different inode.
>>
>> I am concerned that this is a subtle behavior change and especially
>> security modules might be surprised by it. I will spend now more time
>> looking at what security modules are doing with f->f_inode. Thoughts?
>
> This is exactly the same situation where there is an r/o open and
> copy-up later.  Except this makes it more widespread.  So I don't
> think any new issues are introduced (doesn't mean there aren't any old
> ones).
>
> The only risk I see with this feature is exactly the fact that the
> ro/rw inconsistency can happen more often, which might break more
> cases.  I'd really like to work on getting rid of this issue.
> Hopefully next cycle...
>

About that. I suppose you mean taking the route of intercepting
overlayfs file operations, so the only remaining concern would be
mmap?

FYI, I am still maintaining my "copy on read" patches [1] and use them for
snapshots. The last patch is a late addition that is aimed at integrating
with metacopy for "copy on read" for non-clone/non-same fs case.

Rummer has it [2] that Matthew Wilcox is going to tackle sharing page
cache between cloned inodes.

Mathew,

I wanted to bring the overlayfs "metacopy" use case to your attention,
so you can take it into account when implementing page cache sharing.
This was more or less the solution proposed by Al to inconsistent data
of a r/o open file:
- Create a "metacopy" inode in upper layer when opening a file for read
- Share page cache of lower layer inode with upper layer "metacopy" inode
- When upper file is opened for write, data is copied to upper inode and
  page cache sharing is broken.
- After breaking page cache sharing, the upper inode becomes the backing
  inode, but unshared pages content is guarantied to be uptodate and non
  dirty

So "metacopy" is an all-or-nothing private case of file cloning.
If undelying fs supports clones, then page cache sharing is not broken
on open for write, but it is converted from "metacopy" situation, to the
common cloned file situation - the backing inode is the upper inode,
bit its shares all the blocks with lower inode.

Please keep linux-unionfs posted when you have any progress.
I'd be happy to test any WIP you might have.

Thanks,
Amir.

[1] https://github.com/amir73il/linux/commits/ovl-rocopyup
[2] https://marc.info/?l=linux-xfs&m=151705246227384&w=2

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Vivek Goyal]

On Tue, Jan 23, 2018 at 03:47:29PM +0200, Amir Goldstein wrote:
> On Tue, Jan 23, 2018 at 3:45 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Mon, Jan 22, 2018 at 09:46:33PM +0200, Amir Goldstein wrote:
> >> On Mon, Jan 22, 2018 at 9:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> >> > On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >> >> There is a need to support metacopy dentry in midlayer. That means there
> >> >> could be a chain of metacopy dentries.
> >> >>
> >> >> For example, upper could be metacopy, midlayer lower could be metacopy and
> >> >> lowest layer could be actual data inode. This means when we copy up actual
> >> >> data, we should be able to reach to lowest data inode and copy up data from
> >> >> there. And that means we should keep track of all the dentries in origin
> >> >> chain which lead to data inode.
> >> >>
> >> >> Current ovl_check_origin() logic only looks for one origin dentry. This patch
> >> >> enhances ovl_check_origin() to continue to follow origin chain and return
> >> >> all the origin entries found. This is done only if caller of the function
> >> >> set "follow_chain" argument.
> >> >>
> >> >
> >> > We don't really need to keep the entire chain do we?
> >> > We can follow chain but keep only the one inode that is not a metacopy inode.
> >> > All the rest are useless, no?
> >> > Then we don't create a new type of object - non-dir with numlower > 1.
> >> >
> >>
> >> Seems like if you don't keep the entire chain, then no need for
> >> patches 14 and 15.
> >
> > I will need to have atleast 2 lower dentries. One will be top most
> > metadata copy and other lower most data dentry. IOW, both the ends of
> > the chain need to be there.
> >
> >> Also with upper metacopy, you can fix upper origin xattr after
> >> following to the data
> >> origin and forget about middle layer metacopies forever.
> >
> > If upper metacopy is alreday there, then I agree that lower top most
> > becomes inner node of chain and we can get rid of it.
> >
> >>
> >> Am I missing something?
> >
> > I think you are missing the case when there is no upper and lower has
> > a metacopy chain. In that case we need to retain two dentries. One for
> > data copy up and one for metadata copy up.
> >
> 
> OK. so you can get rid of all the middle metacopies while following
> the origin chain. no reason to keep those.

Hi Amir,

What about the case when we have upper and one middle metacopy. As of top
most lower is the ORIGIN for upper. Should we retain that top most lower
ORIGIN as well as lower most data. Or just retain lower most data and
use that as ORIGIN.

Vivek

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Amir Goldstein]

On Tue, Jan 30, 2018 at 9:33 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Tue, Jan 23, 2018 at 03:47:29PM +0200, Amir Goldstein wrote:
>> On Tue, Jan 23, 2018 at 3:45 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> > On Mon, Jan 22, 2018 at 09:46:33PM +0200, Amir Goldstein wrote:
>> >> On Mon, Jan 22, 2018 at 9:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> >> > On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> >> >> There is a need to support metacopy dentry in midlayer. That means there
>> >> >> could be a chain of metacopy dentries.
>> >> >>
>> >> >> For example, upper could be metacopy, midlayer lower could be metacopy and
>> >> >> lowest layer could be actual data inode. This means when we copy up actual
>> >> >> data, we should be able to reach to lowest data inode and copy up data from
>> >> >> there. And that means we should keep track of all the dentries in origin
>> >> >> chain which lead to data inode.
>> >> >>
>> >> >> Current ovl_check_origin() logic only looks for one origin dentry. This patch
>> >> >> enhances ovl_check_origin() to continue to follow origin chain and return
>> >> >> all the origin entries found. This is done only if caller of the function
>> >> >> set "follow_chain" argument.
>> >> >>
>> >> >
>> >> > We don't really need to keep the entire chain do we?
>> >> > We can follow chain but keep only the one inode that is not a metacopy inode.
>> >> > All the rest are useless, no?
>> >> > Then we don't create a new type of object - non-dir with numlower > 1.
>> >> >
>> >>
>> >> Seems like if you don't keep the entire chain, then no need for
>> >> patches 14 and 15.
>> >
>> > I will need to have atleast 2 lower dentries. One will be top most
>> > metadata copy and other lower most data dentry. IOW, both the ends of
>> > the chain need to be there.
>> >
>> >> Also with upper metacopy, you can fix upper origin xattr after
>> >> following to the data
>> >> origin and forget about middle layer metacopies forever.
>> >
>> > If upper metacopy is alreday there, then I agree that lower top most
>> > becomes inner node of chain and we can get rid of it.
>> >
>> >>
>> >> Am I missing something?
>> >
>> > I think you are missing the case when there is no upper and lower has
>> > a metacopy chain. In that case we need to retain two dentries. One for
>> > data copy up and one for metadata copy up.
>> >
>>
>> OK. so you can get rid of all the middle metacopies while following
>> the origin chain. no reason to keep those.
>
> Hi Amir,
>
> What about the case when we have upper and one middle metacopy. As of top
> most lower is the ORIGIN for upper. Should we retain that top most lower
> ORIGIN as well as lower most data. Or just retain lower most data and
> use that as ORIGIN.
>

Not sure I follow.
IIUC, the longest possible chain is lowerstack size 2:
upper -> metadata ORIGIN -> data ORIGIN

If this is what you meant than seems ok to me.

Thanks,
Amir.

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Amir Goldstein]

On Tue, Jan 30, 2018 at 10:03 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Tue, Jan 30, 2018 at 9:33 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> On Tue, Jan 23, 2018 at 03:47:29PM +0200, Amir Goldstein wrote:
>>> On Tue, Jan 23, 2018 at 3:45 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>>> > On Mon, Jan 22, 2018 at 09:46:33PM +0200, Amir Goldstein wrote:
>>> >> On Mon, Jan 22, 2018 at 9:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>> >> > On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>>> >> >> There is a need to support metacopy dentry in midlayer. That means there
>>> >> >> could be a chain of metacopy dentries.
>>> >> >>
>>> >> >> For example, upper could be metacopy, midlayer lower could be metacopy and
>>> >> >> lowest layer could be actual data inode. This means when we copy up actual
>>> >> >> data, we should be able to reach to lowest data inode and copy up data from
>>> >> >> there. And that means we should keep track of all the dentries in origin
>>> >> >> chain which lead to data inode.
>>> >> >>
>>> >> >> Current ovl_check_origin() logic only looks for one origin dentry. This patch
>>> >> >> enhances ovl_check_origin() to continue to follow origin chain and return
>>> >> >> all the origin entries found. This is done only if caller of the function
>>> >> >> set "follow_chain" argument.
>>> >> >>
>>> >> >
>>> >> > We don't really need to keep the entire chain do we?
>>> >> > We can follow chain but keep only the one inode that is not a metacopy inode.
>>> >> > All the rest are useless, no?
>>> >> > Then we don't create a new type of object - non-dir with numlower > 1.
>>> >> >
>>> >>
>>> >> Seems like if you don't keep the entire chain, then no need for
>>> >> patches 14 and 15.
>>> >
>>> > I will need to have atleast 2 lower dentries. One will be top most
>>> > metadata copy and other lower most data dentry. IOW, both the ends of
>>> > the chain need to be there.
>>> >
>>> >> Also with upper metacopy, you can fix upper origin xattr after
>>> >> following to the data
>>> >> origin and forget about middle layer metacopies forever.
>>> >
>>> > If upper metacopy is alreday there, then I agree that lower top most
>>> > becomes inner node of chain and we can get rid of it.
>>> >
>>> >>
>>> >> Am I missing something?
>>> >
>>> > I think you are missing the case when there is no upper and lower has
>>> > a metacopy chain. In that case we need to retain two dentries. One for
>>> > data copy up and one for metadata copy up.
>>> >
>>>
>>> OK. so you can get rid of all the middle metacopies while following
>>> the origin chain. no reason to keep those.
>>
>> Hi Amir,
>>
>> What about the case when we have upper and one middle metacopy. As of top
>> most lower is the ORIGIN for upper. Should we retain that top most lower
>> ORIGIN as well as lower most data. Or just retain lower most data and
>> use that as ORIGIN.
>>
>
> Not sure I follow.
> IIUC, the longest possible chain is lowerstack size 2:
> upper -> metadata ORIGIN -> data ORIGIN
>

If you are wondering if we could drop the metadata ORIGIN,
I think we need it at least for constant and persistent st_ino.


> If this is what you meant than seems ok to me.
>
> Thanks,
> Amir.

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Vivek Goyal]

On Tue, Jan 30, 2018 at 10:03:19PM +0200, Amir Goldstein wrote:
> On Tue, Jan 30, 2018 at 9:33 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Tue, Jan 23, 2018 at 03:47:29PM +0200, Amir Goldstein wrote:
> >> On Tue, Jan 23, 2018 at 3:45 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >> > On Mon, Jan 22, 2018 at 09:46:33PM +0200, Amir Goldstein wrote:
> >> >> On Mon, Jan 22, 2018 at 9:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> >> >> > On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >> >> >> There is a need to support metacopy dentry in midlayer. That means there
> >> >> >> could be a chain of metacopy dentries.
> >> >> >>
> >> >> >> For example, upper could be metacopy, midlayer lower could be metacopy and
> >> >> >> lowest layer could be actual data inode. This means when we copy up actual
> >> >> >> data, we should be able to reach to lowest data inode and copy up data from
> >> >> >> there. And that means we should keep track of all the dentries in origin
> >> >> >> chain which lead to data inode.
> >> >> >>
> >> >> >> Current ovl_check_origin() logic only looks for one origin dentry. This patch
> >> >> >> enhances ovl_check_origin() to continue to follow origin chain and return
> >> >> >> all the origin entries found. This is done only if caller of the function
> >> >> >> set "follow_chain" argument.
> >> >> >>
> >> >> >
> >> >> > We don't really need to keep the entire chain do we?
> >> >> > We can follow chain but keep only the one inode that is not a metacopy inode.
> >> >> > All the rest are useless, no?
> >> >> > Then we don't create a new type of object - non-dir with numlower > 1.
> >> >> >
> >> >>
> >> >> Seems like if you don't keep the entire chain, then no need for
> >> >> patches 14 and 15.
> >> >
> >> > I will need to have atleast 2 lower dentries. One will be top most
> >> > metadata copy and other lower most data dentry. IOW, both the ends of
> >> > the chain need to be there.
> >> >
> >> >> Also with upper metacopy, you can fix upper origin xattr after
> >> >> following to the data
> >> >> origin and forget about middle layer metacopies forever.
> >> >
> >> > If upper metacopy is alreday there, then I agree that lower top most
> >> > becomes inner node of chain and we can get rid of it.
> >> >
> >> >>
> >> >> Am I missing something?
> >> >
> >> > I think you are missing the case when there is no upper and lower has
> >> > a metacopy chain. In that case we need to retain two dentries. One for
> >> > data copy up and one for metadata copy up.
> >> >
> >>
> >> OK. so you can get rid of all the middle metacopies while following
> >> the origin chain. no reason to keep those.
> >
> > Hi Amir,
> >
> > What about the case when we have upper and one middle metacopy. As of top
> > most lower is the ORIGIN for upper. Should we retain that top most lower
> > ORIGIN as well as lower most data. Or just retain lower most data and
> > use that as ORIGIN.
> >
> 
> Not sure I follow.
> IIUC, the longest possible chain is lowerstack size 2:
> upper -> metadata ORIGIN -> data ORIGIN
> 
> If this is what you meant than seems ok to me.

Right, that's what I mean. Ok, I will keep it that way.

Vivek

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Vivek Goyal]

On Tue, Jan 30, 2018 at 10:05:08PM +0200, Amir Goldstein wrote:
> On Tue, Jan 30, 2018 at 10:03 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Tue, Jan 30, 2018 at 9:33 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >> On Tue, Jan 23, 2018 at 03:47:29PM +0200, Amir Goldstein wrote:
> >>> On Tue, Jan 23, 2018 at 3:45 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >>> > On Mon, Jan 22, 2018 at 09:46:33PM +0200, Amir Goldstein wrote:
> >>> >> On Mon, Jan 22, 2018 at 9:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> >>> >> > On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> >>> >> >> There is a need to support metacopy dentry in midlayer. That means there
> >>> >> >> could be a chain of metacopy dentries.
> >>> >> >>
> >>> >> >> For example, upper could be metacopy, midlayer lower could be metacopy and
> >>> >> >> lowest layer could be actual data inode. This means when we copy up actual
> >>> >> >> data, we should be able to reach to lowest data inode and copy up data from
> >>> >> >> there. And that means we should keep track of all the dentries in origin
> >>> >> >> chain which lead to data inode.
> >>> >> >>
> >>> >> >> Current ovl_check_origin() logic only looks for one origin dentry. This patch
> >>> >> >> enhances ovl_check_origin() to continue to follow origin chain and return
> >>> >> >> all the origin entries found. This is done only if caller of the function
> >>> >> >> set "follow_chain" argument.
> >>> >> >>
> >>> >> >
> >>> >> > We don't really need to keep the entire chain do we?
> >>> >> > We can follow chain but keep only the one inode that is not a metacopy inode.
> >>> >> > All the rest are useless, no?
> >>> >> > Then we don't create a new type of object - non-dir with numlower > 1.
> >>> >> >
> >>> >>
> >>> >> Seems like if you don't keep the entire chain, then no need for
> >>> >> patches 14 and 15.
> >>> >
> >>> > I will need to have atleast 2 lower dentries. One will be top most
> >>> > metadata copy and other lower most data dentry. IOW, both the ends of
> >>> > the chain need to be there.
> >>> >
> >>> >> Also with upper metacopy, you can fix upper origin xattr after
> >>> >> following to the data
> >>> >> origin and forget about middle layer metacopies forever.
> >>> >
> >>> > If upper metacopy is alreday there, then I agree that lower top most
> >>> > becomes inner node of chain and we can get rid of it.
> >>> >
> >>> >>
> >>> >> Am I missing something?
> >>> >
> >>> > I think you are missing the case when there is no upper and lower has
> >>> > a metacopy chain. In that case we need to retain two dentries. One for
> >>> > data copy up and one for metadata copy up.
> >>> >
> >>>
> >>> OK. so you can get rid of all the middle metacopies while following
> >>> the origin chain. no reason to keep those.
> >>
> >> Hi Amir,
> >>
> >> What about the case when we have upper and one middle metacopy. As of top
> >> most lower is the ORIGIN for upper. Should we retain that top most lower
> >> ORIGIN as well as lower most data. Or just retain lower most data and
> >> use that as ORIGIN.
> >>
> >
> > Not sure I follow.
> > IIUC, the longest possible chain is lowerstack size 2:
> > upper -> metadata ORIGIN -> data ORIGIN
> >
> 
> If you are wondering if we could drop the metadata ORIGIN,
> I think we need it at least for constant and persistent st_ino.

Right, I was wondering can be drop metadata ORIGIN and just live with
data ORIGIN.

My initial impression is that constant and persistent st_ino can be
achieved using just data ORIGIN alone as well. But I might be missing
some finer detail.

Vivek

Re: [PATCH v10 07/18] ovl: Add mechanism to create a chain of origin dentries

[Amir Goldstein]

On Tue, Jan 30, 2018 at 10:15 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Tue, Jan 30, 2018 at 10:05:08PM +0200, Amir Goldstein wrote:
>> On Tue, Jan 30, 2018 at 10:03 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> > On Tue, Jan 30, 2018 at 9:33 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> >> On Tue, Jan 23, 2018 at 03:47:29PM +0200, Amir Goldstein wrote:
>> >>> On Tue, Jan 23, 2018 at 3:45 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> >>> > On Mon, Jan 22, 2018 at 09:46:33PM +0200, Amir Goldstein wrote:
>> >>> >> On Mon, Jan 22, 2018 at 9:34 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> >>> >> > On Mon, Jan 22, 2018 at 8:39 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> >>> >> >> There is a need to support metacopy dentry in midlayer. That means there
>> >>> >> >> could be a chain of metacopy dentries.
>> >>> >> >>
>> >>> >> >> For example, upper could be metacopy, midlayer lower could be metacopy and
>> >>> >> >> lowest layer could be actual data inode. This means when we copy up actual
>> >>> >> >> data, we should be able to reach to lowest data inode and copy up data from
>> >>> >> >> there. And that means we should keep track of all the dentries in origin
>> >>> >> >> chain which lead to data inode.
>> >>> >> >>
>> >>> >> >> Current ovl_check_origin() logic only looks for one origin dentry. This patch
>> >>> >> >> enhances ovl_check_origin() to continue to follow origin chain and return
>> >>> >> >> all the origin entries found. This is done only if caller of the function
>> >>> >> >> set "follow_chain" argument.
>> >>> >> >>
>> >>> >> >
>> >>> >> > We don't really need to keep the entire chain do we?
>> >>> >> > We can follow chain but keep only the one inode that is not a metacopy inode.
>> >>> >> > All the rest are useless, no?
>> >>> >> > Then we don't create a new type of object - non-dir with numlower > 1.
>> >>> >> >
>> >>> >>
>> >>> >> Seems like if you don't keep the entire chain, then no need for
>> >>> >> patches 14 and 15.
>> >>> >
>> >>> > I will need to have atleast 2 lower dentries. One will be top most
>> >>> > metadata copy and other lower most data dentry. IOW, both the ends of
>> >>> > the chain need to be there.
>> >>> >
>> >>> >> Also with upper metacopy, you can fix upper origin xattr after
>> >>> >> following to the data
>> >>> >> origin and forget about middle layer metacopies forever.
>> >>> >
>> >>> > If upper metacopy is alreday there, then I agree that lower top most
>> >>> > becomes inner node of chain and we can get rid of it.
>> >>> >
>> >>> >>
>> >>> >> Am I missing something?
>> >>> >
>> >>> > I think you are missing the case when there is no upper and lower has
>> >>> > a metacopy chain. In that case we need to retain two dentries. One for
>> >>> > data copy up and one for metadata copy up.
>> >>> >
>> >>>
>> >>> OK. so you can get rid of all the middle metacopies while following
>> >>> the origin chain. no reason to keep those.
>> >>
>> >> Hi Amir,
>> >>
>> >> What about the case when we have upper and one middle metacopy. As of top
>> >> most lower is the ORIGIN for upper. Should we retain that top most lower
>> >> ORIGIN as well as lower most data. Or just retain lower most data and
>> >> use that as ORIGIN.
>> >>
>> >
>> > Not sure I follow.
>> > IIUC, the longest possible chain is lowerstack size 2:
>> > upper -> metadata ORIGIN -> data ORIGIN
>> >
>>
>> If you are wondering if we could drop the metadata ORIGIN,
>> I think we need it at least for constant and persistent st_ino.
>
> Right, I was wondering can be drop metadata ORIGIN and just live with
> data ORIGIN.
>
> My initial impression is that constant and persistent st_ino can be
> achieved using just data ORIGIN alone as well. But I might be missing
> some finer detail.
>

That makes sense some to me (st_ino from data ORIGIN), but it may
complicate the code in several cases that assume that st_ino and file handle
and index key all come from lowerstack[0].

So I guess I lean towards always keeping metadata ORIGIN around and
using its inode for st_ino and index file handle.

Amir.