[SRU][Zesty][PATCH 00/10] LP#1728489: tar -x sometimes fails on overlayfs

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 00/10] LP#1728489: tar -x sometimes fails on overlayfs

Daniel Axtens
BugLink: https://bugs.launchpad.net/bugs/1728489

[SRU Justification]

[Impact]

A user is seeing failures from extracting tar archives on overlay
filesystems on the 4.4 kernel in constrained environments. The error
presents as:

`tar: ./deps/0/bin: Directory renamed before its status could be extracted`

Following this thread
(https://www.spinics.net/lists/linux-unionfs/msg00856.html), it appears
that this occurs when entries in the kernel's inode cache are
reclaimed, and subsequent lookups return new inode numbers.

Further testing showed that when setting
`/proc/sys/vm/vfs_cache_pressure` to 0 (don't allow the kernel to
reclaim inode cache entries due to memory pressure) the error does not
recur, supporting the hypothesis that cache entries are being
evicted. However, this setting may lead to a kernel OOM so is not a
reasonable workaround even temporarily.

The error cannot be reproduced on a 4.13 kernel, due to the series at
https://www.spinics.net/lists/linux-fsdevel/msg110235.html. The
particular relevant commit is
b7a807dc2010334e62e0afd89d6f7a8913eb14ff, which needs a couple of
dependencies.

[Fix]
For Zesty, backport the entire series.
For Xenial, where a full backport is not feasible, backport the key
commit and the short list of dependencies.

[Testcase]

# Testing this bug

The testcase for this particular bug is simple - create an overlay
filesystem with all layers on the same underlying file system, and
then see if the inode of a directory is constant across dropping the
caches:

mkdir -p /upper/upper /upper/work /lower
mount -t overlay none /mnt -o lowerdir=/lower,upperdir=/upper/upper,workdir=/upper/work
cd /mnt
mkdir a
stat a # observe inode number
echo 2 > /proc/sys/vm/drop_caches
stat a # compare inode number

If the inode number is the same, the fix is successful.

# Regression testing

I have run the unionmount test suite from
https://git.infradead.org/users/dhowells/unionmount-testsuite.git in
overlay mode (./run --ov), and verified that it still passes.

(The series cover letter mentions a fork of the test suite at
https://github.com/amir73il/unionmount-testsuite/commits/overlayfs-devel. I
have *not* attempted to get this running: it assumes a range of
changes that are not present in our kernels.)

[Regression Potential]

As this changes overlayfs, there is potential for regression in the
form of unexpected breakages to overlaysfs behaviour.

I think this is adequately addressed by the regression testing.

One option to reduce the regression potential on Zesty is to reduce
the set of patches applied - rather than including the whole series we
could include just the patches to solve this bug, which are much
easier to inspect for correctness.

Amir Goldstein (9):
  ovl: check if all layers are on the same fs
  ovl: store file handle of lower inode on copy up
  ovl: use an auxiliary var for overlay root entry
  ovl: lookup non-dir copy-up-origin by file handle
  ovl: set the ORIGIN type flag
  ovl: persistent inode number for directories
  ovl: constant st_ino/st_dev across copy up
  ovl: persistent inode numbers for upper hardlinks
  ovl: update documentation w.r.t. constant inode numbers

Miklos Szeredi (1):
  ovl: merge getattr for dir and nondir

 Documentation/filesystems/overlayfs.txt |   9 +-
 fs/overlayfs/copy_up.c                  |  82 +++++++++++++++++++
 fs/overlayfs/dir.c                      |  34 +-------
 fs/overlayfs/inode.c                    |  64 ++++++++++++++-
 fs/overlayfs/namei.c                    | 141 ++++++++++++++++++++++++++++++--
 fs/overlayfs/overlayfs.h                |  41 ++++++++++
 fs/overlayfs/ovl_entry.h                |   2 +
 fs/overlayfs/super.c                    |   8 ++
 fs/overlayfs/util.c                     |  17 +++-
 9 files changed, 355 insertions(+), 43 deletions(-)

--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 01/10] ovl: check if all layers are on the same fs

Daniel Axtens
From: Amir Goldstein <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1728489

Some features can only work when all layers are on the same fs.  Test this
condition during mount time, so features can check them later.

Add helper ovl_same_sb() to return the common super block in case all
layers are on the same fs.

Signed-off-by: Amir Goldstein <[hidden email]>
Signed-off-by: Miklos Szeredi <[hidden email]>
(backported from commit 7bcd74b98d7bac3e5149894caaf72de6989af7f0)
Signed-off-by: Daniel Axtens <[hidden email]>
---
 fs/overlayfs/overlayfs.h | 1 +
 fs/overlayfs/ovl_entry.h | 2 ++
 fs/overlayfs/super.c     | 8 ++++++++
 fs/overlayfs/util.c      | 7 +++++++
 4 files changed, 18 insertions(+)

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index f14fb7b56b09..ee05d77ad7cd 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -154,6 +154,7 @@ int ovl_want_write(struct dentry *dentry);
 void ovl_drop_write(struct dentry *dentry);
 struct dentry *ovl_workdir(struct dentry *dentry);
 const struct cred *ovl_override_creds(struct super_block *sb);
+struct super_block *ovl_same_sb(struct super_block *sb);
 struct ovl_entry *ovl_alloc_entry(unsigned int numlower);
 bool ovl_dentry_remote(struct dentry *dentry);
 bool ovl_dentry_weird(struct dentry *dentry);
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index d14bca1850d9..1894ce31c20b 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -27,6 +27,8 @@ struct ovl_fs {
  struct ovl_config config;
  /* creds of process who forced instantiation of super block */
  const struct cred *creator_cred;
+ /* sb common to all layers */
+ struct super_block *same_sb;
 };
 
 /* private information held for every overlayfs dentry */
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index ed2607904a1a..18c8bb3baff5 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -865,11 +865,19 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 
  ufs->lower_mnt[ufs->numlower] = mnt;
  ufs->numlower++;
