ovl: make room to store state flags in dentry

ovl_path_type() is called quite often (e.g.: on every file open) to calculate the path type flags of overlay dentry. Those flags are rarely changed after dentry instantiation and when changed, flags can only be added. (e.g.: on copyup, rename and create).

Store the type value in ovl_entry and update the flags when needed, so ovl_path_type() just returns the stored value.

The old ovl_path_type() took care of clearing OVL_TYPE_MERGE for a non-dir entry. It did so for a reason that seem obsolete. In any case, the code never checks OVL_TYPE_MERGE on non-dir entry, so merge flag for non-dir is harmless.

Signed-off-by Amir Goldstein <[email protected]>

---
 fs/overlayfs/namei.c     |  1 +
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/ovl_entry.h |  3 +++
 fs/overlayfs/super.c     |  1 +
 fs/overlayfs/util.c      | 20 ++++++++++++--------
 5 files changed, 18 insertions(+), 8 deletions(-)

fs/overlayfs/namei.c +1 -0

index 70a6b64..c230ee1 100644
@@ -481,6 +481,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
        kfree(stack);
        kfree(d.redirect);
        dentry->d_fsdata = oe;
+       ovl_update_type(dentry);
        d_add(dentry, inode);
 
        return NULL;

fs/overlayfs/overlayfs.h +1 -0

index 2ef53f0..d37ec4c 100644
@@ -168,6 +168,7 @@ struct ovl_entry *ovl_alloc_entry(unsigned int numlower);
 bool ovl_dentry_remote(struct dentry *dentry);
 bool ovl_dentry_weird(struct dentry *dentry);
 enum ovl_path_type ovl_path_type(struct dentry *dentry);
+enum ovl_path_type ovl_update_type(struct dentry *dentry);
 void ovl_path_upper(struct dentry *dentry, struct path *path);
 void ovl_path_lower(struct dentry *dentry, struct path *path);
 enum ovl_path_type ovl_path_real(struct dentry *dentry, struct path *path);

fs/overlayfs/ovl_entry.h +3 -0

index 85c886d..b07c29f 100644
@@ -33,6 +33,8 @@ struct ovl_fs {
        wait_queue_head_t copyup_wq;
 };
 
+enum ovl_path_type;
+
 /* private information held for every overlayfs dentry */
 struct ovl_entry {
        struct dentry *__upperdentry;

@@ -46,6 +48,7 @@ struct ovl_entry {
                };
                struct rcu_head rcu;
        };
+       enum ovl_path_type type;
        unsigned numlower;
        struct path lowerstack[];
 };

fs/overlayfs/super.c +1 -0

index f771366..bdbed27 100644
@@ -977,6 +977,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
        kfree(stack);
 
        root_dentry->d_fsdata = oe;
+       ovl_update_type(root_dentry);
 
        realinode = d_inode(ovl_dentry_real(root_dentry));
        ovl_inode_init(d_inode(root_dentry), realinode, !!upperpath.dentry);

fs/overlayfs/util.c +12 -9

index aaac1ac..f9693a8 100644
@@ -70,21 +70,24 @@ bool ovl_dentry_weird(struct dentry *dentry)
 enum ovl_path_type ovl_path_type(struct dentry *dentry)
 {
        struct ovl_entry *oe = dentry->d_fsdata;
-       enum ovl_path_type type = 0;
 
-       if (oe->__upperdentry) {
-               type = __OVL_PATH_UPPER;
+       return oe->type;
+}
+
+enum ovl_path_type ovl_update_type(struct dentry *dentry)
+{
+       struct ovl_entry *oe = dentry->d_fsdata;
+       enum ovl_path_type type = oe->type;
 
-               /*
-                * Non-dir dentry can hold lower dentry from previous
-                * location.
-                */
-               if (oe->numlower && d_is_dir(dentry))
+       if (oe->__upperdentry) {
+               type |= __OVL_PATH_UPPER;
+               if (oe->numlower)
                        type |= __OVL_PATH_MERGE;
        } else {
                if (oe->numlower > 1)
                        type |= __OVL_PATH_MERGE;
        }
+       oe->type = type;
        return type;
 }
 

@@ -248,6 +251,7 @@ void ovl_dentry_update(struct dentry *dentry, struct dentry *upperdentry)
         */
        smp_wmb();
        oe->__upperdentry = upperdentry;
+       ovl_update_type(dentry);
 }
 
 void ovl_inode_init(struct inode *inode, struct inode *realinode, bool is_upper)
--

2.7.4

Quoted 7 lines · Amir Goldstein

ovl_path_type() is called quite often (e.g.: on every file open) to calculate the path type flags of overlay dentry. Those flags are rarely changed after dentry instantiation and when changed, flags can only be added. (e.g.: on copyup, rename and create).

Store the type value in ovl_entry and update the flags when needed, so ovl_path_type() just returns the stored value.

Since accessing __upperdentry is lockless we need to be careful with memory ordering issues.

For example in the new version of ovl_dentry_update() we first store oe->__upperdentry then oe->type. In a concurrent ovl_path_real() we first read oe->type and then based on the OVL_TYPE_UPPER flag read oe->__upperdentry. Without any barriers the above could very easily result in NULL upperdentry being read. The compiler or the CPU could reorder the stores in ovl_dentry_update(), etc...

For the gory details see Documentation/memory-barriers.txt

Quoted 12 lines · Miklos Szeredi

Quoted 7 lines · Amir Goldstein

ovl_path_type() is called quite often (e.g.: on every file open) to calculate the path type flags of overlay dentry. Those flags are rarely changed after dentry instantiation and when changed, flags can only be added. (e.g.: on copyup, rename and create).

Store the type value in ovl_entry and update the flags when needed, so ovl_path_type() just returns the stored value.

Since accessing __upperdentry is lockless we need to be careful with memory ordering issues.

Quoted 7 lines

For example in the new version of ovl_dentry_update() we first store oe->__upperdentry then oe->type. In a concurrent ovl_path_real() we first read oe->type and then based on the OVL_TYPE_UPPER flag read oe->__upperdentry. Without any barriers the above could very easily result in NULL upperdentry being read. The compiler or the CPU could reorder the stores in ovl_dentry_update(), etc...

Right.. thanks for pointing that out. I'll take better care.

FYI, I am just in the midst of adding oe->__tempdentry and OVL_TYPE_RO_UPPER for copy up on O_RDONLY.
I started by trying to "upgrade" oe->__upperdentry from tmpfile to linked file, but that was real messy.

Quoted 2 lines

For the gory details see Documentation/memory-barriers.txt

I know about them, but still easy for me to get them wrong... Hopefully will have something for you to review by tomorrow.