linux-kernel - [PATCH 7/9] staging: lustre: ldlm: tidy list walking in ldlm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <150872002980.3340.10618324564169458593.stgit@noble>
Date:   Mon, 23 Oct 2017 11:53:49 +1100
From:   NeilBrown <neilb@...e.com>
To:     Oleg Drokin <oleg.drokin@...el.com>,
        James Simmons <jsimmons@...radead.org>,
        Andreas Dilger <andreas.dilger@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     lustre-devel@...ts.lustre.org, linux-kernel@...r.kernel.org
Subject: [PATCH 7/9] staging: lustre: 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.

Signed-off-by: NeilBrown <neilb@...e.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_flock.c |   45 ++++++++++-------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c
index 1bf56892fcf5..0bf6dce1c5b1 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c
@@ -59,17 +59,6 @@
 #include <linux/list.h>
 #include "ldlm_internal.h"
 
-/**
- * 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)
 {
@@ -125,8 +114,8 @@ static int ldlm_process_flock_lock(struct ldlm_lock *req)
 {
 	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;
@@ -151,23 +140,23 @@ static int ldlm_process_flock_lock(struct ldlm_lock *req)
 	/* 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;
 		}
 	}
 
-	/* 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;
@@ -295,7 +284,7 @@ static int ldlm_process_flock_lock(struct ldlm_lock *req)
 						 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;
 	}
@@ -309,8 +298,12 @@ static int ldlm_process_flock_lock(struct ldlm_lock *req)
 
 	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);
 	}
 
 	/* In case we're reprocessing the requested lock we can't destroy