LU-16958 llite: migrate vs regular ops deadlock

When it need to lock inode in lov_conf_set(), it could have hold
inode's lli_layout_mutex, we need unlock the layout mutex before
taking its inode lock to keep the lock order.

Fixes: 51d62f2122f ("LU-16637 llite: call truncate_inode_pages() in inode lock")
Signed-off-by: Bobi Jam <bobijam@whamcloud.com>
Change-Id: I7ee58039a6d31daefc625ac571a52baf112f8151
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/51641
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Alex Zhuravlev <bzzz@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/include/cl_object.h b/lustre/include/cl_object.h
index c09b31a..3bc3afa 100644 (file)
--- a/lustre/include/cl_object.h
+++ b/lustre/include/cl_object.h
@@ -305,6 +305,8 @@ enum coo_inode_opc {
        COIO_INODE_UNLOCK,
        COIO_SIZE_LOCK,
        COIO_SIZE_UNLOCK,
+       COIO_LAYOUT_LOCK,
+       COIO_LAYOUT_UNLOCK,
 };
 
 /**

diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index 8004643..81bffd9 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -6283,6 +6283,7 @@ int ll_layout_refresh(struct inode *inode, __u32 *gen)
 
        /* take layout lock mutex to enqueue layout lock exclusively. */
        mutex_lock(&lli->lli_layout_mutex);
+       lli->lli_layout_lock_owner = current;
 
        while (1) {
                /* mostly layout lock is caching on the local side, so try to
@@ -6304,6 +6305,7 @@ int ll_layout_refresh(struct inode *inode, __u32 *gen)
 
        if (rc == 0)
                *gen = ll_layout_version_get(lli);
+       lli->lli_layout_lock_owner = NULL;
        mutex_unlock(&lli->lli_layout_mutex);
 
        RETURN(rc);

diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h
index dd0f989..01d4bb8 100644 (file)
--- a/lustre/llite/llite_internal.h
+++ b/lustre/llite/llite_internal.h
@@ -281,6 +281,7 @@ struct ll_inode_info {
 
        /* mutex to request for layout lock exclusively. */
        struct mutex                    lli_layout_mutex;
+       struct task_struct              *lli_layout_lock_owner;
        /* Layout version, protected by lli_layout_lock */
        __u32                           lli_layout_gen;
        spinlock_t                      lli_layout_lock;

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index 5da7d20..ed41b20 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -1274,6 +1274,7 @@ void ll_lli_init(struct ll_inode_info *lli)
                lli->lli_gid = (__u32) -1;
        }
        mutex_init(&lli->lli_layout_mutex);
+       lli->lli_layout_lock_owner = NULL;
        /* ll_cl_context initialize */
        INIT_LIST_HEAD(&lli->lli_lccs);
        seqlock_init(&lli->lli_page_inv_lock);

