ovl: Allocate anonymous devs for lowerdirs

PATCH Superseded Chandan Rajendra 3 participants 13 messages Jun 23, 2017 – Sep 22, 2017 Overlay filesystem

Share & tools

Chandan Rajendra

author

2017-06-23 11:01 UTC #0 lore raw

PATCH v2 1/1 ovl: Allocate anonymous devs for lowerdirs

For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.

Signed-off-by Chandan Rajendra <[email protected]>

---
Changelog:
v1->v2: Drop code that provided unique st_dev across copy up.

 fs/overlayfs/inode.c     | 20 ++++++++++++++++++++
 fs/overlayfs/ovl_entry.h |  8 +++++++-
 fs/overlayfs/super.c     | 33 +++++++++++++++++++++++++--------
 3 files changed, 52 insertions(+), 9 deletions(-)

fs/overlayfs/inode.c +20 -0

index d613e2c..e30bdca 100644
@@ -8,11 +8,29 @@
  */
 
 #include <linux/fs.h>
+#include <linux/mount.h>
 #include <linux/slab.h>
 #include <linux/cred.h>
 #include <linux/xattr.h>
 #include <linux/posix_acl.h>
 #include "overlayfs.h"
+#include "ovl_entry.h"
+
+static dev_t ovl_get_pseudo_dev(struct dentry *dentry, dev_t dev)
+{
+	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+	int i;
+
+	if (ofs->upper_mnt && ofs->upper_mnt->mnt_sb->s_dev == dev)
+		return dev;
+
+	for (i = 0; i < ofs->numlower; i++) {
+		if (ofs->lower_mnt[i].real_dev == dev)
+			return ofs->lower_mnt[i].pseudo_dev;
+	}
+
+	return dev;
+}
 
 int ovl_setattr(struct dentry *dentry, struct iattr *attr)
 {

@@ -116,6 +134,8 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
 		 */
 		stat->dev = dentry->d_sb->s_dev;
 		stat->ino = dentry->d_inode->i_ino;
+	} else {
+		stat->dev = ovl_get_pseudo_dev(dentry, stat->dev);
 	}
 
 	/*

fs/overlayfs/ovl_entry.h +7 -1

index 34bc4a9..92aebe9 100644
@@ -16,11 +16,17 @@ struct ovl_config {
 	bool redirect_dir;
 };
 
+struct ovl_lower_mnt {
+	struct vfsmount *mnt;
+	dev_t real_dev;
+	dev_t pseudo_dev;
+};
+
 /* private information held for overlayfs's superblock */
 struct ovl_fs {
 	struct vfsmount *upper_mnt;
 	unsigned numlower;
-	struct vfsmount **lower_mnt;
+	struct ovl_lower_mnt *lower_mnt;
 	struct dentry *workdir;
 	long namelen;
 	/* pathnames of lower and upper dirs, for show_options */

fs/overlayfs/super.c +25 -8

index 4882ffb..c922a92 100644
@@ -172,8 +172,10 @@ static void ovl_put_super(struct super_block *sb)
 
 	dput(ufs->workdir);
 	mntput(ufs->upper_mnt);
-	for (i = 0; i < ufs->numlower; i++)
-		mntput(ufs->lower_mnt[i]);
+	for (i = 0; i < ufs->numlower; i++) {
+		mntput(ufs->lower_mnt[i].mnt);
+		free_anon_bdev(ufs->lower_mnt[i].pseudo_dev);
+	}
 	kfree(ufs->lower_mnt);
 
 	kfree(ufs->config.lowerdir);

@@ -908,15 +910,25 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	}
 
 	err = -ENOMEM;
-	ufs->lower_mnt = kcalloc(numlower, sizeof(struct vfsmount *), GFP_KERNEL);
+	ufs->lower_mnt = kcalloc(numlower, sizeof(struct ovl_lower_mnt),
+				GFP_KERNEL);
 	if (ufs->lower_mnt == NULL)
 		goto out_put_workdir;
 	for (i = 0; i < numlower; i++) {
-		struct vfsmount *mnt = clone_private_mount(&stack[i]);
+		struct vfsmount *mnt;
+		dev_t dev;
 
+		err = get_anon_bdev(&dev);
+		if (err) {
+			pr_err("overlayfs: failed to get anonymous bdev for lowerpath\n");
+			goto out_put_lower_mnt;
+		}
+
+		mnt = clone_private_mount(&stack[i]);
 		err = PTR_ERR(mnt);
 		if (IS_ERR(mnt)) {
 			pr_err("overlayfs: failed to clone lowerpath\n");
+			free_anon_bdev(dev);
 			goto out_put_lower_mnt;
 		}
 		/*

@@ -925,7 +937,9 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		 */
 		mnt->mnt_flags |= MNT_READONLY | MNT_NOATIME;
 
-		ufs->lower_mnt[ufs->numlower] = mnt;
+		ufs->lower_mnt[ufs->numlower].mnt = mnt;
+		ufs->lower_mnt[ufs->numlower].real_dev = mnt->mnt_sb->s_dev;
+		ufs->lower_mnt[ufs->numlower].pseudo_dev = dev;
 		ufs->numlower++;
 
 		/* Check if all lower layers are on same sb */

@@ -980,7 +994,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	}
 	for (i = 0; i < numlower; i++) {
 		oe->lowerstack[i].dentry = stack[i].dentry;
-		oe->lowerstack[i].mnt = ufs->lower_mnt[i];
+		oe->lowerstack[i].mnt = ufs->lower_mnt[i].mnt;
 	}
 	kfree(stack);