+
+ /* Check if all lower layers are on same sb */
+ if (i == 0)
+ ufs->same_sb = mnt->mnt_sb;
+ else if (ufs->same_sb != mnt->mnt_sb)
+ ufs->same_sb = NULL;
  }
 
  /* If the upper fs is nonexistent, we mark overlayfs r/o too */
  if (!ufs->upper_mnt)
  sb->s_flags |= MS_RDONLY;
+ else if (ufs->upper_mnt->mnt_sb != ufs->same_sb)
+ ufs->same_sb = NULL;
 
  if (remote)
  sb->s_d_op = &ovl_reval_dentry_operations;
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 952286f4826c..9889daaae78b 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -39,6 +39,13 @@ const struct cred *ovl_override_creds(struct super_block *sb)
  return override_creds(ofs->creator_cred);
 }
 
+struct super_block *ovl_same_sb(struct super_block *sb)
+{
+ struct ovl_fs *ofs = sb->s_fs_info;
+
+ return ofs->same_sb;
+}
+
 struct ovl_entry *ovl_alloc_entry(unsigned int numlower)
 {
  size_t size = offsetof(struct ovl_entry, lowerstack[numlower]);
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 02/10] ovl: store file handle of lower inode on copy up

Daniel Axtens
In reply to this post by Daniel Axtens
From: Amir Goldstein <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1728489

Sometimes it is interesting to know if an upper file is pure upper or a
copy up target, and if it is a copy up target, it may be interesting to
find the copy up origin.

This will be used to preserve lower inode numbers across copy up.

Store the lower inode file handle in upper inode extended attribute
overlay.origin on copy up to use it later for these cases.  Store the lower
filesystem uuid along side the file handle, so we can validate that we are
looking for the origin file in the original fs.

If lower fs does not support NFS export ops store a zero sized xattr so we
can always use the overlay.origin xattr to distinguish between a copy up
and a pure upper inode.

Signed-off-by: Amir Goldstein <[hidden email]>
Signed-off-by: Miklos Szeredi <[hidden email]>
(backported from commit 3a1e819b4e80216e00ef6a4dfe67fa142450c5e1)
Signed-off-by: Daniel Axtens <[hidden email]>
---
 fs/overlayfs/copy_up.c   | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/overlayfs/overlayfs.h | 36 +++++++++++++++++++++
 2 files changed, 118 insertions(+)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 23e47e4af3c1..1173ea56e2e0 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -19,6 +19,7 @@
 #include <linux/namei.h>
 #include <linux/fdtable.h>
 #include <linux/ratelimit.h>
+#include <linux/exportfs.h>
 #include "overlayfs.h"
 
 #define OVL_COPY_UP_CHUNK_SIZE (1 << 20)
@@ -247,6 +248,79 @@ int ovl_set_attr(struct dentry *upperdentry, struct kstat *stat)
  return err;
 }
 
+static struct ovl_fh *ovl_encode_fh(struct dentry *lower, uuid_be *uuid)
+{
+ struct ovl_fh *fh;
+ int fh_type, fh_len, dwords;
+ void *buf;
+ int buflen = MAX_HANDLE_SZ;
+
+ buf = kmalloc(buflen, GFP_TEMPORARY);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+
+ /*
+ * We encode a non-connectable file handle for non-dir, because we
+ * only need to find the lower inode number and we don't want to pay
+ * the price or reconnecting the dentry.
+ */
+ dwords = buflen >> 2;
+ fh_type = exportfs_encode_fh(lower, buf, &dwords, 0);
+ buflen = (dwords << 2);
+
+ fh = ERR_PTR(-EIO);
+ if (WARN_ON(fh_type < 0) ||
+    WARN_ON(buflen > MAX_HANDLE_SZ) ||
+    WARN_ON(fh_type == FILEID_INVALID))
+ goto out;
+
+ BUILD_BUG_ON(MAX_HANDLE_SZ + offsetof(struct ovl_fh, fid) > 255);
+ fh_len = offsetof(struct ovl_fh, fid) + buflen;
+ fh = kmalloc(fh_len, GFP_KERNEL);
+ if (!fh) {
+ fh = ERR_PTR(-ENOMEM);
+ goto out;
+ }
+
+ fh->version = OVL_FH_VERSION;
+ fh->magic = OVL_FH_MAGIC;
+ fh->type = fh_type;
+ fh->flags = OVL_FH_FLAG_CPU_ENDIAN;
+ fh->len = fh_len;
+ fh->uuid = *uuid;
+ memcpy(fh->fid, buf, buflen);
+
+out:
+ kfree(buf);
+ return fh;
+}
+
+static int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
+  struct dentry *upper)
+{
+ struct super_block *sb = lower->d_sb;
+ uuid_be *uuid = (uuid_be *) &sb->s_uuid;
+ const struct ovl_fh *fh = NULL;
+ int err;
+
+ /*
+ * When lower layer doesn't support export operations store a 'null' fh,
+ * so we can use the overlay.origin xattr to distignuish between a copy
+ * up and a pure upper inode.
+ */
+ if (sb->s_export_op && sb->s_export_op->fh_to_dentry &&
+    uuid_be_cmp(*uuid, NULL_UUID_BE)) {
+ fh = ovl_encode_fh(lower, uuid);
+ if (IS_ERR(fh))
+ return PTR_ERR(fh);
+ }
+
+ err = ovl_do_setxattr(upper, OVL_XATTR_ORIGIN, fh, fh ? fh->len : 0, 0);
+ kfree(fh);
+
+ return err;
+}
+
 static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
       struct dentry *dentry, struct path *lowerpath,
       struct kstat *stat, const char *link)
@@ -315,6 +389,14 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,
  if (err)
  goto out_cleanup;
 
