* [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data
@ 2017-11-28 15:59 Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 01/15] ovl: Do not look for OVL_XATTR_NLINK if index is not there Vivek Goyal
` (15 more replies)
0 siblings, 16 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
Hi,
Here is the V8 of the patches. Primarily I have taken care of comments
from Amir.
This applies on top of the fix I posted with subject "ovl: Pass
ovl_get_nlink() parameters in right order"
Amir, I have added one more patch int series where I pass index dentry
to ovl_get_nlink() to get rid of a warning during testing.
I think only outstanding concern with this patch series now is how to
detect that metacopy feature was ever enabled and there are some
metacopy only files present. In that case we probably don't want to
allow user to mount with metacopy=off. We currently don't seem to have
a generic infrastructure which keeps track of features and backward
compatibility stuff etc. Discussion still seems to be on upstream.
Is this a blocker for this patch series or this is something we can
live with? Only downside of mounting a file system which has metacopied
files (with metacopy=off), is that user will see truncated files.
Vivek
Amir Goldstein (1):
ovl: disable redirect_dir and index when no xattr support
Vivek Goyal (14):
ovl: Do not look for OVL_XATTR_NLINK if index is not there
ovl: ovl_check_setxattr() get rid of redundant -EOPNOTSUPP check
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: 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: Fix ovl_getattr() to get number of blocks from lower
ovl: Set OVL_UPPERDATA flag during ovl_lookup()
ovl: Do not expose metacopy only upper dentry from d_real()
ovl: Fix encryption/compression status of a metacopy only file
ovl: Enable metadata only feature
fs/overlayfs/Kconfig | 8 +++
fs/overlayfs/copy_up.c | 162 ++++++++++++++++++++++++++++++++++-------------
fs/overlayfs/dir.c | 1 +
fs/overlayfs/inode.c | 65 +++++++++----------
fs/overlayfs/namei.c | 42 +++++++++++-
fs/overlayfs/overlayfs.h | 20 +++++-
fs/overlayfs/ovl_entry.h | 1 +
fs/overlayfs/super.c | 62 ++++++++++++++++--
fs/overlayfs/util.c | 100 +++++++++++++++++++++++++----
9 files changed, 363 insertions(+), 98 deletions(-)
--
2.13.6
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v8 01/15] ovl: Do not look for OVL_XATTR_NLINK if index is not there
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 19:09 ` Amir Goldstein
2017-11-28 15:59 ` [PATCH v8 02/15] ovl: disable redirect_dir and index when no xattr support Vivek Goyal
` (14 subsequent siblings)
15 siblings, 1 reply; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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 another parameter to function and pass index dentry. If there is
no index dentry, that should mean that we never set OVL_XATTR_NLINK
and just return fallback.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
fs/overlayfs/inode.c | 6 +++++-
fs/overlayfs/namei.c | 2 +-
fs/overlayfs/overlayfs.h | 1 +
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 00b6b294272a..8dcfe875ace8 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -544,6 +544,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,6 +552,9 @@ unsigned int ovl_get_nlink(struct dentry *lowerdentry,
char buf[13];
int err;
+ if (!index)
+ return fallback;
+
if (!lowerdentry || !upperdentry || d_inode(lowerdentry)->i_nlink == 1)
return fallback;
@@ -670,7 +674,7 @@ struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry,
goto out;
}
- nlink = ovl_get_nlink(lowerdentry, upperdentry,
+ nlink = ovl_get_nlink(lowerdentry, upperdentry, index,
realinode->i_nlink);
set_nlink(inode, nlink);
} else {
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 5ef69bc09e0c..a11d77361bef 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -435,7 +435,7 @@ int ovl_verify_index(struct dentry *index, struct ovl_path *lower,
/* Check if index is orphan and don't warn before cleaning it */
if (d_inode(index)->i_nlink == 1 &&
- ovl_get_nlink(origin.dentry, index, 0) == 0)
+ ovl_get_nlink(origin.dentry, index, index, 0) == 0)
err = -ENOENT;
dput(origin.dentry);
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 13eab09a6b6f..383bbd3b1fd0 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -275,6 +275,7 @@ 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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 02/15] ovl: disable redirect_dir and index when no xattr support
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 01/15] ovl: Do not look for OVL_XATTR_NLINK if index is not there Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 03/15] ovl: ovl_check_setxattr() get rid of redundant -EOPNOTSUPP check Vivek Goyal
` (13 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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 be03578181d2..ee41bde87b14 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -921,7 +921,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 03/15] ovl: ovl_check_setxattr() get rid of redundant -EOPNOTSUPP check
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 01/15] ovl: Do not look for OVL_XATTR_NLINK if index is not there Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 02/15] ovl: disable redirect_dir and index when no xattr support Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 04/15] ovl: Create origin xattr on copy up for all files Vivek Goyal
` (12 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
At mount time we check if upper supports xattr or not and set ofs->noxattr
field accordingly. But in ovl_check_setxattr() still seems to have logic
which expects that ovl_do_setxattr() can return -EOPNOTSUPP. Not sure when
and how can we hit this code. Feels redundant to me. So I am removing
this code.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
fs/overlayfs/util.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index d6bb1c9f5e7a..77dc00438d54 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -365,21 +365,12 @@ int ovl_check_setxattr(struct dentry *dentry, struct dentry *upperdentry,
const char *name, const void *value, size_t size,
int xerr)
{
- int err;
struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
if (ofs->noxattr)
return xerr;
- err = ovl_do_setxattr(upperdentry, name, value, size, 0);
-
- if (err == -EOPNOTSUPP) {
- pr_warn("overlayfs: cannot set %s xattr on upper\n", name);
- ofs->noxattr = true;
- return xerr;
- }
-
- return err;
+ return ovl_do_setxattr(upperdentry, name, value, size, 0);
}
int ovl_set_impure(struct dentry *dentry, struct dentry *upperdentry)
--
2.13.6
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 04/15] ovl: Create origin xattr on copy up for all files
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (2 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 03/15] ovl: ovl_check_setxattr() get rid of redundant -EOPNOTSUPP check Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 05/15] ovl: Provide a mount option metacopy=on/off for metadata copyup Vivek Goyal
` (11 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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".
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 05/15] ovl: Provide a mount option metacopy=on/off for metadata copyup
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (3 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 04/15] ovl: Create origin xattr on copy up for all files Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 19:15 ` Amir Goldstein
2017-11-28 15:59 ` [PATCH v8 06/15] ovl: During copy up, first copy up metadata and then data Vivek Goyal
` (10 subsequent siblings)
15 siblings, 1 reply; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
fs/overlayfs/Kconfig | 8 ++++++++
fs/overlayfs/ovl_entry.h | 1 +
fs/overlayfs/super.c | 40 +++++++++++++++++++++++++++++++++++++---
3 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
index cbfc196e5dc5..17a0b17ad14c 100644
--- a/fs/overlayfs/Kconfig
+++ b/fs/overlayfs/Kconfig
@@ -43,3 +43,11 @@ 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.
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 93eb6a044dd2..2720ed21ad53 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -15,6 +15,7 @@ struct ovl_config {
bool default_permissions;
bool redirect_dir;
bool index;
+ bool metacopy;
};
struct ovl_layer {
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index ee41bde87b14..ed4f42d9446a 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -46,6 +46,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;
@@ -319,6 +324,9 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
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;
}
@@ -352,6 +360,8 @@ enum {
OPT_REDIRECT_DIR_OFF,
OPT_INDEX_ON,
OPT_INDEX_OFF,
+ OPT_METACOPY_ON,
+ OPT_METACOPY_OFF,
OPT_ERR,
};
@@ -364,6 +374,8 @@ static const match_table_t ovl_tokens = {
{OPT_REDIRECT_DIR_OFF, "redirect_dir=off"},
{OPT_INDEX_ON, "index=on"},
{OPT_INDEX_OFF, "index=off"},
+ {OPT_METACOPY_ON, "metacopy=on"},
+ {OPT_METACOPY_OFF, "metacopy=off"},
{OPT_ERR, NULL}
};
@@ -444,6 +456,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;
@@ -657,9 +677,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;
@@ -923,7 +945,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);
}
@@ -1164,6 +1187,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
ofs->config.redirect_dir = ovl_redirect_dir_def;
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;
@@ -1209,6 +1233,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 06/15] ovl: During copy up, first copy up metadata and then data
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (4 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 05/15] ovl: Provide a mount option metacopy=on/off for metadata copyup Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 07/15] ovl: Move the copy up helpers to copy_up.c Vivek Goyal
` (9 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 07/15] ovl: Move the copy up helpers to copy_up.c
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (5 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 06/15] ovl: During copy up, first copy up metadata and then data Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 19:16 ` Amir Goldstein
2017-11-28 15:59 ` [PATCH v8 08/15] ovl: Copy up only metadata during copy up where it makes sense Vivek Goyal
` (8 subsequent siblings)
15 siblings, 1 reply; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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.
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 8dcfe875ace8..472653f31ce7 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 383bbd3b1fd0..1ca946ec57c9 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -287,7 +287,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);
@@ -320,6 +319,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 08/15] ovl: Copy up only metadata during copy up where it makes sense
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (6 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 07/15] ovl: Move the copy up helpers to copy_up.c Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 09/15] ovl: Add helper ovl_already_copied_up() Vivek Goyal
` (7 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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.
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 09/15] ovl: Add helper ovl_already_copied_up()
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (7 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 08/15] ovl: Copy up only metadata during copy up where it makes sense Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 10/15] ovl: A new xattr OVL_XATTR_METACOPY for file on upper Vivek Goyal
` (6 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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 1ca946ec57c9..6eb1769541e9 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 77dc00438d54..572bf46e9f2e 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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 10/15] ovl: A new xattr OVL_XATTR_METACOPY for file on upper
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (8 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 09/15] ovl: Add helper ovl_already_copied_up() Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 19:53 ` Amir Goldstein
2017-11-28 15:59 ` [PATCH v8 11/15] ovl: Fix ovl_getattr() to get number of blocks from lower Vivek Goyal
` (5 subsequent siblings)
15 siblings, 1 reply; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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.
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 | 71 +++++++++++++++++++++++++++++++++++++++++++++---
5 files changed, 136 insertions(+), 11 deletions(-)
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 65e993e931ba..f7225782b046 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 e13921824c70..6ebd351d8a4c 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 6eb1769541e9..2c1418cde6c6 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 ed4f42d9446a..c20da899fcc7 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1281,6 +1281,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 572bf46e9f2e..8c1b8399703e 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -231,6 +231,57 @@ 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 +365,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 +392,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 11/15] ovl: Fix ovl_getattr() to get number of blocks from lower
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (9 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 10/15] ovl: A new xattr OVL_XATTR_METACOPY for file on upper Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 12/15] ovl: Set OVL_UPPERDATA flag during ovl_lookup() Vivek Goyal
` (4 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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 | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 472653f31ce7..da5cc2c6c413 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -76,6 +76,10 @@ 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;
+
+ if (ovl_dentry_upper(dentry) && !ovl_has_upperdata(dentry))
+ metacopy = true;
type = ovl_path_real(dentry, &realpath);
old_cred = ovl_override_creds(dentry->d_sb);
@@ -93,7 +97,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,
@@ -117,6 +122,9 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
WARN_ON_ONCE(stat->dev != lowerstat.dev);
else
stat->dev = ovl_get_pseudo_dev(dentry);
+
+ if (metacopy)
+ stat->blocks = lowerstat.blocks;
}
if (samefs) {
/*
--
2.13.6
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 12/15] ovl: Set OVL_UPPERDATA flag during ovl_lookup()
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (10 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 11/15] ovl: Fix ovl_getattr() to get number of blocks from lower Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 13/15] ovl: Do not expose metacopy only upper dentry from d_real() Vivek Goyal
` (3 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
fs/overlayfs/namei.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index a11d77361bef..11340262df6e 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -25,6 +25,28 @@ struct ovl_lookup_data {
char *redirect;
};
+/* err < 0, 0 if no metacopy xattr, 1 if metacopy xattr found */
+static int ovl_check_metacopy(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;
+}
+
static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
size_t prelen, const char *post)
{
@@ -602,6 +624,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
struct dentry *this;
unsigned int i;
int err;
+ bool metacopy = false;
struct ovl_lookup_data d = {
.name = dentry->d_name,
.is_dir = false,
@@ -642,6 +665,20 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
roe->numlower, &stack, &ctr);
if (err)
goto out_put_upper;
+
+ err = ovl_check_metacopy(upperdentry);
+ metacopy = err;
+ if (err < 0)
+ goto out_put_upper;
+ if (metacopy && !ctr) {
+ /*
+ * Found an upper with metacopy set but
+ * at the same time there is no lower
+ * dentry. Something is not right.
+ */
+ err = -ESTALE;
+ goto out_put_upper;
+ }
}
if (d.redirect) {
@@ -726,6 +763,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 13/15] ovl: Do not expose metacopy only upper dentry from d_real()
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (11 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 12/15] ovl: Set OVL_UPPERDATA flag during ovl_lookup() Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 14/15] ovl: Fix encryption/compression status of a metacopy only file Vivek Goyal
` (2 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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().
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.
- For metadata inode, mtime should be same as lower and not change. (data
can't be modified on metadata inode without copyup).
- 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.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
fs/overlayfs/super.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index c20da899fcc7..fcb66cd9b413 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -84,8 +84,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))
@@ -101,14 +107,21 @@ 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;
}
+lower:
real = ovl_dentry_lower(dentry);
if (!real)
goto bug;
--
2.13.6
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 14/15] ovl: Fix encryption/compression status of a metacopy only file
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (12 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 13/15] ovl: Do not expose metacopy only upper dentry from d_real() Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 15/15] ovl: Enable metadata only feature Vivek Goyal
2017-11-28 17:04 ` [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Amir Goldstein
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
If file is metacopy only, it is possible that lower is encrypted while
other is not. In that case, report file as encrypted (despite the fact
that only data is encrypted while metadata is not).
Similarly if lower is compressed, report that in stat for a metacopy
only file.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
fs/overlayfs/inode.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index da5cc2c6c413..5fbfb670963d 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -66,6 +66,23 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr)
return err;
}
+#define OVL_STATX_ATTR_MASK (STATX_ATTR_ENCRYPTED | STATX_ATTR_COMPRESSED)
+
+static void ovl_stat_fix_attributes(struct kstat *ustat, struct kstat *lstat) {
+ unsigned int attr_mask, attr;
+
+ attr_mask = lstat->attributes_mask & OVL_STATX_ATTR_MASK;
+ if (!attr_mask)
+ return;
+
+ attr = lstat->attributes & attr_mask;
+ if (!attr)
+ return;
+
+ ustat->attributes_mask |= attr_mask;
+ ustat->attributes |= attr;
+}
+
int ovl_getattr(const struct path *path, struct kstat *stat,
u32 request_mask, unsigned int flags)
{
@@ -123,8 +140,10 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
else
stat->dev = ovl_get_pseudo_dev(dentry);
- if (metacopy)
+ if (metacopy) {
stat->blocks = lowerstat.blocks;
+ ovl_stat_fix_attributes(stat, &lowerstat);
+ }
}
if (samefs) {
/*
--
2.13.6
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 15/15] ovl: Enable metadata only feature
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (13 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 14/15] ovl: Fix encryption/compression status of a metacopy only file Vivek Goyal
@ 2017-11-28 15:59 ` Vivek Goyal
2017-11-28 17:04 ` [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Amir Goldstein
15 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 15:59 UTC (permalink / raw
To: linux-unionfs; +Cc: miklos, amir73il, vgoyal
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 f7225782b046..241520604362 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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
` (14 preceding siblings ...)
2017-11-28 15:59 ` [PATCH v8 15/15] ovl: Enable metadata only feature Vivek Goyal
@ 2017-11-28 17:04 ` Amir Goldstein
2017-11-28 18:22 ` Vivek Goyal
15 siblings, 1 reply; 22+ messages in thread
From: Amir Goldstein @ 2017-11-28 17:04 UTC (permalink / raw
To: Vivek Goyal; +Cc: overlayfs, Miklos Szeredi
On Tue, Nov 28, 2017 at 5:59 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> Hi,
>
> Here is the V8 of the patches. Primarily I have taken care of comments
> from Amir.
>
> This applies on top of the fix I posted with subject "ovl: Pass
> ovl_get_nlink() parameters in right order"
>
> Amir, I have added one more patch int series where I pass index dentry
> to ovl_get_nlink() to get rid of a warning during testing.
>
> I think only outstanding concern with this patch series now is how to
> detect that metacopy feature was ever enabled and there are some
> metacopy only files present. In that case we probably don't want to
> allow user to mount with metacopy=off. We currently don't seem to have
> a generic infrastructure which keeps track of features and backward
> compatibility stuff etc. Discussion still seems to be on upstream.
>
> Is this a blocker for this patch series or this is something we can
> live with? Only downside of mounting a file system which has metacopied
> files (with metacopy=off), is that user will see truncated files.
IMO it is fine to mount with metacopy=off, but user should not see
truncated file. IMO if metacopy=off, overlayfs still needs to copy up
data from origin if origin can be decoded and if origin cannot be decoded
user should get -EIO.
Mounting an overlayfs with metacopies with kernel that does not support
metacopy will have unexpected results, same as with redirect_dir.
Amir.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data
2017-11-28 17:04 ` [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Amir Goldstein
@ 2017-11-28 18:22 ` Vivek Goyal
0 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2017-11-28 18:22 UTC (permalink / raw
To: Amir Goldstein; +Cc: overlayfs, Miklos Szeredi
On Tue, Nov 28, 2017 at 07:04:33PM +0200, Amir Goldstein wrote:
> On Tue, Nov 28, 2017 at 5:59 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > Hi,
> >
> > Here is the V8 of the patches. Primarily I have taken care of comments
> > from Amir.
> >
> > This applies on top of the fix I posted with subject "ovl: Pass
> > ovl_get_nlink() parameters in right order"
> >
> > Amir, I have added one more patch int series where I pass index dentry
> > to ovl_get_nlink() to get rid of a warning during testing.
Amir, can you please have a look at patch 1 in the series. That's new patch
as compared to V7.
> >
> > I think only outstanding concern with this patch series now is how to
> > detect that metacopy feature was ever enabled and there are some
> > metacopy only files present. In that case we probably don't want to
> > allow user to mount with metacopy=off. We currently don't seem to have
> > a generic infrastructure which keeps track of features and backward
> > compatibility stuff etc. Discussion still seems to be on upstream.
> >
> > Is this a blocker for this patch series or this is something we can
> > live with? Only downside of mounting a file system which has metacopied
> > files (with metacopy=off), is that user will see truncated files.
>
> IMO it is fine to mount with metacopy=off, but user should not see
> truncated file. IMO if metacopy=off, overlayfs still needs to copy up
> data from origin if origin can be decoded and if origin cannot be decoded
> user should get -EIO.
Right. That's the current behavior with the patch. I forgot to mention
that user will see truncated files only with old kernels.
> Mounting an overlayfs with metacopies with kernel that does not support
> metacopy will have unexpected results, same as with redirect_dir.
Right. As of now old kernel should see truncated files.
Vivek
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 01/15] ovl: Do not look for OVL_XATTR_NLINK if index is not there
2017-11-28 15:59 ` [PATCH v8 01/15] ovl: Do not look for OVL_XATTR_NLINK if index is not there Vivek Goyal
@ 2017-11-28 19:09 ` Amir Goldstein
0 siblings, 0 replies; 22+ messages in thread
From: Amir Goldstein @ 2017-11-28 19:09 UTC (permalink / raw
To: Vivek Goyal; +Cc: overlayfs, Miklos Szeredi
On Tue, Nov 28, 2017 at 5:59 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> 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 another parameter to function and pass index dentry. If there is
> no index dentry, that should mean that we never set OVL_XATTR_NLINK
> and just return fallback.
>
The fix is conceptually correct, but I would do it a bit differently.
Instead of adding the index argument, pass index instead of upperdentry.
They are the same inode anyway and the dentry is only used to get to
the inode in that helper. Expect for the warning which refers to index
anyway. In some cases of lookup (indexed lower) upperdentry here will
really be the index dentry (and not the upper alias above lower).
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
> fs/overlayfs/inode.c | 6 +++++-
> fs/overlayfs/namei.c | 2 +-
> fs/overlayfs/overlayfs.h | 1 +
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 00b6b294272a..8dcfe875ace8 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -544,6 +544,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,6 +552,9 @@ unsigned int ovl_get_nlink(struct dentry *lowerdentry,
> char buf[13];
> int err;
>
> + if (!index)
> + return fallback;
> +
> if (!lowerdentry || !upperdentry || d_inode(lowerdentry)->i_nlink == 1)
> return fallback;
>
> @@ -670,7 +674,7 @@ struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry,
> goto out;
> }
>
> - nlink = ovl_get_nlink(lowerdentry, upperdentry,
> + nlink = ovl_get_nlink(lowerdentry, upperdentry, index,
> realinode->i_nlink);
> set_nlink(inode, nlink);
> } else {
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index 5ef69bc09e0c..a11d77361bef 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -435,7 +435,7 @@ int ovl_verify_index(struct dentry *index, struct ovl_path *lower,
>
> /* Check if index is orphan and don't warn before cleaning it */
> if (d_inode(index)->i_nlink == 1 &&
> - ovl_get_nlink(origin.dentry, index, 0) == 0)
> + ovl_get_nlink(origin.dentry, index, index, 0) == 0)
> err = -ENOENT;
Here you see how silly that added arg looks...
Amir.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 05/15] ovl: Provide a mount option metacopy=on/off for metadata copyup
2017-11-28 15:59 ` [PATCH v8 05/15] ovl: Provide a mount option metacopy=on/off for metadata copyup Vivek Goyal
@ 2017-11-28 19:15 ` Amir Goldstein
0 siblings, 0 replies; 22+ messages in thread
From: Amir Goldstein @ 2017-11-28 19:15 UTC (permalink / raw
To: Vivek Goyal; +Cc: overlayfs, Miklos Szeredi
On Tue, Nov 28, 2017 at 5:59 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> 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.
>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
> fs/overlayfs/Kconfig | 8 ++++++++
> fs/overlayfs/ovl_entry.h | 1 +
> fs/overlayfs/super.c | 40 +++++++++++++++++++++++++++++++++++++---
> 3 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
> index cbfc196e5dc5..17a0b17ad14c 100644
> --- a/fs/overlayfs/Kconfig
> +++ b/fs/overlayfs/Kconfig
> @@ -43,3 +43,11 @@ 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.
Just add some scary stuff about mounting with old kernel.
And the text about module/mount option.
Use the gist version like in REDIRECT_DIR, not like I did...
Amir.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 07/15] ovl: Move the copy up helpers to copy_up.c
2017-11-28 15:59 ` [PATCH v8 07/15] ovl: Move the copy up helpers to copy_up.c Vivek Goyal
@ 2017-11-28 19:16 ` Amir Goldstein
0 siblings, 0 replies; 22+ messages in thread
From: Amir Goldstein @ 2017-11-28 19:16 UTC (permalink / raw
To: Vivek Goyal; +Cc: overlayfs, Miklos Szeredi
On Tue, Nov 28, 2017 at 5:59 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> 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.
>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 10/15] ovl: A new xattr OVL_XATTR_METACOPY for file on upper
2017-11-28 15:59 ` [PATCH v8 10/15] ovl: A new xattr OVL_XATTR_METACOPY for file on upper Vivek Goyal
@ 2017-11-28 19:53 ` Amir Goldstein
0 siblings, 0 replies; 22+ messages in thread
From: Amir Goldstein @ 2017-11-28 19:53 UTC (permalink / raw
To: Vivek Goyal; +Cc: overlayfs, Miklos Szeredi
On Tue, Nov 28, 2017 at 5:59 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> 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.
>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Except for minor nits below.
...
> +static bool ovl_should_check_upperdata(struct dentry *dentry) {
You should run your patch through checkpatch.
Kernel coding style doesn't like opening brackets for function without newline.
Amir.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-11-28 19:53 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-28 15:59 [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 01/15] ovl: Do not look for OVL_XATTR_NLINK if index is not there Vivek Goyal
2017-11-28 19:09 ` Amir Goldstein
2017-11-28 15:59 ` [PATCH v8 02/15] ovl: disable redirect_dir and index when no xattr support Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 03/15] ovl: ovl_check_setxattr() get rid of redundant -EOPNOTSUPP check Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 04/15] ovl: Create origin xattr on copy up for all files Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 05/15] ovl: Provide a mount option metacopy=on/off for metadata copyup Vivek Goyal
2017-11-28 19:15 ` Amir Goldstein
2017-11-28 15:59 ` [PATCH v8 06/15] ovl: During copy up, first copy up metadata and then data Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 07/15] ovl: Move the copy up helpers to copy_up.c Vivek Goyal
2017-11-28 19:16 ` Amir Goldstein
2017-11-28 15:59 ` [PATCH v8 08/15] ovl: Copy up only metadata during copy up where it makes sense Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 09/15] ovl: Add helper ovl_already_copied_up() Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 10/15] ovl: A new xattr OVL_XATTR_METACOPY for file on upper Vivek Goyal
2017-11-28 19:53 ` Amir Goldstein
2017-11-28 15:59 ` [PATCH v8 11/15] ovl: Fix ovl_getattr() to get number of blocks from lower Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 12/15] ovl: Set OVL_UPPERDATA flag during ovl_lookup() Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 13/15] ovl: Do not expose metacopy only upper dentry from d_real() Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 14/15] ovl: Fix encryption/compression status of a metacopy only file Vivek Goyal
2017-11-28 15:59 ` [PATCH v8 15/15] ovl: Enable metadata only feature Vivek Goyal
2017-11-28 17:04 ` [RFC PATCH v8 00/15] overlayfs: Delayed copy up of data Amir Goldstein
2017-11-28 18:22 ` Vivek Goyal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.