@@ -999,8 +1013,11 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 out_put_cred:
 	put_cred(ufs->creator_cred);
 out_put_lower_mnt:
-	for (i = 0; i < ufs->numlower; i++)
-		mntput(ufs->lower_mnt[i]);
+	for (i = 0; i < ufs->numlower; i++) {
+		if (ufs->lower_mnt[i].mnt)
+			free_anon_bdev(ufs->lower_mnt[i].pseudo_dev);
+		mntput(ufs->lower_mnt[i].mnt);
+	}
 	kfree(ufs->lower_mnt);
 out_put_workdir:
 	dput(ufs->workdir);

Signature · 1 line

2.9.4

Amir Goldstein

maintainer Reviewed-by Tested-by on v2 1/1

2017-06-23 13:34 UTC #1 lore raw

Quoted 7 lines · Chandan Rajendra

For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.
Signed-off-by Chandan Rajendra <[email protected]>
---

Reviewed-by Amir Goldstein <[email protected]>

Tested-by Amir Goldstein <[email protected]>

Miklos,

I re-created the branch ovl-constino [1] on top of ovl-hardlinks and without the consistent dino patches.

Applied Chandan's patch and resolved conflicts with my patches. Then, applied my patch to relax constant st_ino for non-samefs on stat(2).

Mutilated unionmount-testsuite layers check [2] to get over the unexpected pseudo dev and now tests pass for non samefs including constant ino verification and persistent ino verification for non-dir.

Chandan,

If you can fix the mutilated unionmount-testsuite check_layer(), that would be nice.

Thanks,
Amir.

[1] https://github.com/amir73il/linux/commits/ovl-constino
[2] https://github.com/amir73il/unionmount-testsuite/commits/ovl-constino

Chandan Rajendra

author

2017-06-23 16:47 UTC #2 lore raw

Quoted 21 lines · IST Amir Goldstein

Quoted 7 lines · Chandan Rajendra
For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.
Signed-off-by Chandan Rajendra <[email protected]>
---
Reviewed-by Amir Goldstein <[email protected]>
Tested-by Amir Goldstein <[email protected]>
Miklos,
I re-created the branch ovl-constino [1] on top of ovl-hardlinks and without the consistent dino patches.
Applied Chandan's patch and resolved conflicts with my patches. Then, applied my patch to relax constant st_ino for non-samefs on stat(2).

Quoted 9 lines

Mutilated unionmount-testsuite layers check [2] to get over the unexpected pseudo dev and now tests pass for non samefs including constant ino verification and persistent ino verification for non-dir.
Chandan,
If you can fix the mutilated unionmount-testsuite check_layer(), that would be nice.

Sure, I will work on it right away. Thanks for your guidance.

Signature · 1 line

chandan

Chandan Rajendra

author

2017-06-27 07:31 UTC #3 lore raw

In reply to Amir Goldstein

Quoted 21 lines · IST Amir Goldstein

Quoted 7 lines · Chandan Rajendra
For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.
Signed-off-by Chandan Rajendra <[email protected]>
---
Reviewed-by Amir Goldstein <[email protected]>
Tested-by Amir Goldstein <[email protected]>
Miklos,
I re-created the branch ovl-constino [1] on top of ovl-hardlinks and without the consistent dino patches.
Applied Chandan's patch and resolved conflicts with my patches. Then, applied my patch to relax constant st_ino for non-samefs on stat(2).

Quoted 9 lines

Mutilated unionmount-testsuite layers check [2] to get over the unexpected pseudo dev and now tests pass for non samefs including constant ino verification and persistent ino verification for non-dir.
Chandan,
If you can fix the mutilated unionmount-testsuite check_layer(), that would be nice.

Amir,

In check_layer() we have the following,

# TODO: for non-samefs, check file dev matches pseudo dev # and that pseduo dev != real dev
raise TestError(name + ": File on unexpected layer")

"check file dev matches pseudo dev" ... To do this we would have to have the list of pseudo dev ids for each of the lowerdirs. One crude way of getting such a list would be to create files having names with a known prefix under each of the lowerdirs before mounting the overlayfs instance. After mounting the overlayfs instance we could do a stat(2) on $overlay_mount/tempfile[i] to get the pseudo dev ids.

These pseudo dev ids can then be used in check_layer().

Please let me know your views on this.

Signature · 1 line

chandan

Amir Goldstein

maintainer

2017-06-27 09:04 UTC #4 lore raw

Quoted 47 lines · Chandan Rajendra

Quoted 21 lines · IST Amir Goldstein
Quoted 7 lines · Chandan Rajendra
For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.
Signed-off-by Chandan Rajendra <[email protected]>
---
Reviewed-by Amir Goldstein <[email protected]>
Tested-by Amir Goldstein <[email protected]>
Miklos,
I re-created the branch ovl-constino [1] on top of ovl-hardlinks and without the consistent dino patches.
Applied Chandan's patch and resolved conflicts with my patches. Then, applied my patch to relax constant st_ino for non-samefs on stat(2).
Quoted 9 lines
Mutilated unionmount-testsuite layers check [2] to get over the unexpected pseudo dev and now tests pass for non samefs including constant ino verification and persistent ino verification for non-dir.
Chandan,
If you can fix the mutilated unionmount-testsuite check_layer(), that would be nice.
Amir,
In check_layer() we have the following,
# TODO: for non-samefs, check file dev matches pseudo dev # and that pseduo dev != real dev
raise TestError(name + ": File on unexpected layer")
"check file dev matches pseudo dev" ... To do this we would have to have the list of pseudo dev ids for each of the lowerdirs. One crude way of getting such a list would be to create files having names with a known prefix under each of the lowerdirs before mounting the overlayfs instance. After mounting the overlayfs instance we could do a stat(2) on $overlay_mount/tempfile[i] to get the pseudo dev ids.