+ /*
+ * Store identifier of lower inode in upper inode xattr to
+ * allow lookup of the copy up origin inode.
+ */
+ err = ovl_set_origin(dentry, lowerpath->dentry, newdentry);
+ if (err)
+ goto out_cleanup;
+
  err = ovl_do_rename(wdir, newdentry, udir, upper, 0);
  if (err)
  goto out_cleanup;
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index ee05d77ad7cd..9442b35dba84 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -8,6 +8,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/uuid.h>
 
 enum ovl_path_type {
  __OVL_PATH_UPPER = (1 << 0),
@@ -20,6 +21,41 @@ enum ovl_path_type {
 #define OVL_XATTR_PREFIX XATTR_TRUSTED_PREFIX "overlay."
 #define OVL_XATTR_OPAQUE OVL_XATTR_PREFIX "opaque"
 #define OVL_XATTR_REDIRECT OVL_XATTR_PREFIX "redirect"
+#define OVL_XATTR_ORIGIN OVL_XATTR_PREFIX "origin"
+
+/*
+ * The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
+ * where:
+ * origin.fh - exported file handle of the lower file
+ * origin.uuid - uuid of the lower filesystem
+ */
+#define OVL_FH_VERSION 0
+#define OVL_FH_MAGIC 0xfb
+
+/* CPU byte order required for fid decoding:  */
+#define OVL_FH_FLAG_BIG_ENDIAN (1 << 0)
+#define OVL_FH_FLAG_ANY_ENDIAN (1 << 1)
+
+#define OVL_FH_FLAG_ALL (OVL_FH_FLAG_BIG_ENDIAN | OVL_FH_FLAG_ANY_ENDIAN)
+
+#if defined(__LITTLE_ENDIAN)
+#define OVL_FH_FLAG_CPU_ENDIAN 0
+#elif defined(__BIG_ENDIAN)
+#define OVL_FH_FLAG_CPU_ENDIAN OVL_FH_FLAG_BIG_ENDIAN
+#else
+#error Endianness not defined
+#endif
+
+/* On-disk and in-memeory format for redirect by file handle */
+struct ovl_fh {
+ u8 version; /* 0 */
+ u8 magic; /* 0xfb */
+ u8 len; /* size of this header + size of fid */
+ u8 flags; /* OVL_FH_FLAG_* */
+ u8 type; /* fid_type of fid */
+ uuid_be uuid; /* uuid of filesystem */
+ u8 fid[0]; /* file identifier */
+} __packed;
 
 #define OVL_ISUPPER_MASK 1UL
 
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 03/10] ovl: use an auxiliary var for overlay root entry

Daniel Axtens
In reply to this post by Daniel Axtens
From: Amir Goldstein <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1728489

Signed-off-by: Amir Goldstein <[hidden email]>
Signed-off-by: Miklos Szeredi <[hidden email]>
(cherry picked from commit c22205d0584bc65cfc9a65db0e15a9b69f5cdf64)
Signed-off-by: Daniel Axtens <[hidden email]>

---
(There's no description in the upstream commit either.)
---
 fs/overlayfs/namei.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 023bb0b03352..42a98e21cf41 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -219,6 +219,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
  const struct cred *old_cred;
  struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
  struct ovl_entry *poe = dentry->d_parent->d_fsdata;
+ struct ovl_entry *roe = dentry->d_sb->s_root->d_fsdata;
  struct path *stack = NULL;
  struct dentry *upperdir, *upperdentry = NULL;
  unsigned int ctr = 0;
@@ -258,7 +259,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
  if (!upperredirect)
  goto out_put_upper;
  if (d.redirect[0] == '/')
- poe = dentry->d_sb->s_root->d_fsdata;
+ poe = roe;
  }
  upperopaque = d.opaque;
  }
@@ -289,10 +290,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
  if (d.stop)
  break;
 
- if (d.redirect &&
-    d.redirect[0] == '/' &&
-    poe != dentry->d_sb->s_root->d_fsdata) {
- poe = dentry->d_sb->s_root->d_fsdata;
+ if (d.redirect && d.redirect[0] == '/' && poe != roe) {
+ poe = roe;
 
  /* Find the current layer on the root dentry */
  for (i = 0; i < poe->numlower; i++)
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 04/10] ovl: lookup non-dir copy-up-origin by file handle

Daniel Axtens
In reply to this post by Daniel Axtens
From: Amir Goldstein <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1728489

If overlay.origin xattr is found on a non-dir upper inode try to get lower
dentry by calling exportfs_decode_fh().

On failure to lookup by file handle to lower layer, do not lookup the copy
up origin by name, because the lower found by name could be another file in
case the upper file was renamed.

Signed-off-by: Amir Goldstein <[hidden email]>
Signed-off-by: Miklos Szeredi <[hidden email]>
(cherry picked from commit a9d019573e881472aa62f093fa599ad68cd0fc1e)
Signed-off-by: Daniel Axtens <[hidden email]>
---
 fs/overlayfs/namei.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 42a98e21cf41..1ef8735cefc8 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -11,6 +11,8 @@
 #include <linux/namei.h>
 #include <linux/xattr.h>
 #include <linux/ratelimit.h>
+#include <linux/mount.h>
+#include <linux/exportfs.h>
 #include "overlayfs.h"
 #include "ovl_entry.h"
 
@@ -80,6 +82,90 @@ static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
  goto err_free;
 }
 
