From c0aa5d97da570ea5abca5fb7323c70a3f8fd3b24 Mon Sep 17 00:00:00 2001 From: "Mr. NeilBrown" Date: Sat, 10 Dec 2022 09:17:46 -0500 Subject: [PATCH] LU-6142 ldlm: tidy list walking in ldlm_flock() Use list_for_each_entry variants to avoid the explicit list_entry() calls. This allows us to use list_for_each_entry_safe_from() instread of adding a local list-walking macro. Also improve some comments so that it is more obvious that the locks are sorted per-owner and that we need to find the insertion point. Linux-commit: 3ac5a67 ("staging: lustre: ldlm: tidy list walking in ldlm_flock()") Change-Id: Ie9a756a898a9c58db1b4f446694603a4efa37352 Signed-off-by: Mr. NeilBrown Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/49358 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andriy Skulysh Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin --- lustre/ldlm/ldlm_flock.c | 55 +++++++++++++++++++----------------------------- 1 file changed, 22 insertions(+), 33 deletions(-) diff --git a/lustre/ldlm/ldlm_flock.c b/lustre/ldlm/ldlm_flock.c index 84e56af..770cf94 100644 --- a/lustre/ldlm/ldlm_flock.c +++ b/lustre/ldlm/ldlm_flock.c @@ -62,17 +62,6 @@ int ldlm_flock_blocking_ast(struct ldlm_lock *lock, struct ldlm_lock_desc *desc, void *data, int flag); -/** - * list_for_remaining_safe - iterate over the remaining entries in a list - * and safeguard against removal of a list entry. - * \param pos the &struct list_head to use as a loop counter. pos MUST - * have been initialized prior to using it in this macro. - * \param n another &struct list_head to use as temporary storage - * \param head the head for your list. - */ -#define list_for_remaining_safe(pos, n, head) \ - for (n = pos->next; pos != (head); pos = n, n = pos->next) - static inline int ldlm_same_flock_owner(struct ldlm_lock *lock, struct ldlm_lock *new) { @@ -287,8 +276,8 @@ ldlm_process_flock_lock(struct ldlm_lock *req, __u64 *flags, { struct ldlm_resource *res = req->l_resource; struct ldlm_namespace *ns = ldlm_res_to_ns(res); - struct list_head *tmp; - struct list_head *ownlocks = NULL; + struct ldlm_lock *tmp; + struct ldlm_lock *ownlocks = NULL; struct ldlm_lock *lock = NULL; struct ldlm_lock *new = req; struct ldlm_lock *new2 = NULL; @@ -301,8 +290,8 @@ ldlm_process_flock_lock(struct ldlm_lock *req, __u64 *flags, struct list_head *grant_work = (intention == LDLM_PROCESS_ENQUEUE ? NULL : work_list); #endif - ENTRY; + ENTRY; CDEBUG(D_DLMTRACE, "flags %#llx owner %llu pid %u mode %u start " "%llu end %llu\n", *flags, new->l_policy_data.l_flock.owner, @@ -327,11 +316,9 @@ reprocess: /* This loop determines where this processes locks start * in the resource lr_granted list. */ - list_for_each(tmp, &res->lr_granted) { - lock = list_entry(tmp, struct ldlm_lock, - l_res_link); + list_for_each_entry(lock, &res->lr_granted, l_res_link) { if (ldlm_same_flock_owner(lock, req)) { - ownlocks = tmp; + ownlocks = lock; break; } } @@ -344,13 +331,10 @@ reprocess: /* This loop determines if there are existing locks * that conflict with the new lock request. */ - list_for_each(tmp, &res->lr_granted) { - lock = list_entry(tmp, struct ldlm_lock, - l_res_link); - + list_for_each_entry(lock, &res->lr_granted, l_res_link) { if (ldlm_same_flock_owner(lock, req)) { if (!ownlocks) - ownlocks = tmp; + ownlocks = lock; continue; } @@ -432,15 +416,16 @@ reprocess: ldlm_flock_blocking_unlink(req); #endif /* HAVE_SERVER_SUPPORT */ - /* Scan the locks owned by this process that overlap this request. + /* Scan the locks owned by this process to find the insertion point + * (as locks are ordered), and to handle overlaps. * We may have to merge or split existing locks. */ - if (!ownlocks) - ownlocks = &res->lr_granted; - - list_for_remaining_safe(ownlocks, tmp, &res->lr_granted) { - lock = list_entry(ownlocks, struct ldlm_lock, l_res_link); - + if (ownlocks) + lock = ownlocks; + else + lock = list_entry(&res->lr_granted, + struct ldlm_lock, l_res_link); + list_for_each_entry_safe_from(lock, tmp, &res->lr_granted, l_res_link) { if (!ldlm_same_flock_owner(lock, new)) break; @@ -573,7 +558,7 @@ reprocess: lock->l_granted_mode); /* insert new2 at lock */ - ldlm_resource_add_lock(res, ownlocks, new2); + ldlm_resource_add_lock(res, &lock->l_res_link, new2); LDLM_LOCK_RELEASE(new2); break; } @@ -588,8 +573,12 @@ reprocess: /* Add req to the granted queue before calling ldlm_reprocess_all(). */ if (!added) { list_del_init(&req->l_res_link); - /* insert new lock before ownlocks in list. */ - ldlm_resource_add_lock(res, ownlocks, req); + /* insert new lock before "lock", which might be the + * next lock for this owner, or might be the first + * lock for the next owner, or might not be a lock at + * all, but instead points at the head of the list + */ + ldlm_resource_add_lock(res, &lock->l_res_link, req); } if (*flags != LDLM_FL_WAIT_NOREPROC) { -- 1.8.3.1