Quoted 4 lines

These pseudo dev ids can then be used in check_layer().
Please let me know your views on this.

I think that "check file dev matches pseudo dev" part is probably not worth the effort.

Better concentrate on "pseduo dev != real dev" part. real dev is already recorded (note_lower_fs/note_upper_fs) so it should be easier. Please go over the check_layer tests and specifically the changes I've made to them. It's possible that the tests could be improved also for the samefs case (i.e. verify that in samefs st_dev is always overlay dev).

BTW, I just force push the ovl-constino unionmount branch to include an opt-in run option --verify.
Without this option, constant ino is not checked at all. This is so older kernels could still be tested with latests unionmount testsuite.
Please rebase.

Thanks,
Amir.

Chandan Rajendra

author

2017-06-30 10:58 UTC #5 lore raw

In reply to Chandan Rajendra

Quoted 26 lines · IST Chandan Rajendra

Quoted 25 lines · IST Amir Goldstein
Quoted 7 lines · Chandan Rajendra
For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.
Signed-off-by Chandan Rajendra <[email protected]>
---
Reviewed-by Amir Goldstein <[email protected]>
Tested-by Amir Goldstein <[email protected]>
Miklos,
I re-created the branch ovl-constino [1] on top of ovl-hardlinks and without the consistent dino patches.
Applied Chandan's patch and resolved conflicts with my patches. Then, applied my patch to relax constant st_ino for non-samefs on stat(2).
Mutilated unionmount-testsuite layers check [2] to get over the unexpected pseudo dev and now tests pass for non samefs including constant ino verification and persistent ino verification for non-dir.

Quoted 6 lines

Quoted 5 lines
Chandan,
If you can fix the mutilated unionmount-testsuite check_layer(), that would be nice.

Hi Amir,

In dentry.created(), we have

        self.__upper = on_upper or not inode or inode.filetype() == "d"

Do you happen to know why we mark the dentry as being present on the upperdir filesystem when "inode" evaluates to false?

Consider the case of /lowerdir/a/foo101 file being created in set_up.py. ctx.record_file() ends up creating a new 'dentry' object with no inode associated with it. Hence self.__upper is set to the value True. Isn't this incorrect since the file foo101 is actually being created on lowerdir?

I added the following new test scenario to check_layer() code,

elif dentry.on_upper() and not self.config().is_samefs() and dev != self.upper_fs():
raise TestError(name + ": Upperdir file has incorrect dev id")

In the above code snippet, dentry.on_upper() incorrectly evaluates to True and hence we end up raising an exception.

Maybe dentry.created should have the following statement instead,

     self.__upper = on_upper or (inode and inode.filetype() == "d")

Signature · 1 line

chandan

Amir Goldstein

maintainer

2017-06-30 14:50 UTC #6 lore raw

Quoted 34 lines · Chandan Rajendra

Quoted 26 lines · IST Chandan Rajendra
Quoted 25 lines · IST Amir Goldstein
Quoted 7 lines · Chandan Rajendra
For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.
Signed-off-by Chandan Rajendra <[email protected]>
---
Reviewed-by Amir Goldstein <[email protected]>
Tested-by Amir Goldstein <[email protected]>
Miklos,
I re-created the branch ovl-constino [1] on top of ovl-hardlinks and without the consistent dino patches.
Applied Chandan's patch and resolved conflicts with my patches. Then, applied my patch to relax constant st_ino for non-samefs on stat(2).
Mutilated unionmount-testsuite layers check [2] to get over the unexpected pseudo dev and now tests pass for non samefs including constant ino verification and persistent ino verification for non-dir.
Quoted 6 lines
Quoted 5 lines
Chandan,
If you can fix the mutilated unionmount-testsuite check_layer(), that would be nice.

Quoted 8 lines

Hi Amir,
In dentry.created(), we have
        self.__upper = on_upper or not inode or inode.filetype() == "d"
Do you happen to know why we mark the dentry as being present on the upperdir filesystem when "inode" evaluates to false?

I don't know why that was done this way, but IIUC, only way for inode to be None is by passing None as filetype to record_file.

Quoted 5 lines

Consider the case of /lowerdir/a/foo101 file being created in set_up.py. ctx.record_file() ends up creating a new 'dentry' object with no inode associated with it. Hence self.__upper is set to the value True. Isn't this incorrect since the file foo101 is actually being created on lowerdir?

IIUC, self.__upper will only be true for no_foo and no_dir, which are created with filetype None.

Quoted 6 lines

I added the following new test scenario to check_layer() code,
elif dentry.on_upper() and not self.config().is_samefs() and dev != self.upper_fs():
raise TestError(name + ": Upperdir file has incorrect dev id")

Quoted 6 lines

In the above code snippet, dentry.on_upper() incorrectly evaluates to True and hence we end up raising an exception.
Maybe dentry.created should have the following statement instead,
     self.__upper = on_upper or (inode and inode.filetype() == "d")