+static int ovl_acceptable(void *ctx, struct dentry *dentry)
+{
+ return 1;
+}
+
+static struct dentry *ovl_get_origin(struct dentry *dentry,
+     struct vfsmount *mnt)
+{
+ int res;
+ struct ovl_fh *fh = NULL;
+ struct dentry *origin = NULL;
+ int bytes;
+
+ res = vfs_getxattr(dentry, OVL_XATTR_ORIGIN, NULL, 0);
+ if (res < 0) {
+ if (res == -ENODATA || res == -EOPNOTSUPP)
+ return NULL;
+ goto fail;
+ }
+ /* Zero size value means "copied up but origin unknown" */
+ if (res == 0)
+ return NULL;
+
+ fh  = kzalloc(res, GFP_TEMPORARY);
+ if (!fh)
+ return ERR_PTR(-ENOMEM);
+
+ res = vfs_getxattr(dentry, OVL_XATTR_ORIGIN, fh, res);
+ if (res < 0)
+ goto fail;
+
+ if (res < sizeof(struct ovl_fh) || res < fh->len)
+ goto invalid;
+
+ if (fh->magic != OVL_FH_MAGIC)
+ goto invalid;
+
+ /* Treat larger version and unknown flags as "origin unknown" */
+ if (fh->version > OVL_FH_VERSION || fh->flags & ~OVL_FH_FLAG_ALL)
+ goto out;
+
+ /* Treat endianness mismatch as "origin unknown" */
+ if (!(fh->flags & OVL_FH_FLAG_ANY_ENDIAN) &&
+    (fh->flags & OVL_FH_FLAG_BIG_ENDIAN) != OVL_FH_FLAG_CPU_ENDIAN)
+ goto out;
+
+ bytes = (fh->len - offsetof(struct ovl_fh, fid));
+
+ /*
+ * Make sure that the stored uuid matches the uuid of the lower
+ * layer where file handle will be decoded.
+ */
+ if (uuid_be_cmp(fh->uuid, *(uuid_be *) &mnt->mnt_sb->s_uuid))
+ goto out;
+
+ origin = exportfs_decode_fh(mnt, (struct fid *)fh->fid,
+    bytes >> 2, (int)fh->type,
+    ovl_acceptable, NULL);
+ if (IS_ERR(origin)) {
+ /* Treat stale file handle as "origin unknown" */
+ if (origin == ERR_PTR(-ESTALE))
+ origin = NULL;
+ goto out;
+ }
+
+ if (ovl_dentry_weird(origin) ||
+    ((d_inode(origin)->i_mode ^ d_inode(dentry)->i_mode) & S_IFMT)) {
+ dput(origin);
+ origin = NULL;
+ goto invalid;
+ }
+
+out:
+ kfree(fh);
+ return origin;
+
+fail:
+ pr_warn_ratelimited("overlayfs: failed to get origin (%i)\n", res);
+ goto out;
+invalid:
+ pr_warn_ratelimited("overlayfs: invalid origin (%*phN)\n", res, fh);
+ goto out;
+}
+
 static bool ovl_is_opaquedir(struct dentry *dentry)
 {
  int res;
@@ -191,6 +277,45 @@ static int ovl_lookup_layer(struct dentry *base, struct ovl_lookup_data *d,
  return 0;
 }
 
+
+static int ovl_check_origin(struct dentry *dentry, struct dentry *upperdentry,
+    struct path **stackp, unsigned int *ctrp)
+{
+ struct super_block *same_sb = ovl_same_sb(dentry->d_sb);
+ struct ovl_entry *roe = dentry->d_sb->s_root->d_fsdata;
+ struct vfsmount *mnt;
+ struct dentry *origin;
+
+ if (!same_sb || !roe->numlower)
+ return 0;
+
+       /*
+ * Since all layers are on the same fs, we use the first layer for
+ * decoding the file handle.  We may get a disconnected dentry,
+ * which is fine, because we only need to hold the origin inode in
+ * cache and use its inode number.  We may even get a connected dentry,
+ * that is not under the first layer's root.  That is also fine for
+ * using it's inode number - it's the same as if we held a reference
+ * to a dentry in first layer that was moved under us.
+ */
+ mnt = roe->lowerstack[0].mnt;
+
+ origin = ovl_get_origin(upperdentry, mnt);
+ if (IS_ERR_OR_NULL(origin))
+ return PTR_ERR(origin);
+
+ BUG_ON(*stackp || *ctrp);
+ *stackp = kmalloc(sizeof(struct path), GFP_TEMPORARY);
+ if (!*stackp) {
+ dput(origin);
+ return -ENOMEM;
+ }
+ **stackp = (struct path) { .dentry = origin, .mnt = mnt };
+ *ctrp = 1;
+
+ return 0;
+}
+
 /*
  * Returns next layer in stack starting from top.
  * Returns -1 if this is the last layer.
@@ -253,6 +378,13 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
  err = -EREMOTE;
  goto out;
  }
+ if (upperdentry && !d.is_dir) {
+ BUG_ON(!d.stop || d.redirect);
+ err = ovl_check_origin(dentry, upperdentry,
+       &stack, &ctr);
+ if (err)
+ goto out;
+ }
 
  if (d.redirect) {
  upperredirect = kstrdup(d.redirect, GFP_KERNEL);
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 05/10] ovl: set the ORIGIN type flag

Daniel Axtens
In reply to this post by Daniel Axtens
From: Amir Goldstein <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1728489

For directory entries, non zero oe->numlower implies OVL_TYPE_MERGE.
Define a new type flag OVL_TYPE_ORIGIN to indicate that an entry holds a
reference to its lower copy up origin.

For directory entries ORIGIN := MERGE && UPPER. For non-dir entries ORIGIN
means that a lower type dentry has been recently copied up or that we were
able to find the copy up origin from overlay.origin xattr.

Signed-off-by: Amir Goldstein <[hidden email]>
Signed-off-by: Miklos Szeredi <[hidden email]>
(cherry picked from commit 595485033db2c24178257698254fd4182fdb4123)
Signed-off-by: Daniel Axtens <[hidden email]>
---
 fs/overlayfs/overlayfs.h |  2 ++
 fs/overlayfs/util.c      | 10 ++++++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 9442b35dba84..883a77b0636c 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -13,10 +13,12 @@
 enum ovl_path_type {
  __OVL_PATH_UPPER = (1 << 0),
  __OVL_PATH_MERGE = (1 << 1),
+ __OVL_PATH_ORIGIN = (1 << 2),
 };
 
 #define OVL_TYPE_UPPER(type) ((type) & __OVL_PATH_UPPER)
 #define OVL_TYPE_MERGE(type) ((type) & __OVL_PATH_MERGE)
+#define OVL_TYPE_ORIGIN(type) ((type) & __OVL_PATH_ORIGIN)
 
 #define OVL_XATTR_PREFIX XATTR_TRUSTED_PREFIX "overlay."
 #define OVL_XATTR_OPAQUE OVL_XATTR_PREFIX "opaque"
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 9889daaae78b..b65808b2c3ce 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -81,11 +81,13 @@ enum ovl_path_type ovl_path_type(struct dentry *dentry)
  type = __OVL_PATH_UPPER;
 
  /*
- * Non-dir dentry can hold lower dentry from previous
- * location.
+ * Non-dir dentry can hold lower dentry of its copy up origin.
  */
- if (oe->numlower && d_is_dir(dentry))
- type |= __OVL_PATH_MERGE;
+ if (oe->numlower) {
+ type |= __OVL_PATH_ORIGIN;
+ if (d_is_dir(dentry))
+ type |= __OVL_PATH_MERGE;
+ }
  } else {
  if (oe->numlower > 1)
  type |= __OVL_PATH_MERGE;
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 06/10] ovl: persistent inode number for directories