diff --git a/lustre/llite/vvp_object.c b/lustre/llite/vvp_object.c
index 32bb6d9..be8ad1b 100644 (file)
--- a/lustre/llite/vvp_object.c
+++ b/lustre/llite/vvp_object.c
@@ -235,6 +235,7 @@ static int vvp_inode_ops(const struct lu_env *env, struct cl_object *obj,
                         enum coo_inode_opc opc, void *data)
 {
        struct inode *inode = vvp_object_inode(obj);
+       struct ll_inode_info *lli = ll_i2info(inode);
        int rc = 0;
 
        ENTRY;
@@ -252,17 +253,31 @@ static int vvp_inode_ops(const struct lu_env *env, struct cl_object *obj,
                        rc = -ENOLCK;
                break;
        case COIO_SIZE_LOCK:
-               if (ll_i2info(inode)->lli_size_lock_owner != current)
+               if (lli->lli_size_lock_owner != current)
                        ll_inode_size_lock(inode);
                else
                        rc = -EALREADY;
                break;
        case COIO_SIZE_UNLOCK:
-               if (ll_i2info(inode)->lli_size_lock_owner == current)
+               if (lli->lli_size_lock_owner == current)
                        ll_inode_size_unlock(inode);
                else
                        rc = -ENOLCK;
                break;
+       case COIO_LAYOUT_LOCK:
+               if (lli->lli_layout_lock_owner != current) {
+                       mutex_lock(&lli->lli_layout_mutex);
+                       lli->lli_layout_lock_owner = current;
+               }
+               break;
+       case COIO_LAYOUT_UNLOCK:
+               if (lli->lli_layout_lock_owner == current) {
+                       lli->lli_layout_lock_owner = NULL;
+                       mutex_unlock(&lli->lli_layout_mutex);
+               } else {
+                       rc = -ENOLCK;
+               }
+               break;
        default:
                rc = -EINVAL;
                break;

diff --git a/lustre/lov/lov_object.c b/lustre/lov/lov_object.c
index 16f4bf6..023c348 100644 (file)
--- a/lustre/lov/lov_object.c
+++ b/lustre/lov/lov_object.c
@@ -1369,6 +1369,7 @@ static int lov_conf_set(const struct lu_env *env, struct cl_object *obj,
        struct cl_object *top = cl_object_top(obj);
        bool unlock_inode = false;
        bool lock_inode_size = false;
+       bool lock_layout = false;
        ENTRY;
 
        if (conf->coc_opc == OBJECT_CONF_SET &&
@@ -1440,11 +1441,12 @@ retry:
                        /**
                         * we need unlocked lov conf and get inode lock.
                         * It's possible we have already taken inode's size
-                        * mutex, so we need keep such lock order, lest deadlock
-                        * happens:
-                        *   inode lock       (ll_inode_lock())
-                        *   inode size lock  (ll_inode_size_lock())
-                        *   lov conf lock    (lov_conf_lock())
+                        * mutex and/or layout mutex, so we need keep such lock
+                        * order, lest deadlock happens:
+                        *   inode lock        (ll_inode_lock())
+                        *   inode size lock   (ll_inode_size_lock())
+                        *   inode layout lock (ll_layout_refresh())
+                        *   lov conf lock     (lov_conf_lock())
                         *
                         * e.g.
                         *   vfs_setxattr                inode locked
@@ -1453,10 +1455,20 @@ retry:
                         *         ll_file_inode_init
                         *           cl_conf_set
                         *             lov_conf_set      lov conf locked
+                        *
+                        *   ll_migrate                  inode locked
+                        *     ...
+                        *       ll_layout_refresh       inode layout locked
+                        *         ll_layout_conf
+                        *           cl_conf_set
+                        *             lov_conf_set      lov conf locked
                         */
                        lov_conf_unlock(lov);
-                       if (cl_object_inode_ops(
-                                       env, top, COIO_SIZE_UNLOCK, NULL) == 0)
+                       if (cl_object_inode_ops(env, top, COIO_LAYOUT_UNLOCK,
+                                               NULL) == 0)
+                               lock_layout = true;
+                       if (cl_object_inode_ops(env, top, COIO_SIZE_UNLOCK,
+                                               NULL) == 0)
                                lock_inode_size = true;
 
                        /* take lock in order */
@@ -1464,8 +1476,11 @@ retry:
                                        env, top, COIO_INODE_LOCK, NULL) == 0)
                                unlock_inode = true;
                        if (lock_inode_size)
-                               cl_object_inode_ops(
-                                       env, top, COIO_SIZE_LOCK, NULL);
+                               cl_object_inode_ops(env, top, COIO_SIZE_LOCK,
+                                                   NULL);
+                       if (lock_layout)
+                               cl_object_inode_ops(env, top, COIO_LAYOUT_LOCK,
+                                                   NULL);
                        goto retry;
                }
                set_bit(LO_LAYOUT_INVALID, &lov->lo_obj_flags);