That looks correct to me.
That change will only make a difference for dentries that are dentry.is_negative(),
but this use case is already checked above in check_layer() anyway.

Thanks,
Amir.

Miklos Szeredi

maintainer

2017-07-14 09:38 UTC #7 lore raw

In reply to Chandan Rajendra

Quoted 36 lines · Chandan Rajendra

For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.
Signed-off-by Chandan Rajendra <[email protected]>
---
Changelog:
v1->v2: Drop code that provided unique st_dev across copy up.
 fs/overlayfs/inode.c     | 20 ++++++++++++++++++++
 fs/overlayfs/ovl_entry.h |  8 +++++++-
 fs/overlayfs/super.c     | 33 +++++++++++++++++++++++++--------
 3 files changed, 52 insertions(+), 9 deletions(-)
fs/overlayfs/inode.c +8 -0
index d613e2c..e30bdca 100644
@@ -8,11 +8,29 @@
  */
 
 #include <linux/fs.h>
+#include <linux/mount.h>
 #include <linux/slab.h>
 #include <linux/cred.h>
 #include <linux/xattr.h>
 #include <linux/posix_acl.h>
 #include "overlayfs.h"
+#include "ovl_entry.h"
+
+static dev_t ovl_get_pseudo_dev(struct dentry *dentry, dev_t dev)
+{
+       struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+       int i;
+

fs/overlayfs/inode.c line 33

Quoted 10 lines

fs/overlayfs/inode.c +10 -0

+       if (ofs->upper_mnt && ofs->upper_mnt->mnt_sb->s_dev == dev)
+               return dev;
+
+       for (i = 0; i < ofs->numlower; i++) {
+               if (ofs->lower_mnt[i].real_dev == dev)
+                       return ofs->lower_mnt[i].pseudo_dev;
+       }
+
+       return dev;
+}

This is a good way to test out the functionality. But there's a trivial way to optimize away the O(num of layers) from this function:
instead of using struct path for lower stack, use

struct ovl_lower {
        struct ovl_lower_mnt *layer;
        struct dentry *dentry;
};

That means one more dereference to get the vfsmount, but I don't think we need to care about that.

Quoted 17 lines

fs/overlayfs/inode.c +2 -0

 int ovl_setattr(struct dentry *dentry, struct iattr *attr)
 {

@@ -116,6 +134,8 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
                 */
                stat->dev = dentry->d_sb->s_dev;
                stat->ino = dentry->d_inode->i_ino;
+       } else {
+               stat->dev = ovl_get_pseudo_dev(dentry, stat->dev);
        }
 
        /*

fs/overlayfs/ovl_entry.h +0 -0

index 34bc4a9..92aebe9 100644

fs/overlayfs/ovl_entry.h struct ovl_config { · line 21

Quoted 6 lines

fs/overlayfs/ovl_entry.h +3 -0

@@ -16,11 +16,17 @@ struct ovl_config {
        bool redirect_dir;
 };
 
+struct ovl_lower_mnt {
+       struct vfsmount *mnt;
+       dev_t real_dev;

Why store the real_dev? It's stored in mnt->mnt_sb->s_dev, right?

Quoted 94 lines

fs/overlayfs/ovl_entry.h +4 -1

+       dev_t pseudo_dev;
+};
+
 /* private information held for overlayfs's superblock */
 struct ovl_fs {
        struct vfsmount *upper_mnt;
        unsigned numlower;
-       struct vfsmount **lower_mnt;
+       struct ovl_lower_mnt *lower_mnt;
        struct dentry *workdir;
        long namelen;
        /* pathnames of lower and upper dirs, for show_options */

fs/overlayfs/super.c +25 -9

index 4882ffb..c922a92 100644
@@ -172,8 +172,10 @@ static void ovl_put_super(struct super_block *sb)
 
        dput(ufs->workdir);
        mntput(ufs->upper_mnt);
-       for (i = 0; i < ufs->numlower; i++)
-               mntput(ufs->lower_mnt[i]);
+       for (i = 0; i < ufs->numlower; i++) {
+               mntput(ufs->lower_mnt[i].mnt);
+               free_anon_bdev(ufs->lower_mnt[i].pseudo_dev);
+       }
        kfree(ufs->lower_mnt);
 
        kfree(ufs->config.lowerdir);

@@ -908,15 +910,25 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
        }
 
        err = -ENOMEM;
-       ufs->lower_mnt = kcalloc(numlower, sizeof(struct vfsmount *), GFP_KERNEL);
+       ufs->lower_mnt = kcalloc(numlower, sizeof(struct ovl_lower_mnt),
+                               GFP_KERNEL);
        if (ufs->lower_mnt == NULL)
                goto out_put_workdir;
        for (i = 0; i < numlower; i++) {
-               struct vfsmount *mnt = clone_private_mount(&stack[i]);
+               struct vfsmount *mnt;
+               dev_t dev;
 
+               err = get_anon_bdev(&dev);
+               if (err) {
+                       pr_err("overlayfs: failed to get anonymous bdev for lowerpath\n");
+                       goto out_put_lower_mnt;
+               }
+
+               mnt = clone_private_mount(&stack[i]);
                err = PTR_ERR(mnt);
                if (IS_ERR(mnt)) {
                        pr_err("overlayfs: failed to clone lowerpath\n");
+                       free_anon_bdev(dev);
                        goto out_put_lower_mnt;
                }
                /*

@@ -925,7 +937,9 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
                 */
                mnt->mnt_flags |= MNT_READONLY | MNT_NOATIME;
 
-               ufs->lower_mnt[ufs->numlower] = mnt;
+               ufs->lower_mnt[ufs->numlower].mnt = mnt;
+               ufs->lower_mnt[ufs->numlower].real_dev = mnt->mnt_sb->s_dev;
+               ufs->lower_mnt[ufs->numlower].pseudo_dev = dev;
                ufs->numlower++;
 
                /* Check if all lower layers are on same sb */

@@ -980,7 +994,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
        }
        for (i = 0; i < numlower; i++) {
                oe->lowerstack[i].dentry = stack[i].dentry;
-               oe->lowerstack[i].mnt = ufs->lower_mnt[i];
+               oe->lowerstack[i].mnt = ufs->lower_mnt[i].mnt;
        }
        kfree(stack);