Daniel Axtens
In reply to this post by Daniel Axtens
From: Amir Goldstein <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1728489

stat(2) on overlay directories reports the overlay temp inode
number, which is constant across copy up, but is not persistent.

When all layers are on the same fs, report the copy up origin inode
number for directories.

This inode number is persistent, unique across the overlay mount and
constant across copy up.

Signed-off-by: Amir Goldstein <[hidden email]>
Signed-off-by: Miklos Szeredi <[hidden email]>
(backported from commit b7a807dc2010334e62e0afd89d6f7a8913eb14ff)
Signed-off-by: Daniel Axtens <[hidden email]>
---
 fs/overlayfs/dir.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 2e1f16d22c16..94d9ee8aa837 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -149,12 +149,38 @@ static int ovl_dir_getattr(struct vfsmount *mnt, struct dentry *dentry,
  type = ovl_path_real(dentry, &realpath);
  old_cred = ovl_override_creds(dentry->d_sb);
  err = vfs_getattr(&realpath, stat);
- revert_creds(old_cred);
  if (err)
- return err;
+ goto out;
+
+ /*
+ * When all layers are on the same fs, use the copy-up-origin st_ino,
+ * which is persistent, unique and constant across copy up.
+ *
+ * Otherwise the pair {real st_ino; overlay st_dev} is not unique, so
+ * use the non persistent overlay st_ino.
+ */
+ if (ovl_same_sb(dentry->d_sb)) {
+ if (OVL_TYPE_ORIGIN(type)) {
+ struct kstat lowerstat;
+
+ ovl_path_lower(dentry, &realpath);
+ err = vfs_getattr(&realpath, &lowerstat);
+ if (err)
+ goto out;
+
+ WARN_ON_ONCE(stat->dev != lowerstat.dev);
+ stat->ino = lowerstat.ino;
+ }
+ } else {
+ stat->ino = dentry->d_inode->i_ino;
+ }
 
+ /*
+ * Always use the overlay st_dev for directories, so 'find -xdev' will
+ * scan the entire overlay mount and won't cross the overlay mount
+ * boundaries.
+ */
  stat->dev = dentry->d_sb->s_dev;
- stat->ino = dentry->d_inode->i_ino;
 
  /*
  * It's probably not worth it to count subdirs to get the
@@ -163,8 +189,10 @@ static int ovl_dir_getattr(struct vfsmount *mnt, struct dentry *dentry,
  */
  if (OVL_TYPE_MERGE(type))
  stat->nlink = 1;
+out:
+ revert_creds(old_cred);
 
- return 0;
+ return err;
 }
 
 /* Common operations required to be done after creation of file on upper */
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 07/10] ovl: constant st_ino/st_dev across copy up

Daniel Axtens
In reply to this post by Daniel Axtens
From: Amir Goldstein <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1728489

When all layers are on the same underlying filesystem, let stat(2) return
st_dev/st_ino values of the copy up origin inode if it is known.

This results in constant st_ino/st_dev representation of files in an
overlay mount before and after copy up.

When the underlying filesystem support NFS exportfs, the result is also
persistent st_ino/st_dev representation before and after mount cycle.

Lower hardlinks are broken on copy up to different upper files, so we
cannot use the lower origin st_ino for those different files, even for the
same fs case.

When all overlay layers are on the same fs, use overlay st_dev for non-dirs
to get the correct result from du -x.

Signed-off-by: Amir Goldstein <[hidden email]>
Signed-off-by: Miklos Szeredi <[hidden email]>
(backported from commit 72b608f08528458334218a809d66ea94d924c378)
Signed-off-by: Daniel Axtens <[hidden email]>
---
 fs/overlayfs/inode.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 08643ac44a02..c9eb9193f4ef 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -59,14 +59,50 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr)
 static int ovl_getattr(struct vfsmount *mnt, struct dentry *dentry,
  struct kstat *stat)
 {
+ enum ovl_path_type type;
  struct path realpath;
  const struct cred *old_cred;
  int err;
 
- ovl_path_real(dentry, &realpath);
+ type = ovl_path_real(dentry, &realpath);
  old_cred = ovl_override_creds(dentry->d_sb);
  err = vfs_getattr(&realpath, stat);
+ if (err)
+ goto out;
+
+ /*
+ * When all layers are on the same fs, all real inode number are
+ * unique, so we use the overlay st_dev, which is friendly to du -x.
+ *
+ * We also use st_ino of the copy up origin, if we know it.
+ * This guaranties constant st_dev/st_ino across copy up.
+ *
+ * If filesystem supports NFS export ops, this also guaranties
+ * persistent st_ino across mount cycle.
+ */
+ if (ovl_same_sb(dentry->d_sb)) {
+ if (OVL_TYPE_ORIGIN(type)) {
+ struct kstat lowerstat;
+
+ ovl_path_lower(dentry, &realpath);
+ err = vfs_getattr(&realpath, &lowerstat);
+ if (err)
+ goto out;
+
+ WARN_ON_ONCE(stat->dev != lowerstat.dev);
+ /*
+ * Lower hardlinks are broken on copy up to different
+ * upper files, so we cannot use the lower origin st_ino
+ * for those different files, even for the same fs case.
+ */
+ if (lowerstat.nlink == 1)
+ stat->ino = lowerstat.ino;
+ }
+ stat->dev = dentry->d_sb->s_dev;
+ }
+out:
  revert_creds(old_cred);
+
  return err;
 }
 
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 08/10] ovl: merge getattr for dir and nondir

