fs/dcache.c: __d_unalias: complicated control flow

[bulwahn]

fs/dcache.c:2896:2: warning: mutex 'dentry->d_sb->s_vfs_rename_mutex' is not held on every path through here [-Wthread-safety-analysis]
        __d_move(alias, dentry, false);
        ^
fs/dcache.c:2889:7: note: mutex acquired here
        if (!mutex_trylock(&dentry->d_sb->s_vfs_rename_mutex))
             ^
fs/dcache.c:2899:6: warning: mutex 'dentry->d_sb->s_vfs_rename_mutex' is not held on every path through here [-Wthread-safety-analysis]
        if (m2)
            ^
fs/dcache.c:2889:7: note: mutex acquired here
        if (!mutex_trylock(&dentry->d_sb->s_vfs_rename_mutex))
             ^
fs/dcache.c:2902:3: warning: releasing mutex 'm1' that was not held [-Wthread-safety-analysis]
                mutex_unlock(m1);
                ^

Suggestion: just rewrite this function to have a clearer control flow.

[bulwahn]

Rewriting this function makes the code understandable and then the warnings disappear as well:

diff --git a/fs/dcache.c b/fs/dcache.c
index aac41adf4743..59ffeaa69122 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2877,30 +2877,22 @@ struct dentry *d_ancestor(struct dentry *p1, struct dentry *p2)
 static int __d_unalias(struct inode *inode,
                struct dentry *dentry, struct dentry *alias)
 {
-       struct mutex *m1 = NULL;
-       struct rw_semaphore *m2 = NULL;
-       int ret = -ESTALE;
-
        /* If alias and dentry share a parent, then no extra locks required */
-       if (alias->d_parent == dentry->d_parent)
-               goto out_unalias;
-
+       if (alias->d_parent == dentry->d_parent) {
+               __d_move(alias, dentry, false);
+               return 0;
+       }
        /* See lock_rename() */
        if (!mutex_trylock(&dentry->d_sb->s_vfs_rename_mutex))
-               goto out_err;
-       m1 = &dentry->d_sb->s_vfs_rename_mutex;
-       if (!inode_trylock_shared(alias->d_parent->d_inode))
-               goto out_err;
-       m2 = &alias->d_parent->d_inode->i_rwsem;
-out_unalias:
+               return -ESTALE;
+       if (!inode_trylock_shared(alias->d_parent->d_inode)) {
+               mutex_unlock(&dentry->d_sb->s_vfs_rename_mutex);
+               return -ESTALE;
+       }
        __d_move(alias, dentry, false);
-       ret = 0;
-out_err:
-       if (m2)
-               up_read(m2);
-       if (m1)
-               mutex_unlock(m1);
-       return ret;
+       up_read(&alias->d_parent->d_inode->i_rwsem);
+       mutex_unlock(&dentry->d_sb->s_vfs_rename_mutex);
+       return 0;
 }

 /**