@@ -999,8 +1013,11 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 out_put_cred:
        put_cred(ufs->creator_cred);
 out_put_lower_mnt:
-       for (i = 0; i < ufs->numlower; i++)
-               mntput(ufs->lower_mnt[i]);
+       for (i = 0; i < ufs->numlower; i++) {
+               if (ufs->lower_mnt[i].mnt)
+                       free_anon_bdev(ufs->lower_mnt[i].pseudo_dev);
+               mntput(ufs->lower_mnt[i].mnt);
+       }
        kfree(ufs->lower_mnt);
 out_put_workdir:
        dput(ufs->workdir);
--

2.9.4

Chandan Rajendra

author

2017-07-15 14:27 UTC #8 lore raw

Quoted 53 lines · IST Miklos Szeredi

Quoted 36 lines · Chandan Rajendra
For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.
Signed-off-by Chandan Rajendra <[email protected]>
---
Changelog:
v1->v2: Drop code that provided unique st_dev across copy up.
 fs/overlayfs/inode.c     | 20 ++++++++++++++++++++
 fs/overlayfs/ovl_entry.h |  8 +++++++-
 fs/overlayfs/super.c     | 33 +++++++++++++++++++++++++--------
 3 files changed, 52 insertions(+), 9 deletions(-)
fs/overlayfs/inode.c +8 -0
index d613e2c..e30bdca 100644
@@ -8,11 +8,29 @@
  */
 
 #include <linux/fs.h>
+#include <linux/mount.h>
 #include <linux/slab.h>
 #include <linux/cred.h>
 #include <linux/xattr.h>
 #include <linux/posix_acl.h>
 #include "overlayfs.h"
+#include "ovl_entry.h"
+
+static dev_t ovl_get_pseudo_dev(struct dentry *dentry, dev_t dev)
+{
+       struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+       int i;
+
fs/overlayfs/inode.c line 33
Quoted 10 lines
fs/overlayfs/inode.c +10 -0
+       if (ofs->upper_mnt && ofs->upper_mnt->mnt_sb->s_dev == dev)
+               return dev;
+
+       for (i = 0; i < ofs->numlower; i++) {
+               if (ofs->lower_mnt[i].real_dev == dev)
+                       return ofs->lower_mnt[i].pseudo_dev;
+       }
+
+       return dev;
+}
This is a good way to test out the functionality. But there's a trivial way to optimize away the O(num of layers) from this function:
instead of using struct path for lower stack, use

Quoted 8 lines

struct ovl_lower {
        struct ovl_lower_mnt *layer;
        struct dentry *dentry;
};
That means one more dereference to get the vfsmount, but I don't think we need to care about that.

Hi Miklos,

IIUC, 'struct ovl_entry' would then be,

/* private information held for every overlayfs dentry */

struct ovl_entry {
        union {
                struct {
                        unsigned long has_upper;
                        bool opaque;
                };
                struct rcu_head rcu;
        };
        unsigned numlower;
        struct ovl_lower lowerstack[];
};

... ovl_path_lower() would then be having a "struct ovl_lower *" as its second argument.

With the above listed changes, ovl_get_pseudo_dev() can then be fed a "struct ovl_path_lower *" and hence, as you had indicated the 'for' loop can be replaced with a statement returning the value of ovl_lower->layer->pseudo_dev.

Quoted 20 lines