Daniel Axtens
In reply to this post by Daniel Axtens
From: Miklos Szeredi <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1728489

Signed-off-by: Miklos Szeredi <[hidden email]>
(backported from commit 5b712091a3a3904b0ae8311e18e6b540a070d464)
Signed-off-by: Daniel Axtens <[hidden email]>

---
This patch also has no description in upstream.
---
 fs/overlayfs/dir.c       | 59 +-----------------------------------------------
 fs/overlayfs/inode.c     | 28 ++++++++++++++++++++---
 fs/overlayfs/overlayfs.h |  2 ++
 3 files changed, 28 insertions(+), 61 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 94d9ee8aa837..0f57a42426ef 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -138,63 +138,6 @@ static int ovl_set_opaque(struct dentry *dentry, struct dentry *upperdentry)
  return err;
 }
 
-static int ovl_dir_getattr(struct vfsmount *mnt, struct dentry *dentry,
- struct kstat *stat)
-{
- int err;
- enum ovl_path_type type;
- struct path realpath;
- const struct cred *old_cred;
-
- type = ovl_path_real(dentry, &realpath);
- old_cred = ovl_override_creds(dentry->d_sb);
- err = vfs_getattr(&realpath, stat);
- if (err)
- goto out;
-
- /*
- * When all layers are on the same fs, use the copy-up-origin st_ino,
- * which is persistent, unique and constant across copy up.
- *
- * Otherwise the pair {real st_ino; overlay st_dev} is not unique, so
- * use the non persistent overlay st_ino.
- */
- if (ovl_same_sb(dentry->d_sb)) {
- if (OVL_TYPE_ORIGIN(type)) {
- struct kstat lowerstat;
-
- ovl_path_lower(dentry, &realpath);
- err = vfs_getattr(&realpath, &lowerstat);
- if (err)
- goto out;
-
- WARN_ON_ONCE(stat->dev != lowerstat.dev);
- stat->ino = lowerstat.ino;
- }
- } else {
- stat->ino = dentry->d_inode->i_ino;
- }
-
- /*
- * Always use the overlay st_dev for directories, so 'find -xdev' will
- * scan the entire overlay mount and won't cross the overlay mount
- * boundaries.
- */
- stat->dev = dentry->d_sb->s_dev;
-
- /*
- * It's probably not worth it to count subdirs to get the
- * correct link count.  nlink=1 seems to pacify 'find' and
- * other utilities.
- */
- if (OVL_TYPE_MERGE(type))
- stat->nlink = 1;
-out:
- revert_creds(old_cred);
-
- return err;
-}
-
 /* Common operations required to be done after creation of file on upper */
 static void ovl_instantiate(struct dentry *dentry, struct inode *inode,
     struct dentry *newdentry, bool hardlink)
@@ -1096,7 +1039,7 @@ const struct inode_operations ovl_dir_inode_operations = {
  .create = ovl_create,
  .mknod = ovl_mknod,
  .permission = ovl_permission,
- .getattr = ovl_dir_getattr,
+ .getattr = ovl_getattr,
  .listxattr = ovl_listxattr,
  .get_acl = ovl_get_acl,
  .update_time = ovl_update_time,
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index c9eb9193f4ef..c80109b340a4 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -56,12 +56,13 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr)
  return err;
 }
 
-static int ovl_getattr(struct vfsmount *mnt, struct dentry *dentry,
- struct kstat *stat)
+int ovl_getattr(struct vfsmount *mnt, struct dentry *dentry,
+ struct kstat *stat)
 {
  enum ovl_path_type type;
  struct path realpath;
  const struct cred *old_cred;
+ bool is_dir = S_ISDIR(dentry->d_inode->i_mode);
  int err;
 
  type = ovl_path_real(dentry, &realpath);
@@ -95,11 +96,32 @@ static int ovl_getattr(struct vfsmount *mnt, struct dentry *dentry,
  * upper files, so we cannot use the lower origin st_ino
  * for those different files, even for the same fs case.
  */
- if (lowerstat.nlink == 1)
+ if (is_dir || lowerstat.nlink == 1)
  stat->ino = lowerstat.ino;
  }
  stat->dev = dentry->d_sb->s_dev;
+ } else if (is_dir) {
+ /*
+ * If not all layers are on the same fs the pair {real st_ino;
+ * overlay st_dev} is not unique, so use the non persistent
+ * overlay st_ino.
+ *
+ * Always use the overlay st_dev for directories, so 'find
+ * -xdev' will scan the entire overlay mount and won't cross the
+ * overlay mount boundaries.
+ */
+ stat->dev = dentry->d_sb->s_dev;
+ stat->ino = dentry->d_inode->i_ino;
  }
+
+ /*
+ * It's probably not worth it to count subdirs to get the
+ * correct link count.  nlink=1 seems to pacify 'find' and
+ * other utilities.
+ */
+ if (is_dir && OVL_TYPE_MERGE(type))
+ stat->nlink = 1;
+
 out:
  revert_creds(old_cred);
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 883a77b0636c..929165460f28 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -237,6 +237,8 @@ void ovl_workdir_cleanup(struct inode *dir, struct vfsmount *mnt,
 
 /* inode.c */
 int ovl_setattr(struct dentry *dentry, struct iattr *attr);
+int ovl_getattr(struct vfsmount *mnt, struct dentry *dentry,
+ struct kstat *stat);
 int ovl_permission(struct inode *inode, int mask);
 int ovl_xattr_set(struct dentry *dentry, const char *name, const void *value,
   size_t size, int flags);
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 09/10] ovl: persistent inode numbers for upper hardlinks

Daniel Axtens
In reply to this post by Daniel Axtens
From: Amir Goldstein <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1728489

An upper type non directory dentry that is a copy up target
should have a reference to its lower copy up origin.

