From: Bobi Jam Date: Wed, 12 Jul 2023 15:05:27 +0000 (+0800) Subject: LU-16958 llite: migrate vs regular ops deadlock X-Git-Tag: 2.15.57~5 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=refs%2Fchanges%2F41%2F51641%2F2;p=fs%2Flustre-release.git 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 Change-Id: I7ee58039a6d31daefc625ac571a52baf112f8151 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/51641 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Alex Zhuravlev Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin --- diff --git a/lustre/include/cl_object.h b/lustre/include/cl_object.h index c09b31a..3bc3afa 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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);