From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751976AbdGSEgT (ORCPT ); Wed, 19 Jul 2017 00:36:19 -0400 Received: from mx2.suse.de ([195.135.220.15]:43538 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751228AbdGSEgS (ORCPT ); Wed, 19 Jul 2017 00:36:18 -0400 From: NeilBrown To: Patrick Farrell , Oleg Drokin , Greg Kroah-Hartman , Andreas Dilger Date: Wed, 19 Jul 2017 14:36:07 +1000 Cc: Linux Kernel Mailing List , Lustre Development List Subject: Re: [lustre-devel] [PATCH 10/12] staging: lustre: ldlm: tidy list walking in ldlm_flock() In-Reply-To: References: <150041997277.20736.17112251996623587423.stgit@noble> <150042040763.20736.17969183285195201575.stgit@noble> Message-ID: <87lgnlxcug.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain 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 on behalf of NeilBrown > 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 > --- > 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 > #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 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEG8Yp69OQ2HB7X0l6Oeye3VZigbkFAllu4bgACgkQOeye3VZi gblLHQ/+NAUvnWPda5uLEfwstbFJ8e//EKS7nBo5jN4ASRJvpawGF62ve4EXCZvH LfY1SU8NgiTu8eOEqBDCForxh50xp0k2mv8Qf+rpFiDFeU/ysZp/RnTuXRU5aNF3 X/J/MGYTSpwNK0pRxZwdKj3+W46Nhfn7/p/7dL9+Oaz69/UIrRJ+4WQt7PCzRegh g2pSr+qZMtI0WJMfLRycXhCHT8/lQCK9hVb3376I/+3YCoGJaurXa3gWoirhyVrj 33MBdvGS1ihSOK3+Fjq5aWw8Ow74he2AVE5qZ+pyk+wDY7FTlVYw7ekpGtM+wg02 EjCWTLt5VuvOQ0OTJFDABweVisqDmINUrUl8m6Ruhmjk6ePhal5A1yIiFl5DN5f8 1NKVn6vCjTsgmb0a7zjCdyNmzYtK/UASGOefh6sfsbNKbO/fWXdQ1XBnTkMrhwNt +w0/XUoBfv/hc8U3NMNkfyIpy5aM82awq74Wbz5X8GEY0hL2vrmvDDe6hVHk4C0Z nWo3q2reC2BBzTaGBWjEWLOMJXjADTjQnR6ALd3nHll+UxjldxxE4dmBPozjnDLV YqTtMCU5NgK370VJq/foLV5awcjiGu6QtHulZYMPLL8Kgccr8Qz6QyZ8gmqbV9ii VU+BYuJ4j58ct1hXoS9W986230MsJOK2Y+A1J4c/EcdWqIlMH2I= =EaMx -----END PGP SIGNATURE----- --=-=-=--