There are three ways for an upper type dentry to be instantiated:
1. A lower type dentry that is being copied up
2. An entry that is found in upper dir by ovl_lookup()
3. A negative dentry is hardlinked to an upper type dentry

In the first case, the lower reference is set before copy up.
In the second case, the lower reference is found by ovl_lookup().
In the last case of hardlinked upper dentry, it is not easy to
update the lower reference of the negative dentry.  Instead,
drop the newly hardlinked negative dentry from dcache and let
the next access call ovl_lookup() to find its lower reference.

This makes sure that the inode number reported by stat(2) after
the hardlink is created is the same inode number that will be
reported by stat(2) after mount cycle, which is the inode number
of the lower copy up origin of the hardlink source.

NOTE that this does not fix breaking of lower hardlinks on copy
up, but only fixes the case of lower nlink == 1, whose upper copy
up inode is hardlinked in upper dir.

Signed-off-by: Amir Goldstein <[hidden email]>
Signed-off-by: Miklos Szeredi <[hidden email]>
(cherry picked from commit 5b6c9053fb38a66fd5c6177fcf5022b24767811a)
Signed-off-by: Daniel Axtens <[hidden email]>
---
 fs/overlayfs/dir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 0f57a42426ef..bee957923dc9 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -152,6 +152,9 @@ static void ovl_instantiate(struct dentry *dentry, struct inode *inode,
  inc_nlink(inode);
  }
  d_instantiate(dentry, inode);
+ /* Force lookup of new upper hardlink to find its lower */
+ if (hardlink)
+ d_drop(dentry);
 }
 
 static bool ovl_type_merge(struct dentry *dentry)
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Zesty][PATCH 10/10] ovl: update documentation w.r.t. constant inode numbers

Daniel Axtens
In reply to this post by Daniel Axtens
From: Amir Goldstein <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1728489

Signed-off-by: Amir Goldstein <[hidden email]>
Signed-off-by: Miklos Szeredi <[hidden email]>
(cherry picked from commit 65f2673832d4647f6d46783d8d745d2b15d090c4)
Signed-off-by: Daniel Axtens <[hidden email]>

---
Once again, no upstream description.
---
 Documentation/filesystems/overlayfs.txt | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
index 634d03e20c2d..c9e884b52698 100644
--- a/Documentation/filesystems/overlayfs.txt
+++ b/Documentation/filesystems/overlayfs.txt
@@ -21,12 +21,19 @@ from accessing the corresponding object from the original filesystem.
 This is most obvious from the 'st_dev' field returned by stat(2).
 
 While directories will report an st_dev from the overlay-filesystem,
-all non-directory objects will report an st_dev from the lower or
+non-directory objects may report an st_dev from the lower filesystem or
 upper filesystem that is providing the object.  Similarly st_ino will
 only be unique when combined with st_dev, and both of these can change
 over the lifetime of a non-directory object.  Many applications and
 tools ignore these values and will not be affected.
 
+In the special case of all overlay layers on the same underlying
+filesystem, all objects will report an st_dev from the overlay
+filesystem and st_ino from the underlying filesystem.  This will
+make the overlay mount more compliant with filesystem scanners and
+overlay objects will be distinguishable from the corresponding
+objects in the original filesystem.
+
 Upper and Lower
 ---------------
 
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Zesty][PATCH 00/10] LP#1728489: tar -x sometimes fails on overlayfs

Stefan Bader-2
In reply to this post by Daniel Axtens
On 30.10.2017 07:53, Daniel Axtens wrote:

