<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text --><style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
</head>
<body>
<div><br>
Neil,<br>
<br>
Minor...<br>
"order might not be a lock" looks like it should say "or"?<br>
<br>
- Patrick
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> lustre-devel <lustre-devel-bounces@lists.lustre.org> on behalf of NeilBrown <neilb@suse.com><br>
<b>Sent:</b> Tuesday, July 18, 2017 6:26:47 PM<br>
<b>To:</b> Oleg Drokin; Greg Kroah-Hartman; Andreas Dilger<br>
<b>Cc:</b> Linux Kernel Mailing List; Lustre Development List<br>
<b>Subject:</b> [lustre-devel] [PATCH 10/12] staging: lustre: ldlm: tidy list walking in ldlm_flock()</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt;">
<div class="PlainText">Use list_for_each_entry variants to<br>
avoid the explicit list_entry() calls.<br>
This allows us to use list_for_each_entry_safe_from()<br>
instread of adding a local list-walking macro.<br>
<br>
Also improve some comments so that it is more obvious<br>
that the locks are sorted per-owner and that we need<br>
to find the insertion point.<br>
<br>
Signed-off-by: NeilBrown <neilb@suse.com><br>
---<br>
drivers/staging/lustre/lustre/ldlm/ldlm_flock.c | 45 ++++++++++-------------<br>
1 file changed, 19 insertions(+), 26 deletions(-)<br>
<br>
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c<br>
index 9a888e1ce923..58227728a002 100644<br>
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c<br>
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c<br>
@@ -59,17 +59,6 @@<br>
#include <linux/list.h><br>
#include "ldlm_internal.h"<br>
<br>
-/**<br>
- * list_for_remaining_safe - iterate over the remaining entries in a list<br>
- * and safeguard against removal of a list entry.<br>
- * \param pos the &struct list_head to use as a loop counter. pos MUST<br>
- * have been initialized prior to using it in this macro.<br>
- * \param n another &struct list_head to use as temporary storage<br>
- * \param head the head for your list.<br>
- */<br>
-#define list_for_remaining_safe(pos, n, head) \<br>
- for (n = pos->next; pos != (head); pos = n, n = pos->next)<br>
-<br>
static inline int<br>
ldlm_same_flock_owner(struct ldlm_lock *lock, struct ldlm_lock *new)<br>
{<br>
@@ -125,8 +114,8 @@ static int ldlm_process_flock_lock(struct ldlm_lock *req)<br>
{<br>
struct ldlm_resource *res = req->l_resource;<br>
struct ldlm_namespace *ns = ldlm_res_to_ns(res);<br>
- struct list_head *tmp;<br>
- struct list_head *ownlocks = NULL;<br>
+ struct ldlm_lock *tmp;<br>
+ struct ldlm_lock *ownlocks = NULL;<br>
struct ldlm_lock *lock = NULL;<br>
struct ldlm_lock *new = req;<br>
struct ldlm_lock *new2 = NULL;<br>
@@ -151,23 +140,23 @@ static int ldlm_process_flock_lock(struct ldlm_lock *req)<br>
/* This loop determines where this processes locks start<br>
* in the resource lr_granted list.<br>
*/<br>
- list_for_each(tmp, &res->lr_granted) {<br>
- lock = list_entry(tmp, struct ldlm_lock,<br>
- l_res_link);<br>
+ list_for_each_entry(lock, &res->lr_granted, l_res_link) {<br>
if (ldlm_same_flock_owner(lock, req)) {<br>
- ownlocks = tmp;<br>
+ ownlocks = lock;<br>
break;<br>
}<br>
}<br>
<br>
- /* Scan the locks owned by this process that overlap this request.<br>
+ /* Scan the locks owned by this process to find the insertion point<br>
+ * (as locks are ordered), and to handle overlaps.<br>
* We may have to merge or split existing locks.<br>
*/<br>
- if (!ownlocks)<br>
- ownlocks = &res->lr_granted;<br>
-<br>
- list_for_remaining_safe(ownlocks, tmp, &res->lr_granted) {<br>
- lock = list_entry(ownlocks, struct ldlm_lock, l_res_link);<br>
+ if (ownlocks)<br>
+ lock = ownlocks;<br>
+ else<br>
+ lock = list_entry(&res->lr_granted,<br>
+ struct ldlm_lock, l_res_link);<br>
+ list_for_each_entry_safe_from(lock, tmp, &res->lr_granted, l_res_link) {<br>
<br>
if (!ldlm_same_flock_owner(lock, new))<br>
break;<br>
@@ -295,7 +284,7 @@ static int ldlm_process_flock_lock(struct ldlm_lock *req)<br>
lock->l_granted_mode);<br>
<br>
/* insert new2 at lock */<br>
- ldlm_resource_add_lock(res, ownlocks, new2);<br>
+ ldlm_resource_add_lock(res, &lock->l_res_link, new2);<br>
LDLM_LOCK_RELEASE(new2);<br>
break;<br>
}<br>
@@ -309,8 +298,12 @@ static int ldlm_process_flock_lock(struct ldlm_lock *req)<br>
<br>
if (!added) {<br>
list_del_init(&req->l_res_link);<br>
- /* insert new lock before ownlocks in list. */<br>
- ldlm_resource_add_lock(res, ownlocks, req);<br>
+ /* insert new lock before "lock", which might be<br>
+ * the next lock for this owner, or might be the first<br>
+ * lock for the next owner, order might not be a lock<br>
+ * at all, but instead points at the head of the list<br>
+ */<br>
+ ldlm_resource_add_lock(res, &lock->l_res_link, req);<br>
}<br>
<br>
/* In case we're reprocessing the requested lock we can't destroy<br>
<br>
<br>
_______________________________________________<br>
lustre-devel mailing list<br>
lustre-devel@lists.lustre.org<br>
<a href="http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org">http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org</a><br>
</div>
</span></font>
</body>
</html>