fs/overlayfs/inode.c +2 -0

 int ovl_setattr(struct dentry *dentry, struct iattr *attr)
 {

@@ -116,6 +134,8 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
                 */
                stat->dev = dentry->d_sb->s_dev;
                stat->ino = dentry->d_inode->i_ino;
+       } else {
+               stat->dev = ovl_get_pseudo_dev(dentry, stat->dev);
        }
 
        /*

fs/overlayfs/ovl_entry.h +0 -0

index 34bc4a9..92aebe9 100644
@@ -16,11 +16,17 @@ struct ovl_config {
        bool redirect_dir;
 };

Quoted 5 lines

fs/overlayfs/ovl_entry.h struct ovl_config { · line 21
Quoted 3 lines
fs/overlayfs/ovl_entry.h +3 -0
+struct ovl_lower_mnt {
+       struct vfsmount *mnt;
+       dev_t real_dev;
Why store the real_dev? It's stored in mnt->mnt_sb->s_dev, right?

You are right. I will fix it up in the next version of this patch.

Thanks for your review comments.

Quoted 4 lines

Quoted 4 lines
fs/overlayfs/ovl_entry.h +3 -0
+       dev_t pseudo_dev;
+};
+

Signature · 1 line

chandan

Chandan Rajendra

author

2017-07-24 09:17 UTC #9 lore raw

PATCH v3 1/1 ovl: Allocate anonymous devs for lowerdirs

In reply to Miklos Szeredi

Signed-off-by Chandan Rajendra <[email protected]>

---
Changelog:

v2->v3: Optimize away O(n) search for pseudo dev in ovl_get_pseudo_dev().

v1->v2: Drop code that provided unique st_dev across copy up.

 fs/overlayfs/copy_up.c   |  1 -
 fs/overlayfs/inode.c     | 18 +++++++++++++
 fs/overlayfs/namei.c     | 37 +++++++++++++-------------
 fs/overlayfs/overlayfs.h |  5 ++--
 fs/overlayfs/ovl_entry.h | 14 ++++++++--
 fs/overlayfs/readdir.c   |  4 +--
 fs/overlayfs/super.c     | 69 ++++++++++++++++++++++++++++++------------------
 fs/overlayfs/util.c      |  8 ++++--
 8 files changed, 102 insertions(+), 54 deletions(-)

fs/overlayfs/copy_up.c +0 -1

index f193976..fc27f92 100644
@@ -22,7 +22,6 @@
 #include <linux/ratelimit.h>
 #include <linux/exportfs.h>
 #include "overlayfs.h"
-#include "ovl_entry.h"
 
 #define OVL_COPY_UP_CHUNK_SIZE (1 << 20)

fs/overlayfs/inode.c +18 -0

index 69f4fc2..8e81aa7 100644
@@ -8,6 +8,7 @@
  */
 
 #include <linux/fs.h>
+#include <linux/mount.h>
 #include <linux/slab.h>
 #include <linux/cred.h>
 #include <linux/xattr.h>

@@ -15,6 +16,21 @@
 #include <linux/ratelimit.h>
 #include "overlayfs.h"
 
+
+static dev_t ovl_get_pseudo_dev(struct dentry *dentry, dev_t dev)
+{
+	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+	struct ovl_entry *oe = dentry->d_fsdata;
+
+	if (ofs->upper_mnt && ofs->upper_mnt->mnt_sb->s_dev == dev)
+		return dev;
+
+	if (oe->numlower)
+		return oe->lowerstack[0].layer->pseudo_dev;
+
+	return dev;
+}
+
 int ovl_setattr(struct dentry *dentry, struct iattr *attr)
 {
 	int err;

@@ -121,6 +137,8 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
 		 */
 		stat->dev = dentry->d_sb->s_dev;
 		stat->ino = dentry->d_inode->i_ino;
+	} else {
+		stat->dev = ovl_get_pseudo_dev(dentry, stat->dev);
 	}
 
 	/*

fs/overlayfs/namei.c +18 ~~-19~~

index 9bc0e58..c286719 100644
@@ -15,7 +15,6 @@
 #include <linux/mount.h>
 #include <linux/exportfs.h>
 #include "overlayfs.h"
-#include "ovl_entry.h"
 
 struct ovl_lookup_data {
 	struct qstr name;

@@ -286,16 +285,15 @@ static int ovl_lookup_layer(struct dentry *base, struct ovl_lookup_data *d,
 
 
 static int ovl_check_origin(struct dentry *upperdentry,
-			    struct path *lowerstack, unsigned int numlower,
-			    struct path **stackp, unsigned int *ctrp)
+			    struct ovl_lower *lower, unsigned int numlower,
+			    struct ovl_lower **stackp, unsigned int *ctrp)
 {
 	struct vfsmount *mnt;
 	struct dentry *origin = NULL;
 	int i;
 
-
 	for (i = 0; i < numlower; i++) {
-		mnt = lowerstack[i].mnt;
+		mnt = lower[i].layer->mnt;
 		origin = ovl_get_origin(upperdentry, mnt);
 		if (IS_ERR(origin))
 			return PTR_ERR(origin);

@@ -309,12 +307,12 @@ static int ovl_check_origin(struct dentry *upperdentry,
 
 	BUG_ON(*ctrp);
 	if (!*stackp)
-		*stackp = kmalloc(sizeof(struct path), GFP_TEMPORARY);
+		*stackp = kmalloc(sizeof(struct ovl_lower), GFP_TEMPORARY);
 	if (!*stackp) {
 		dput(origin);
 		return -ENOMEM;
 	}
-	**stackp = (struct path) { .dentry = origin, .mnt = mnt };
+	**stackp = (struct ovl_lower) { .dentry = origin, .layer = lower[i].layer };
 	*ctrp = 1;
 
 	return 0;

@@ -384,13 +382,13 @@ int ovl_verify_origin(struct dentry *dentry, struct vfsmount *mnt,
  * OVL_XATTR_ORIGIN and that origin file handle can be decoded to lower path.
  * Return 0 on match, -ESTALE on mismatch or stale origin, < 0 on error.
  */
