Messages in this thread | | | From | "Dilger, Andreas" <> | Subject | Re: [PATCH 7/9] staging: lustre: ldlm: tidy list walking in ldlm_flock() | Date | Fri, 27 Oct 2017 09:32:49 +0000 |
| |
On Oct 22, 2017, at 18:53, NeilBrown <neilb@suse.com> wrote: > > 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>
The conversion looks a bit tricky, but appears to be correct.
Reviewed-by: Andreas Dilger <andreas.dilger@intel.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 > >
Cheers, Andreas -- Andreas Dilger Lustre Principal Architect Intel Corporation
| |