> BugLink: https://bugs.launchpad.net/bugs/1728489
>
> [SRU Justification]
>
> [Impact]
>
> A user is seeing failures from extracting tar archives on overlay
> filesystems on the 4.4 kernel in constrained environments. The error
> presents as:
>
> `tar: ./deps/0/bin: Directory renamed before its status could be extracted`
>
> Following this thread
> (https://www.spinics.net/lists/linux-unionfs/msg00856.html), it appears
> that this occurs when entries in the kernel's inode cache are
> reclaimed, and subsequent lookups return new inode numbers.
>
> Further testing showed that when setting
> `/proc/sys/vm/vfs_cache_pressure` to 0 (don't allow the kernel to
> reclaim inode cache entries due to memory pressure) the error does not
> recur, supporting the hypothesis that cache entries are being
> evicted. However, this setting may lead to a kernel OOM so is not a
> reasonable workaround even temporarily.
>
> The error cannot be reproduced on a 4.13 kernel, due to the series at
> https://www.spinics.net/lists/linux-fsdevel/msg110235.html. The
> particular relevant commit is
> b7a807dc2010334e62e0afd89d6f7a8913eb14ff, which needs a couple of
> dependencies.
>
> [Fix]
> For Zesty, backport the entire series.
I would rather prefer to see the same minimal backport approach that has been
done for Xenial to be done in Zesty, too. Zesty is released and in SRU mode.
There should be rather strong reasons to dump a change that big into it.

-Stefan

> For Xenial, where a full backport is not feasible, backport the key
> commit and the short list of dependencies.
>
> [Testcase]
>
> # Testing this bug
>
> The testcase for this particular bug is simple - create an overlay
> filesystem with all layers on the same underlying file system, and
> then see if the inode of a directory is constant across dropping the
> caches:
>
> mkdir -p /upper/upper /upper/work /lower
> mount -t overlay none /mnt -o lowerdir=/lower,upperdir=/upper/upper,workdir=/upper/work
> cd /mnt
> mkdir a
> stat a # observe inode number
> echo 2 > /proc/sys/vm/drop_caches
> stat a # compare inode number
>
> If the inode number is the same, the fix is successful.
>
> # Regression testing
>
> I have run the unionmount test suite from
> https://git.infradead.org/users/dhowells/unionmount-testsuite.git in
> overlay mode (./run --ov), and verified that it still passes.
>
> (The series cover letter mentions a fork of the test suite at
> https://github.com/amir73il/unionmount-testsuite/commits/overlayfs-devel. I
> have *not* attempted to get this running: it assumes a range of
> changes that are not present in our kernels.)
>
> [Regression Potential]
>
> As this changes overlayfs, there is potential for regression in the
> form of unexpected breakages to overlaysfs behaviour.
>
> I think this is adequately addressed by the regression testing.
>
> One option to reduce the regression potential on Zesty is to reduce
> the set of patches applied - rather than including the whole series we
> could include just the patches to solve this bug, which are much
> easier to inspect for correctness.
>
> Amir Goldstein (9):
>   ovl: check if all layers are on the same fs
>   ovl: store file handle of lower inode on copy up
>   ovl: use an auxiliary var for overlay root entry
>   ovl: lookup non-dir copy-up-origin by file handle
>   ovl: set the ORIGIN type flag
>   ovl: persistent inode number for directories
>   ovl: constant st_ino/st_dev across copy up
>   ovl: persistent inode numbers for upper hardlinks
>   ovl: update documentation w.r.t. constant inode numbers
>
> Miklos Szeredi (1):
>   ovl: merge getattr for dir and nondir
>
>  Documentation/filesystems/overlayfs.txt |   9 +-
>  fs/overlayfs/copy_up.c                  |  82 +++++++++++++++++++
>  fs/overlayfs/dir.c                      |  34 +-------
>  fs/overlayfs/inode.c                    |  64 ++++++++++++++-
>  fs/overlayfs/namei.c                    | 141 ++++++++++++++++++++++++++++++--
>  fs/overlayfs/overlayfs.h                |  41 ++++++++++
>  fs/overlayfs/ovl_entry.h                |   2 +
>  fs/overlayfs/super.c                    |   8 ++
>  fs/overlayfs/util.c                     |  17 +++-
>  9 files changed, 355 insertions(+), 43 deletions(-)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

NACK: [SRU][Zesty][PATCH 00/10] LP#1728489: tar -x sometimes fails on overlayfs

Kleber Souza
In reply to this post by Daniel Axtens
A v2 of this patch series has been sent to the mailing list.


Kleber

On 10/30/17 07:53, Daniel Axtens wrote:

> BugLink: https://bugs.launchpad.net/bugs/1728489
>
> [SRU Justification]
>
> [Impact]
>
> A user is seeing failures from extracting tar archives on overlay
> filesystems on the 4.4 kernel in constrained environments. The error
> presents as:
>
> `tar: ./deps/0/bin: Directory renamed before its status could be extracted`
>
> Following this thread
> (https://www.spinics.net/lists/linux-unionfs/msg00856.html), it appears
> that this occurs when entries in the kernel's inode cache are
> reclaimed, and subsequent lookups return new inode numbers.
>
> Further testing showed that when setting
> `/proc/sys/vm/vfs_cache_pressure` to 0 (don't allow the kernel to
> reclaim inode cache entries due to memory pressure) the error does not
> recur, supporting the hypothesis that cache entries are being
> evicted. However, this setting may lead to a kernel OOM so is not a
> reasonable workaround even temporarily.
>
> The error cannot be reproduced on a 4.13 kernel, due to the series at
> https://www.spinics.net/lists/linux-fsdevel/msg110235.html. The
> particular relevant commit is
> b7a807dc2010334e62e0afd89d6f7a8913eb14ff, which needs a couple of
> dependencies.
>
> [Fix]
> For Zesty, backport the entire series.
> For Xenial, where a full backport is not feasible, backport the key
> commit and the short list of dependencies.
>
> [Testcase]
>
> # Testing this bug
>
> The testcase for this particular bug is simple - create an overlay
> filesystem with all layers on the same underlying file system, and
> then see if the inode of a directory is constant across dropping the
> caches:
>
> mkdir -p /upper/upper /upper/work /lower
> mount -t overlay none /mnt -o lowerdir=/lower,upperdir=/upper/upper,workdir=/upper/work
> cd /mnt
> mkdir a
> stat a # observe inode number
> echo 2 > /proc/sys/vm/drop_caches
> stat a # compare inode number
>
> If the inode number is the same, the fix is successful.
>
> # Regression testing
>
> I have run the unionmount test suite from
> https://git.infradead.org/users/dhowells/unionmount-testsuite.git in
> overlay mode (./run --ov), and verified that it still passes.
>
> (The series cover letter mentions a fork of the test suite at
> https://github.com/amir73il/unionmount-testsuite/commits/overlayfs-devel. I
> have *not* attempted to get this running: it assumes a range of
> changes that are not present in our kernels.)
>
> [Regression Potential]
>
> As this changes overlayfs, there is potential for regression in the
> form of unexpected breakages to overlaysfs behaviour.
>
> I think this is adequately addressed by the regression testing.
>
> One option to reduce the regression potential on Zesty is to reduce
> the set of patches applied - rather than including the whole series we
> could include just the patches to solve this bug, which are much
> easier to inspect for correctness.
>
> Amir Goldstein (9):
>   ovl: check if all layers are on the same fs
>   ovl: store file handle of lower inode on copy up
>   ovl: use an auxiliary var for overlay root entry
>   ovl: lookup non-dir copy-up-origin by file handle
>   ovl: set the ORIGIN type flag
>   ovl: persistent inode number for directories
>   ovl: constant st_ino/st_dev across copy up
>   ovl: persistent inode numbers for upper hardlinks
>   ovl: update documentation w.r.t. constant inode numbers
>
> Miklos Szeredi (1):
>   ovl: merge getattr for dir and nondir
>
>  Documentation/filesystems/overlayfs.txt |   9 +-
>  fs/overlayfs/copy_up.c                  |  82 +++++++++++++++++++
>  fs/overlayfs/dir.c                      |  34 +-------
>  fs/overlayfs/inode.c                    |  64 ++++++++++++++-
>  fs/overlayfs/namei.c                    | 141 ++++++++++++++++++++++++++++++--
>  fs/overlayfs/overlayfs.h                |  41 ++++++++++
>  fs/overlayfs/ovl_entry.h                |   2 +
>  fs/overlayfs/super.c                    |   8 ++
>  fs/overlayfs/util.c                     |  17 +++-
>  9 files changed, 355 insertions(+), 43 deletions(-)
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team