Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1690814 > unrolled thread
| Started by | NeilBrown <neilb@suse.com> |
|---|---|
| First post | 2017-07-19 01:30 +0200 |
| Last post | 2017-07-19 06:40 +0200 |
| Articles | 2 — 1 participant |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 10/12] staging: lustre: ldlm: tidy list walking in ldlm_flock() NeilBrown <neilb@suse.com> - 2017-07-19 01:30 +0200
Re: [lustre-devel] [PATCH 10/12] staging: lustre: ldlm: tidy list walking in ldlm_flock() NeilBrown <neilb@suse.com> - 2017-07-19 06:40 +0200
| From | NeilBrown <neilb@suse.com> |
|---|---|
| Date | 2017-07-19 01:30 +0200 |
| Subject | [PATCH 10/12] staging: lustre: ldlm: tidy list walking in ldlm_flock() |
| Message-ID | <u4K6e-ns-11@gated-at.bofh.it> |
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@suse.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 9a888e1ce923..58227728a002 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, order 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
[toc] | [next] | [standalone]
| From | NeilBrown <neilb@suse.com> |
|---|---|
| Date | 2017-07-19 06:40 +0200 |
| Subject | Re: [lustre-devel] [PATCH 10/12] staging: lustre: ldlm: tidy list walking in ldlm_flock() |
| Message-ID | <u4OWd-3rS-3@gated-at.bofh.it> |
| In reply to | #1690814 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Jul 19 2017, Patrick Farrell wrote:
> Neil,
>
> Minor...
> "order might not be a lock" looks like it should say "or"?
Yes: s/order/or/ as you say.
Thanks,
NeilBrown
>
> - Patrick
> ________________________________
> From: lustre-devel <lustre-devel-bounces@lists.lustre.org> on behalf of NeilBrown <neilb@suse.com>
> Sent: Tuesday, July 18, 2017 6:26:47 PM
> To: Oleg Drokin; Greg Kroah-Hartman; Andreas Dilger
> Cc: Linux Kernel Mailing List; Lustre Development List
> Subject: [lustre-devel] [PATCH 10/12] 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@suse.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 9a888e1ce923..58227728a002 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, order 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
>
>
> _______________________________________________
> lustre-devel mailing list
> lustre-devel@lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web