-int ovl_verify_index(struct dentry *index, struct path *lowerstack,
+int ovl_verify_index(struct dentry *index, struct ovl_lower *lower,
 		     unsigned int numlower)
 {
 	struct ovl_fh *fh = NULL;
 	size_t len;
-	struct path origin = { };
-	struct path *stack = &origin;
+	struct ovl_lower origin = { };
+	struct ovl_lower *stack = &origin;
 	unsigned int ctr = 0;
 	int err;

@@ -419,7 +417,7 @@ int ovl_verify_index(struct dentry *index, struct path *lowerstack,
 	if (err)
 		goto fail;
 
-	err = ovl_check_origin(index, lowerstack, numlower, &stack, &ctr);
+	err = ovl_check_origin(index, lower, numlower, &stack, &ctr);
 	if (!err && !ctr)
 		err = -ESTALE;
 	if (err)

@@ -545,7 +543,8 @@ int ovl_path_next(int idx, struct dentry *dentry, struct path *path)
 		idx++;
 	}
 	BUG_ON(idx > oe->numlower);
-	*path = oe->lowerstack[idx - 1];
+	path->dentry = oe->lowerstack[idx - 1].dentry;
+	path->mnt = oe->lowerstack[idx - 1].layer->mnt;
 
 	return (idx < oe->numlower) ? idx + 1 : -1;
 }

@@ -558,7 +557,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	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 ovl_lower *stack = NULL;
 	struct dentry *upperdir, *upperdentry = NULL;
 	struct dentry *index = NULL;
 	unsigned int ctr = 0;

@@ -622,17 +621,17 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 
 	if (!d.stop && poe->numlower) {
 		err = -ENOMEM;
-		stack = kcalloc(ofs->numlower, sizeof(struct path),
+		stack = kcalloc(ofs->numlower, sizeof(struct ovl_lower),
 				GFP_TEMPORARY);
 		if (!stack)
 			goto out_put_upper;
 	}
 
 	for (i = 0; !d.stop && i < poe->numlower; i++) {
-		struct path lowerpath = poe->lowerstack[i];
+		struct ovl_lower lower = poe->lowerstack[i];
 
 		d.last = i == poe->numlower - 1;
-		err = ovl_lookup_layer(lowerpath.dentry, &d, &this);
+		err = ovl_lookup_layer(lower.dentry, &d, &this);
 		if (err)
 			goto out_put;

@@ -640,7 +639,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			continue;
 
 		stack[ctr].dentry = this;
-		stack[ctr].mnt = lowerpath.mnt;
+		stack[ctr].layer = lower.layer;
 		ctr++;
 
 		if (d.stop)

@@ -651,7 +650,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 
 			/* Find the current layer on the root dentry */
 			for (i = 0; i < poe->numlower; i++)
-				if (poe->lowerstack[i].mnt == lowerpath.mnt)
+				if (poe->lowerstack[i].layer->mnt == lower.layer->mnt)
 					break;
 			if (WARN_ON(i == poe->numlower))
 				break;

@@ -676,7 +675,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		goto out_put;
 
 	oe->opaque = upperopaque;
-	memcpy(oe->lowerstack, stack, sizeof(struct path) * ctr);
+	memcpy(oe->lowerstack, stack, sizeof(struct ovl_lower) * ctr);
 	dentry->d_fsdata = oe;
 
 	if (upperdentry)

fs/overlayfs/overlayfs.h +3 -2

index 60d2660..76856dc 100644
@@ -9,6 +9,7 @@
 
 #include <linux/kernel.h>
 #include <linux/uuid.h>
+#include "ovl_entry.h"
 
 enum ovl_path_type {
 	__OVL_PATH_UPPER	= (1 << 0),

@@ -242,7 +243,7 @@ static inline bool ovl_is_impuredir(struct dentry *dentry)
 /* namei.c */
 int ovl_verify_origin(struct dentry *dentry, struct vfsmount *mnt,
 		      struct dentry *origin, bool is_upper, bool set);
-int ovl_verify_index(struct dentry *index, struct path *lowerstack,
+int ovl_verify_index(struct dentry *index, struct ovl_lower *lower,
 		     unsigned int numlower);
 int ovl_get_index_name(struct dentry *origin, struct qstr *name);
 int ovl_path_next(int idx, struct dentry *dentry, struct path *path);

@@ -258,7 +259,7 @@ int ovl_check_d_type_supported(struct path *realpath);
 void ovl_workdir_cleanup(struct inode *dir, struct vfsmount *mnt,
 			 struct dentry *dentry, int level);
 int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
-			 struct path *lowerstack, unsigned int numlower);
+			 struct ovl_lower *lower, unsigned int numlower);
 
 /* inode.c */
 int ovl_set_nlink_upper(struct dentry *dentry);

fs/overlayfs/ovl_entry.h +12 -2

index 878a750..c2d9322 100644
@@ -17,11 +17,21 @@ struct ovl_config {
 	bool index;
 };
 
+struct ovl_lower_mnt {
+	struct vfsmount *mnt;
+	dev_t pseudo_dev;
+};
+
+struct ovl_lower {
+	struct ovl_lower_mnt *layer;
+	struct dentry *dentry;
+};
+
 /* private information held for overlayfs's superblock */
 struct ovl_fs {
 	struct vfsmount *upper_mnt;
 	unsigned numlower;
-	struct vfsmount **lower_mnt;
+	struct ovl_lower_mnt *lower_mnt;
 	/* workbasedir is the path at workdir= mount option */
 	struct dentry *workbasedir;
 	/* workdir is the 'work' directory under workbasedir */

@@ -49,7 +59,7 @@ struct ovl_entry {
 		struct rcu_head rcu;
 	};
 	unsigned numlower;
-	struct path lowerstack[];
+	struct ovl_lower lowerstack[];
 };
 
 struct ovl_entry *ovl_alloc_entry(unsigned int numlower);

fs/overlayfs/readdir.c +2 -2

index 0298463..f286c32 100644
@@ -669,7 +669,7 @@ void ovl_workdir_cleanup(struct inode *dir, struct vfsmount *mnt,
 }
 
 int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
-			 struct path *lowerstack, unsigned int numlower)
+			 struct ovl_lower *lower, unsigned int numlower)
 {
 	int err;
 	struct inode *dir = dentry->d_inode;

@@ -703,7 +703,7 @@ int ovl_indexdir_cleanup(struct dentry *dentry, struct vfsmount *mnt,
 			err = PTR_ERR(index);
 			break;
 		}
-		if (ovl_verify_index(index, lowerstack, numlower)) {
+		if (ovl_verify_index(index, lower, numlower)) {
 			err = ovl_cleanup(dir, index);
 			if (err)
 				break;

fs/overlayfs/super.c +43 ~~-26~~

155 lines not on this page · raw message

fs/overlayfs/util.c +6 -2

index c492ba7..b16aade 100644
@@ -17,7 +17,6 @@
 #include <linux/namei.h>
 #include <linux/ratelimit.h>
 #include "overlayfs.h"
-#include "ovl_entry.h"
 
 int ovl_want_write(struct dentry *dentry)
 {

@@ -125,7 +124,12 @@ void ovl_path_lower(struct dentry *dentry, struct path *path)
 {
 	struct ovl_entry *oe = dentry->d_fsdata;
 
-	*path = oe->numlower ? oe->lowerstack[0] : (struct path) { };
+	if (oe->numlower) {
+		path->mnt = oe->lowerstack[0].layer->mnt;
+		path->dentry = oe->lowerstack[0].dentry;
+	} else {
+		*path = (struct path) { };
+	}
 }
 
 enum ovl_path_type ovl_path_real(struct dentry *dentry, struct path *path)

Signature · 1 line

2.9.3

Chandan Rajendra

author

2017-07-27 06:23 UTC #10 lore raw

Quoted 5 lines · IST Chandan Rajendra

For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.

Quoted 8 lines

Signed-off-by Chandan Rajendra <[email protected]>
---
Changelog:
v2->v3: Optimize away O(n) search for pseudo dev in ovl_get_pseudo_dev().
v1->v2: Drop code that provided unique st_dev across copy up.

I have rebased Amir's "ovl: relax same fs constrain for constant st_ino" patch on top of the changes made in this patch. The tree containing the rebased patch can be found at "https://github.com/chandanr/linux.git rework-anonymous-dev"

I have tested this by executing unionmount-testsuite's "./run --ov --verify".

Signature · 1 line

chandan

Amir Goldstein

maintainer

2017-09-21 17:46 UTC #11 lore raw

Quoted 15 lines · Chandan Rajendra

Quoted 5 lines · IST Chandan Rajendra
For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.
Quoted 8 lines
Signed-off-by Chandan Rajendra <[email protected]>
---
Changelog:
v2->v3: Optimize away O(n) search for pseudo dev in ovl_get_pseudo_dev().
v1->v2: Drop code that provided unique st_dev across copy up.

Quoted 7 lines

I have rebased Amir's "ovl: relax same fs constrain for constant st_ino" patch on top of the changes made in this patch. The tree containing the rebased patch can be found at "https://github.com/chandanr/linux.git rework-anonymous-dev"
I have tested this by executing unionmount-testsuite's "./run --ov --verify".

Chandan,

Can you rebase your work on v4.14-rc1?

You now need to add another patch to relax same fs constrain for constant d_ino in readdir.
Note that you will also need to relax in ovl_cache_update_ino() WARN_ON_ONCE(dir->d_sb->s_dev != stat.dev)

unionmount-testsuite's --verify (non-samefs) does not verify d_ino and the your constant d_ino xfstest does not cover the non-samefs case. A new xfstest, a simple variant of your samefs xfstest should be sufficient.

Cheers,
Amir.

Chandan Rajendra

author

2017-09-22 02:40 UTC #12 lore raw

Quoted 30 lines · IST Amir Goldstein

Quoted 15 lines · Chandan Rajendra
Quoted 5 lines · IST Chandan Rajendra
For stat(2) on lowerdir non-dir entries in non-samefs case, this commit provides unique values for st_dev. The unique values are obtained by allocating anonymous bdevs for each of the lowerdirs in the overlayfs instance.
Quoted 8 lines
Signed-off-by Chandan Rajendra <[email protected]>
---
Changelog:
v2->v3: Optimize away O(n) search for pseudo dev in ovl_get_pseudo_dev().
v1->v2: Drop code that provided unique st_dev across copy up.
Quoted 7 lines
I have rebased Amir's "ovl: relax same fs constrain for constant st_ino" patch on top of the changes made in this patch. The tree containing the rebased patch can be found at "https://github.com/chandanr/linux.git rework-anonymous-dev"
I have tested this by executing unionmount-testsuite's "./run --ov --verify".
Chandan,
Can you rebase your work on v4.14-rc1?

Quoted 9 lines

You now need to add another patch to relax same fs constrain for constant d_ino in readdir.
Note that you will also need to relax in ovl_cache_update_ino() WARN_ON_ONCE(dir->d_sb->s_dev != stat.dev)
unionmount-testsuite's --verify (non-samefs) does not verify d_ino and the your constant d_ino xfstest does not cover the non-samefs case. A new xfstest, a simple variant of your samefs xfstest should be sufficient.

Sure, I will do that and post the patches soon.

Signature · 1 line

chandan