[lustre-devel] [PATCH 00/22] Lustre: use while loop when emptying a list

[lustre-devel] [PATCH 00/22] Lustre: use while loop when emptying a list

[NeilBrown]

If you have a list that needs to be emptied, it is best to have a loop
like
  while (!list_empty(...))

because then it is obvious what the purpose of the loop is.
Many places in lustre use
  list_for_each_entry_safe()
instead, which obscures the purpose.
Several of these were from patches which deliberately converted from
the while loop the list list_for_each_entry_safe() loop, at least some
of which introduced real bugs.

This series reverts all those patches, and then makes other
conversions.
There are still several places which could be converted, but I got
bored...
I've particularly converted all where the list_head is a local
variable. In these cases it is obviously wrong not to empty the
list completely.

For those conversions that I did manual (not reverts) I use
  list_first_entry()
to get the first entry, rather then list_entry(head->next).
Others could be converted as well.

NeilBrown


---

NeilBrown (22):
      Revert "staging: lustre: lnet: api-ni: Use list_for_each_entry_safe"
      Revert "staging: lustre: o2iblnd: Use list_for_each_entry_safe in kiblnd_destroy_fmr_pool_list"
      Revert "staging: lustre: lnet: o2iblnd: Use list_for_each_entry_safe"
      Revert "staging: lustre: lnet: socklnd: Use list_for_each_entry_safe"
      Revert "staging: lustre: lnet: socklnd_proto: Use list_for_each_entry_safe"
      Revert "staging: lustre: osc_cache: Use list_for_each_entry_safe"
      Revert "staging: lustre: lnet: peer: Use list_for_each_entry_safe"
      Revert "staging: lustre: lnet: config: Use list_for_each_entry_safe"
      Revert "staging: lustre: lnet: router: Use list_for_each_entry_safe"
      Revert "staging: lustre: lnet: conrpc: Use list_for_each_entry_safe"
      Revert "staging: lustre: lnet: lib-move: Use list_for_each_entry_safe"
      Revert "staging: lustre: obdclass: Use list_for_each_entry_safe"
      Revert "staging: lustre: lnet: Use list_for_each_entry_safe"
      Revert: Staging: lustre: Iterate list using list_for_each_entry
      lustre: o2iblnd: convert list_for_each_entry_safe() to while(!list_empty())
      lustre: tracefile: convert list_for_each_entry_safe() to while(!list_empty())
      lustre: lib-move:  convert list_for_each_entry_safe() to while(!list_empty())
      lustre: net_fault: convert list_for_each_entry_safe() to while(!list_empty())
      lustre: fld_request: convert list_for_each_entry_safe() to while(!list_empty())
      lustre: lov_obd: convert list_for_each_entry_safe() to while(!list_empty())
      lustre: ldlm_request: convert list_for_each_entry_safe() to while(!list_empty())
      lustre: sec_config: convert list_for_each_entry_safe() to while(!list_empty())


 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |   21 ++++++++++++--------
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |   12 ++++++-----
 .../staging/lustre/lnet/klnds/socklnd/socklnd.c    |    9 +++++----
 .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |    5 +++--
 .../lustre/lnet/klnds/socklnd/socklnd_proto.c      |    4 ++--
 drivers/staging/lustre/lnet/libcfs/tracefile.c     |   12 +++++++----
 drivers/staging/lustre/lnet/lnet/api-ni.c          |   10 ++++++----
 drivers/staging/lustre/lnet/lnet/config.c          |    5 +++--
 drivers/staging/lustre/lnet/lnet/lib-move.c        |   13 +++++++-----
 drivers/staging/lustre/lnet/lnet/net_fault.c       |    7 +++++--
 drivers/staging/lustre/lnet/lnet/peer.c            |    4 ++--
 drivers/staging/lustre/lnet/lnet/router.c          |    4 ++--
 drivers/staging/lustre/lnet/selftest/conrpc.c      |    5 +++--
 drivers/staging/lustre/lustre/fld/fld_request.c    |    6 ++++--
 drivers/staging/lustre/lustre/ldlm/ldlm_request.c  |    6 ++++--
 drivers/staging/lustre/lustre/lov/lov_obd.c        |    5 +++--
 .../staging/lustre/lustre/obdclass/lustre_peer.c   |    5 +++--
 drivers/staging/lustre/lustre/osc/osc_cache.c      |    9 +++++----
 drivers/staging/lustre/lustre/ptlrpc/sec_config.c  |    7 ++++---
 19 files changed, 87 insertions(+), 62 deletions(-)

--
Signature

[lustre-devel] [PATCH 08/22] Revert "staging: lustre: lnet: config: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit cb734cf73eaed9b9bb7f190cceaafc15af0d8815.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/lnet/config.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/lnet/config.c b/drivers/staging/lustre/lnet/lnet/config.c
index 136905db2746..26b799e66e53 100644
--- a/drivers/staging/lustre/lnet/lnet/config.c
+++ b/drivers/staging/lustre/lnet/lnet/config.c
@@ -1026,7 +1026,6 @@ lnet_match_networks(char **networksp, char *ip2nets, __u32 *ipaddrs, int nip)
 	struct list_head *t;
 	struct list_head *t2;
 	struct lnet_text_buf *tb;
-	struct lnet_text_buf *temp;
 	struct lnet_text_buf *tb2;
 	__u32 net1;
 	__u32 net2;
@@ -1049,7 +1048,9 @@ lnet_match_networks(char **networksp, char *ip2nets, __u32 *ipaddrs, int nip)
 	len = 0;
 	rc = 0;
 
-	list_for_each_entry_safe(tb, temp, &raw_entries, ltb_list) {
+	while (!list_empty(&raw_entries)) {
+		tb = list_entry(raw_entries.next, struct lnet_text_buf,
+				ltb_list);
 		strncpy(source, tb->ltb_text, sizeof(source));
 		source[sizeof(source) - 1] = '\0';
 

[lustre-devel] [PATCH 08/22] Revert "staging: lustre: lnet: config: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit cb734cf73eaed9b9bb7f190cceaafc15af0d8815.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lnet/lnet/config.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/lnet/config.c b/drivers/staging/lustre/lnet/lnet/config.c
> index 136905db2746..26b799e66e53 100644
> --- a/drivers/staging/lustre/lnet/lnet/config.c
> +++ b/drivers/staging/lustre/lnet/lnet/config.c
> @@ -1026,7 +1026,6 @@ lnet_match_networks(char **networksp, char *ip2nets, __u32 *ipaddrs, int nip)
>  	struct list_head *t;
>  	struct list_head *t2;
>  	struct lnet_text_buf *tb;
> -	struct lnet_text_buf *temp;
>  	struct lnet_text_buf *tb2;
>  	__u32 net1;
>  	__u32 net2;
> @@ -1049,7 +1048,9 @@ lnet_match_networks(char **networksp, char *ip2nets, __u32 *ipaddrs, int nip)
>  	len = 0;
>  	rc = 0;
>  
> -	list_for_each_entry_safe(tb, temp, &raw_entries, ltb_list) {
> +	while (!list_empty(&raw_entries)) {
> +		tb = list_entry(raw_entries.next, struct lnet_text_buf,
> +				ltb_list);
>  		strncpy(source, tb->ltb_text, sizeof(source));
>  		source[sizeof(source) - 1] = '\0';
>  
> 
> 
> 

[lustre-devel] [PATCH 01/22] Revert "staging: lustre: lnet: api-ni: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit c997866cd27495ae28bc07596457e2bd83fb3275.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.  In this case, the first loop is
currently buggy.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/lnet/api-ni.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c
index d21bceeaceda..51a81075f8d5 100644
--- a/drivers/staging/lustre/lnet/lnet/api-ni.c
+++ b/drivers/staging/lustre/lnet/lnet/api-ni.c
@@ -1077,17 +1077,18 @@ lnet_clear_zombies_nis_locked(void)
 	int i;
 	int islo;
 	struct lnet_ni *ni;
-	struct lnet_ni *temp;
 
 	/*
 	 * Now wait for the NI's I just nuked to show up on ln_zombie_nis
 	 * and shut them down in guaranteed thread context
 	 */
 	i = 2;
-	list_for_each_entry_safe(ni, temp, &the_lnet.ln_nis_zombie, ni_list) {
+	while (!list_empty(&the_lnet.ln_nis_zombie)) {
 		int *ref;
 		int j;
 
+		ni = list_entry(the_lnet.ln_nis_zombie.next,
+				struct lnet_ni, ni_list);
 		list_del_init(&ni->ni_list);
 		cfs_percpt_for_each(ref, j, ni->ni_refs) {
 			if (!*ref)
@@ -1137,7 +1138,6 @@ static void
 lnet_shutdown_lndnis(void)
 {
 	struct lnet_ni *ni;
-	struct lnet_ni *temp;
 	int i;
 
 	/* NB called holding the global mutex */
@@ -1151,7 +1151,9 @@ lnet_shutdown_lndnis(void)
 	the_lnet.ln_shutdown = 1;	/* flag shutdown */
 
 	/* Unlink NIs from the global table */
-	list_for_each_entry_safe(ni, temp, &the_lnet.ln_nis, ni_list) {
+	while (!list_empty(&the_lnet.ln_nis)) {
+		ni = list_entry(the_lnet.ln_nis.next,
+				struct lnet_ni, ni_list);
 		lnet_ni_unlink_locked(ni);
 	}
 

[lustre-devel] [PATCH 01/22] Revert "staging: lustre: lnet: api-ni: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit c997866cd27495ae28bc07596457e2bd83fb3275.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.  In this case, the first loop is
> currently buggy.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lnet/lnet/api-ni.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c
> index d21bceeaceda..51a81075f8d5 100644
> --- a/drivers/staging/lustre/lnet/lnet/api-ni.c
> +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c
> @@ -1077,17 +1077,18 @@ lnet_clear_zombies_nis_locked(void)
>  	int i;
>  	int islo;
>  	struct lnet_ni *ni;
> -	struct lnet_ni *temp;
>  
>  	/*
>  	 * Now wait for the NI's I just nuked to show up on ln_zombie_nis
>  	 * and shut them down in guaranteed thread context
>  	 */
>  	i = 2;
> -	list_for_each_entry_safe(ni, temp, &the_lnet.ln_nis_zombie, ni_list) {
> +	while (!list_empty(&the_lnet.ln_nis_zombie)) {
>  		int *ref;
>  		int j;
>  
> +		ni = list_entry(the_lnet.ln_nis_zombie.next,
> +				struct lnet_ni, ni_list);
>  		list_del_init(&ni->ni_list);
>  		cfs_percpt_for_each(ref, j, ni->ni_refs) {
>  			if (!*ref)
> @@ -1137,7 +1138,6 @@ static void
>  lnet_shutdown_lndnis(void)
>  {
>  	struct lnet_ni *ni;
> -	struct lnet_ni *temp;
>  	int i;
>  
>  	/* NB called holding the global mutex */
> @@ -1151,7 +1151,9 @@ lnet_shutdown_lndnis(void)
>  	the_lnet.ln_shutdown = 1;	/* flag shutdown */
>  
>  	/* Unlink NIs from the global table */
> -	list_for_each_entry_safe(ni, temp, &the_lnet.ln_nis, ni_list) {
> +	while (!list_empty(&the_lnet.ln_nis)) {
> +		ni = list_entry(the_lnet.ln_nis.next,
> +				struct lnet_ni, ni_list);
>  		lnet_ni_unlink_locked(ni);
>  	}
>  
> 
> 
> 

[lustre-devel] [PATCH 03/22] Revert "staging: lustre: lnet: o2iblnd: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit 995ae68c30a5d4947f7685f29b1e69b436ddcdb3.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
index a5ef4cc6c04f..9cf1f64e3d76 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
@@ -3185,7 +3185,6 @@ kiblnd_check_conns(int idx)
 	struct list_head *ptmp;
 	struct kib_peer *peer;
 	struct kib_conn *conn;
-	struct kib_conn *temp;
 	struct kib_conn *tmp;
 	struct list_head *ctmp;
 	unsigned long flags;
@@ -3253,7 +3252,8 @@ kiblnd_check_conns(int idx)
 	 * NOOP, but there were no non-blocking tx descs
 	 * free to do it last time...
 	 */
-	list_for_each_entry_safe(conn, temp, &checksends, ibc_connd_list) {
+	while (!list_empty(&checksends)) {
+		conn = list_entry(checksends.next, struct kib_conn, ibc_connd_list);
 		list_del(&conn->ibc_connd_list);
 
 		spin_lock(&conn->ibc_lock);

[lustre-devel] [PATCH 03/22] Revert "staging: lustre: lnet: o2iblnd: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit 995ae68c30a5d4947f7685f29b1e69b436ddcdb3.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
> index a5ef4cc6c04f..9cf1f64e3d76 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
> @@ -3185,7 +3185,6 @@ kiblnd_check_conns(int idx)
>  	struct list_head *ptmp;
>  	struct kib_peer *peer;
>  	struct kib_conn *conn;
> -	struct kib_conn *temp;
>  	struct kib_conn *tmp;
>  	struct list_head *ctmp;
>  	unsigned long flags;
> @@ -3253,7 +3252,8 @@ kiblnd_check_conns(int idx)
>  	 * NOOP, but there were no non-blocking tx descs
>  	 * free to do it last time...
>  	 */
> -	list_for_each_entry_safe(conn, temp, &checksends, ibc_connd_list) {
> +	while (!list_empty(&checksends)) {
> +		conn = list_entry(checksends.next, struct kib_conn, ibc_connd_list);
>  		list_del(&conn->ibc_connd_list);
>  
>  		spin_lock(&conn->ibc_lock);
> 
> 
> 

[lustre-devel] [PATCH 06/22] Revert "staging: lustre: osc_cache: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit 4a81ce53a61c72afb079c096599a5d34749b9dd7.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/osc/osc_cache.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c
index 15a48173e148..87d0d16d942b 100644
--- a/drivers/staging/lustre/lustre/osc/osc_cache.c
+++ b/drivers/staging/lustre/lustre/osc/osc_cache.c
@@ -2060,7 +2060,6 @@ static unsigned int get_write_extents(struct osc_object *obj,
 {
 	struct client_obd *cli = osc_cli(obj);
 	struct osc_extent *ext;
-	struct osc_extent *temp;
 	struct extent_rpc_data data = {
 		.erd_rpc_list = rpclist,
 		.erd_page_count = 0,
@@ -2070,7 +2069,9 @@ static unsigned int get_write_extents(struct osc_object *obj,
 	};
 
 	LASSERT(osc_object_is_locked(obj));
-	list_for_each_entry_safe(ext, temp, &obj->oo_hp_exts, oe_link) {
+	while (!list_empty(&obj->oo_hp_exts)) {
+		ext = list_entry(obj->oo_hp_exts.next, struct osc_extent,
+				 oe_link);
 		LASSERT(ext->oe_state == OES_CACHE);
 		if (!try_to_add_extent_for_io(cli, ext, &data))
 			return data.erd_page_count;
@@ -2829,7 +2830,6 @@ int osc_cache_truncate_start(const struct lu_env *env, struct osc_object *obj,
 {
 	struct client_obd *cli = osc_cli(obj);
 	struct osc_extent *ext;
-	struct osc_extent *temp;
 	struct osc_extent *waiting = NULL;
 	pgoff_t index;
 	LIST_HEAD(list);
@@ -2888,9 +2888,10 @@ int osc_cache_truncate_start(const struct lu_env *env, struct osc_object *obj,
 
 	osc_list_maint(cli, obj);
 
-	list_for_each_entry_safe(ext, temp, &list, oe_link) {
+	while (!list_empty(&list)) {
 		int rc;
 
+		ext = list_entry(list.next, struct osc_extent, oe_link);
 		list_del_init(&ext->oe_link);
 
 		/* extent may be in OES_ACTIVE state because inode mutex

[lustre-devel] [PATCH 06/22] Revert "staging: lustre: osc_cache: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit 4a81ce53a61c72afb079c096599a5d34749b9dd7.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lustre/osc/osc_cache.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c
> index 15a48173e148..87d0d16d942b 100644
> --- a/drivers/staging/lustre/lustre/osc/osc_cache.c
> +++ b/drivers/staging/lustre/lustre/osc/osc_cache.c
> @@ -2060,7 +2060,6 @@ static unsigned int get_write_extents(struct osc_object *obj,
>  {
>  	struct client_obd *cli = osc_cli(obj);
>  	struct osc_extent *ext;
> -	struct osc_extent *temp;
>  	struct extent_rpc_data data = {
>  		.erd_rpc_list = rpclist,
>  		.erd_page_count = 0,
> @@ -2070,7 +2069,9 @@ static unsigned int get_write_extents(struct osc_object *obj,
>  	};
>  
>  	LASSERT(osc_object_is_locked(obj));
> -	list_for_each_entry_safe(ext, temp, &obj->oo_hp_exts, oe_link) {
> +	while (!list_empty(&obj->oo_hp_exts)) {
> +		ext = list_entry(obj->oo_hp_exts.next, struct osc_extent,
> +				 oe_link);
>  		LASSERT(ext->oe_state == OES_CACHE);
>  		if (!try_to_add_extent_for_io(cli, ext, &data))
>  			return data.erd_page_count;
> @@ -2829,7 +2830,6 @@ int osc_cache_truncate_start(const struct lu_env *env, struct osc_object *obj,
>  {
>  	struct client_obd *cli = osc_cli(obj);
>  	struct osc_extent *ext;
> -	struct osc_extent *temp;
>  	struct osc_extent *waiting = NULL;
>  	pgoff_t index;
>  	LIST_HEAD(list);
> @@ -2888,9 +2888,10 @@ int osc_cache_truncate_start(const struct lu_env *env, struct osc_object *obj,
>  
>  	osc_list_maint(cli, obj);
>  
> -	list_for_each_entry_safe(ext, temp, &list, oe_link) {
> +	while (!list_empty(&list)) {
>  		int rc;
>  
> +		ext = list_entry(list.next, struct osc_extent, oe_link);
>  		list_del_init(&ext->oe_link);
>  
>  		/* extent may be in OES_ACTIVE state because inode mutex
> 
> 
> 

[lustre-devel] [PATCH 02/22] Revert "staging: lustre: o2iblnd: Use list_for_each_entry_safe in kiblnd_destroy_fmr_pool_list"

[NeilBrown]

This reverts commit 0d33ec5f95fe068d7e96b6e7ed9216de93f6b5b0.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index 05835cc0f0a5..124870ada28b 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -1306,9 +1306,10 @@ static void kiblnd_destroy_fmr_pool(struct kib_fmr_pool *fpo)
 
 static void kiblnd_destroy_fmr_pool_list(struct list_head *head)
 {
-	struct kib_fmr_pool *fpo, *tmp;
+	struct kib_fmr_pool *fpo;
 
-	list_for_each_entry_safe(fpo, tmp, head, fpo_list) {
+	while (!list_empty(head)) {
+		fpo = list_entry(head->next, struct kib_fmr_pool, fpo_list);
 		list_del(&fpo->fpo_list);
 		kiblnd_destroy_fmr_pool(fpo);
 	}

[lustre-devel] [PATCH 02/22] Revert "staging: lustre: o2iblnd: Use list_for_each_entry_safe in kiblnd_destroy_fmr_pool_list"

[James Simmons]

> This reverts commit 0d33ec5f95fe068d7e96b6e7ed9216de93f6b5b0.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.
>

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> index 05835cc0f0a5..124870ada28b 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> @@ -1306,9 +1306,10 @@ static void kiblnd_destroy_fmr_pool(struct kib_fmr_pool *fpo)
>  
>  static void kiblnd_destroy_fmr_pool_list(struct list_head *head)
>  {
> -	struct kib_fmr_pool *fpo, *tmp;
> +	struct kib_fmr_pool *fpo;
>  
> -	list_for_each_entry_safe(fpo, tmp, head, fpo_list) {
> +	while (!list_empty(head)) {
> +		fpo = list_entry(head->next, struct kib_fmr_pool, fpo_list);
>  		list_del(&fpo->fpo_list);
>  		kiblnd_destroy_fmr_pool(fpo);
>  	}
> 
> 
> 

[lustre-devel] [PATCH 09/22] Revert "staging: lustre: lnet: router: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit 37cce1bcb750ac12773f1c53afe88a8433f53eb3.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/lnet/router.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
index 3f84df6cb3b1..965087b7359c 100644
--- a/drivers/staging/lustre/lnet/lnet/router.c
+++ b/drivers/staging/lustre/lnet/lnet/router.c
@@ -1340,7 +1340,6 @@ lnet_rtrpool_free_bufs(struct lnet_rtrbufpool *rbp, int cpt)
 	int npages = rbp->rbp_npages;
 	struct list_head tmp;
 	struct lnet_rtrbuf *rb;
-	struct lnet_rtrbuf *temp;
 
 	if (!rbp->rbp_nbuffers) /* not initialized or already freed */
 		return;
@@ -1357,7 +1356,8 @@ lnet_rtrpool_free_bufs(struct lnet_rtrbufpool *rbp, int cpt)
 	lnet_net_unlock(cpt);
 
 	/* Free buffers on the free list. */
-	list_for_each_entry_safe(rb, temp, &tmp, rb_list) {
+	while (!list_empty(&tmp)) {
+		rb = list_entry(tmp.next, struct lnet_rtrbuf, rb_list);
 		list_del(&rb->rb_list);
 		lnet_destroy_rtrbuf(rb, npages);
 	}

[lustre-devel] [PATCH 09/22] Revert "staging: lustre: lnet: router: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit 37cce1bcb750ac12773f1c53afe88a8433f53eb3.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lnet/lnet/router.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
> index 3f84df6cb3b1..965087b7359c 100644
> --- a/drivers/staging/lustre/lnet/lnet/router.c
> +++ b/drivers/staging/lustre/lnet/lnet/router.c
> @@ -1340,7 +1340,6 @@ lnet_rtrpool_free_bufs(struct lnet_rtrbufpool *rbp, int cpt)
>  	int npages = rbp->rbp_npages;
>  	struct list_head tmp;
>  	struct lnet_rtrbuf *rb;
> -	struct lnet_rtrbuf *temp;
>  
>  	if (!rbp->rbp_nbuffers) /* not initialized or already freed */
>  		return;
> @@ -1357,7 +1356,8 @@ lnet_rtrpool_free_bufs(struct lnet_rtrbufpool *rbp, int cpt)
>  	lnet_net_unlock(cpt);
>  
>  	/* Free buffers on the free list. */
> -	list_for_each_entry_safe(rb, temp, &tmp, rb_list) {
> +	while (!list_empty(&tmp)) {
> +		rb = list_entry(tmp.next, struct lnet_rtrbuf, rb_list);
>  		list_del(&rb->rb_list);
>  		lnet_destroy_rtrbuf(rb, npages);
>  	}
> 
> 
> 

[lustre-devel] [PATCH 07/22] Revert "staging: lustre: lnet: peer: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit 3e47a1cfba5a8af7dc3c10a4705d8047abdc26c3.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/lnet/peer.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/lnet/peer.c b/drivers/staging/lustre/lnet/lnet/peer.c
index 6ce175d77d0f..7c303ef6bb34 100644
--- a/drivers/staging/lustre/lnet/lnet/peer.c
+++ b/drivers/staging/lustre/lnet/lnet/peer.c
@@ -176,7 +176,6 @@ lnet_peer_tables_cleanup(struct lnet_ni *ni)
 	struct lnet_peer_table *ptable;
 	struct list_head deathrow;
 	struct lnet_peer *lp;
-	struct lnet_peer *temp;
 	int i;
 
 	INIT_LIST_HEAD(&deathrow);
@@ -210,7 +209,8 @@ lnet_peer_tables_cleanup(struct lnet_ni *ni)
 		lnet_net_unlock(i);
 	}
 
-	list_for_each_entry_safe(lp, temp, &deathrow, lp_hashlist) {
+	while (!list_empty(&deathrow)) {
+		lp = list_entry(deathrow.next, struct lnet_peer, lp_hashlist);
 		list_del(&lp->lp_hashlist);
 		kfree(lp);
 	}

[lustre-devel] [PATCH 07/22] Revert "staging: lustre: lnet: peer: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit 3e47a1cfba5a8af7dc3c10a4705d8047abdc26c3.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lnet/lnet/peer.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/lnet/peer.c b/drivers/staging/lustre/lnet/lnet/peer.c
> index 6ce175d77d0f..7c303ef6bb34 100644
> --- a/drivers/staging/lustre/lnet/lnet/peer.c
> +++ b/drivers/staging/lustre/lnet/lnet/peer.c
> @@ -176,7 +176,6 @@ lnet_peer_tables_cleanup(struct lnet_ni *ni)
>  	struct lnet_peer_table *ptable;
>  	struct list_head deathrow;
>  	struct lnet_peer *lp;
> -	struct lnet_peer *temp;
>  	int i;
>  
>  	INIT_LIST_HEAD(&deathrow);
> @@ -210,7 +209,8 @@ lnet_peer_tables_cleanup(struct lnet_ni *ni)
>  		lnet_net_unlock(i);
>  	}
>  
> -	list_for_each_entry_safe(lp, temp, &deathrow, lp_hashlist) {
> +	while (!list_empty(&deathrow)) {
> +		lp = list_entry(deathrow.next, struct lnet_peer, lp_hashlist);
>  		list_del(&lp->lp_hashlist);
>  		kfree(lp);
>  	}
> 
> 
> 

[lustre-devel] [PATCH 04/22] Revert "staging: lustre: lnet: socklnd: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit 2aff15d43a832cd0af0263e4456e5b01e15f86c6.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lnet/klnds/socklnd/socklnd.c    |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
index 6b1df0c98cbd..a48b1b9a850b 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
@@ -1546,7 +1546,6 @@ ksocknal_finalize_zcreq(struct ksock_conn *conn)
 {
 	struct ksock_peer *peer = conn->ksnc_peer;
 	struct ksock_tx *tx;
-	struct ksock_tx *temp;
 	struct ksock_tx *tmp;
 	LIST_HEAD(zlist);
 
@@ -1572,7 +1571,9 @@ ksocknal_finalize_zcreq(struct ksock_conn *conn)
 
 	spin_unlock(&peer->ksnp_lock);
 
-	list_for_each_entry_safe(tx, temp, &zlist, tx_zc_list) {
+	while (!list_empty(&zlist)) {
+		tx = list_entry(zlist.next, struct ksock_tx, tx_zc_list);
+
 		list_del(&tx->tx_zc_list);
 		ksocknal_tx_decref(tx);
 	}
@@ -2270,13 +2271,13 @@ ksocknal_free_buffers(void)
 	if (!list_empty(&ksocknal_data.ksnd_idle_noop_txs)) {
 		struct list_head zlist;
 		struct ksock_tx *tx;
-		struct ksock_tx *temp;
 
 		list_add(&zlist, &ksocknal_data.ksnd_idle_noop_txs);
 		list_del_init(&ksocknal_data.ksnd_idle_noop_txs);
 		spin_unlock(&ksocknal_data.ksnd_tx_lock);
 
-		list_for_each_entry_safe(tx, temp, &zlist, tx_list) {
+		while (!list_empty(&zlist)) {
+			tx = list_entry(zlist.next, struct ksock_tx, tx_list);
 			list_del(&tx->tx_list);
 			kfree(tx);
 		}

[lustre-devel] [PATCH 04/22] Revert "staging: lustre: lnet: socklnd: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit 2aff15d43a832cd0af0263e4456e5b01e15f86c6.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  .../staging/lustre/lnet/klnds/socklnd/socklnd.c    |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> index 6b1df0c98cbd..a48b1b9a850b 100644
> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> @@ -1546,7 +1546,6 @@ ksocknal_finalize_zcreq(struct ksock_conn *conn)
>  {
>  	struct ksock_peer *peer = conn->ksnc_peer;
>  	struct ksock_tx *tx;
> -	struct ksock_tx *temp;
>  	struct ksock_tx *tmp;
>  	LIST_HEAD(zlist);
>  
> @@ -1572,7 +1571,9 @@ ksocknal_finalize_zcreq(struct ksock_conn *conn)
>  
>  	spin_unlock(&peer->ksnp_lock);
>  
> -	list_for_each_entry_safe(tx, temp, &zlist, tx_zc_list) {
> +	while (!list_empty(&zlist)) {
> +		tx = list_entry(zlist.next, struct ksock_tx, tx_zc_list);
> +
>  		list_del(&tx->tx_zc_list);
>  		ksocknal_tx_decref(tx);
>  	}
> @@ -2270,13 +2271,13 @@ ksocknal_free_buffers(void)
>  	if (!list_empty(&ksocknal_data.ksnd_idle_noop_txs)) {
>  		struct list_head zlist;
>  		struct ksock_tx *tx;
> -		struct ksock_tx *temp;
>  
>  		list_add(&zlist, &ksocknal_data.ksnd_idle_noop_txs);
>  		list_del_init(&ksocknal_data.ksnd_idle_noop_txs);
>  		spin_unlock(&ksocknal_data.ksnd_tx_lock);
>  
> -		list_for_each_entry_safe(tx, temp, &zlist, tx_list) {
> +		while (!list_empty(&zlist)) {
> +			tx = list_entry(zlist.next, struct ksock_tx, tx_list);
>  			list_del(&tx->tx_list);
>  			kfree(tx);
>  		}
> 
> 
> 

[lustre-devel] [PATCH 04/22] Revert "staging: lustre: lnet: socklnd: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit 2aff15d43a832cd0af0263e4456e5b01e15f86c6.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  .../staging/lustre/lnet/klnds/socklnd/socklnd.c    |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> index 6b1df0c98cbd..a48b1b9a850b 100644
> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> @@ -1546,7 +1546,6 @@ ksocknal_finalize_zcreq(struct ksock_conn *conn)
>  {
>  	struct ksock_peer *peer = conn->ksnc_peer;
>  	struct ksock_tx *tx;
> -	struct ksock_tx *temp;
>  	struct ksock_tx *tmp;
>  	LIST_HEAD(zlist);
>  
> @@ -1572,7 +1571,9 @@ ksocknal_finalize_zcreq(struct ksock_conn *conn)
>  
>  	spin_unlock(&peer->ksnp_lock);
>  
> -	list_for_each_entry_safe(tx, temp, &zlist, tx_zc_list) {
> +	while (!list_empty(&zlist)) {
> +		tx = list_entry(zlist.next, struct ksock_tx, tx_zc_list);
> +
>  		list_del(&tx->tx_zc_list);
>  		ksocknal_tx_decref(tx);
>  	}
> @@ -2270,13 +2271,13 @@ ksocknal_free_buffers(void)
>  	if (!list_empty(&ksocknal_data.ksnd_idle_noop_txs)) {
>  		struct list_head zlist;
>  		struct ksock_tx *tx;
> -		struct ksock_tx *temp;
>  
>  		list_add(&zlist, &ksocknal_data.ksnd_idle_noop_txs);
>  		list_del_init(&ksocknal_data.ksnd_idle_noop_txs);
>  		spin_unlock(&ksocknal_data.ksnd_tx_lock);
>  
> -		list_for_each_entry_safe(tx, temp, &zlist, tx_list) {
> +		while (!list_empty(&zlist)) {
> +			tx = list_entry(zlist.next, struct ksock_tx, tx_list);
>  			list_del(&tx->tx_list);
>  			kfree(tx);
>  		}
> 
> 
> 

[lustre-devel] [PATCH 05/22] Revert "staging: lustre: lnet: socklnd_proto: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit 1edae04ff85fe65a333949de6101578c015a21fa.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../lustre/lnet/klnds/socklnd/socklnd_proto.c      |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c
index aaa04a5f0527..abfaf5701758 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c
@@ -413,7 +413,6 @@ ksocknal_handle_zcack(struct ksock_conn *conn, __u64 cookie1, __u64 cookie2)
 {
 	struct ksock_peer *peer = conn->ksnc_peer;
 	struct ksock_tx *tx;
-	struct ksock_tx *temp;
 	struct ksock_tx *tmp;
 	LIST_HEAD(zlist);
 	int count;
@@ -448,7 +447,8 @@ ksocknal_handle_zcack(struct ksock_conn *conn, __u64 cookie1, __u64 cookie2)
 
 	spin_unlock(&peer->ksnp_lock);
 
-	list_for_each_entry_safe(tx, temp, &zlist, tx_zc_list) {
+	while (!list_empty(&zlist)) {
+		tx = list_entry(zlist.next, struct ksock_tx, tx_zc_list);
 		list_del(&tx->tx_zc_list);
 		ksocknal_tx_decref(tx);
 	}

[lustre-devel] [PATCH 05/22] Revert "staging: lustre: lnet: socklnd_proto: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit 1edae04ff85fe65a333949de6101578c015a21fa.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  .../lustre/lnet/klnds/socklnd/socklnd_proto.c      |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c
> index aaa04a5f0527..abfaf5701758 100644
> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c
> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c
> @@ -413,7 +413,6 @@ ksocknal_handle_zcack(struct ksock_conn *conn, __u64 cookie1, __u64 cookie2)
>  {
>  	struct ksock_peer *peer = conn->ksnc_peer;
>  	struct ksock_tx *tx;
> -	struct ksock_tx *temp;
>  	struct ksock_tx *tmp;
>  	LIST_HEAD(zlist);
>  	int count;
> @@ -448,7 +447,8 @@ ksocknal_handle_zcack(struct ksock_conn *conn, __u64 cookie1, __u64 cookie2)
>  
>  	spin_unlock(&peer->ksnp_lock);
>  
> -	list_for_each_entry_safe(tx, temp, &zlist, tx_zc_list) {
> +	while (!list_empty(&zlist)) {
> +		tx = list_entry(zlist.next, struct ksock_tx, tx_zc_list);
>  		list_del(&tx->tx_zc_list);
>  		ksocknal_tx_decref(tx);
>  	}
> 
> 
> 

[lustre-devel] [PATCH 22/22] lustre: sec_config: convert list_for_each_entry_safe() to while(!list_empty())

[NeilBrown]

These loops are removing all elements from a list.
So using while(!list_empty()) makes the intent clearer.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/ptlrpc/sec_config.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_config.c b/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
index 2389f9a8f534..1844ada6780f 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
@@ -839,12 +839,13 @@ int sptlrpc_conf_init(void)
 
 void sptlrpc_conf_fini(void)
 {
-	struct sptlrpc_conf *conf, *conf_next;
+	struct sptlrpc_conf *conf;
 
 	mutex_lock(&sptlrpc_conf_lock);
-	list_for_each_entry_safe(conf, conf_next, &sptlrpc_confs, sc_list) {
+	while (!list_empty(&sptlrpc_confs)) {
+		conf = list_first_entry(&sptlrpc_confs,
+					struct sptlrpc_conf, sc_list);
 		sptlrpc_conf_free(conf);
 	}
-	LASSERT(list_empty(&sptlrpc_confs));
 	mutex_unlock(&sptlrpc_conf_lock);
 }

[lustre-devel] [PATCH 22/22] lustre: sec_config: convert list_for_each_entry_safe() to while(!list_empty())

[James Simmons]

> These loops are removing all elements from a list.
> So using while(!list_empty()) makes the intent clearer.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lustre/ptlrpc/sec_config.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_config.c b/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
> index 2389f9a8f534..1844ada6780f 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/sec_config.c
> @@ -839,12 +839,13 @@ int sptlrpc_conf_init(void)
>  
>  void sptlrpc_conf_fini(void)
>  {
> -	struct sptlrpc_conf *conf, *conf_next;
> +	struct sptlrpc_conf *conf;
>  
>  	mutex_lock(&sptlrpc_conf_lock);
> -	list_for_each_entry_safe(conf, conf_next, &sptlrpc_confs, sc_list) {
> +	while (!list_empty(&sptlrpc_confs)) {
> +		conf = list_first_entry(&sptlrpc_confs,
> +					struct sptlrpc_conf, sc_list);
>  		sptlrpc_conf_free(conf);
>  	}
> -	LASSERT(list_empty(&sptlrpc_confs));
>  	mutex_unlock(&sptlrpc_conf_lock);
>  }
> 
> 
> 

[lustre-devel] [PATCH 17/22] lustre: lib-move: convert list_for_each_entry_safe() to while(!list_empty())

[NeilBrown]

These loops are removing all elements from a list.
So using while(!list_empty()) makes the intent clearer.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/lnet/lib-move.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/lnet/lib-move.c b/drivers/staging/lustre/lnet/lnet/lib-move.c
index 2756e91b34bb..19cab374b6bc 100644
--- a/drivers/staging/lustre/lnet/lnet/lib-move.c
+++ b/drivers/staging/lustre/lnet/lnet/lib-move.c
@@ -853,7 +853,6 @@ lnet_drop_routed_msgs_locked(struct list_head *list, int cpt)
 {
 	struct list_head drop;
 	struct lnet_msg *msg;
-	struct lnet_msg *tmp;
 
 	INIT_LIST_HEAD(&drop);
 
@@ -861,7 +860,8 @@ lnet_drop_routed_msgs_locked(struct list_head *list, int cpt)
 
 	lnet_net_unlock(cpt);
 
-	list_for_each_entry_safe(msg, tmp, &drop, msg_list) {
+	while(!list_empty(&drop)) {
+		msg = list_first_entry(&drop, struct lnet_msg, msg_list);
 		lnet_ni_recv(msg->msg_rxpeer->lp_ni, msg->msg_private, NULL,
 			     0, 0, 0, msg->msg_hdr.payload_length);
 		list_del_init(&msg->msg_list);

[lustre-devel] [PATCH 17/22] lustre: lib-move: convert list_for_each_entry_safe() to while(!list_empty())

[James Simmons]

> These loops are removing all elements from a list.
> So using while(!list_empty()) makes the intent clearer.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lnet/lnet/lib-move.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/lnet/lib-move.c b/drivers/staging/lustre/lnet/lnet/lib-move.c
> index 2756e91b34bb..19cab374b6bc 100644
> --- a/drivers/staging/lustre/lnet/lnet/lib-move.c
> +++ b/drivers/staging/lustre/lnet/lnet/lib-move.c
> @@ -853,7 +853,6 @@ lnet_drop_routed_msgs_locked(struct list_head *list, int cpt)
>  {
>  	struct list_head drop;
>  	struct lnet_msg *msg;
> -	struct lnet_msg *tmp;
>  
>  	INIT_LIST_HEAD(&drop);
>  
> @@ -861,7 +860,8 @@ lnet_drop_routed_msgs_locked(struct list_head *list, int cpt)
>  
>  	lnet_net_unlock(cpt);
>  
> -	list_for_each_entry_safe(msg, tmp, &drop, msg_list) {
> +	while(!list_empty(&drop)) {
> +		msg = list_first_entry(&drop, struct lnet_msg, msg_list);
>  		lnet_ni_recv(msg->msg_rxpeer->lp_ni, msg->msg_private, NULL,
>  			     0, 0, 0, msg->msg_hdr.payload_length);
>  		list_del_init(&msg->msg_list);
> 
> 
> 

[lustre-devel] [PATCH 21/22] lustre: ldlm_request: convert list_for_each_entry_safe() to while(!list_empty())

[NeilBrown]

These loops are removing all elements from a list.
So using while(!list_empty()) makes the intent clearer.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_request.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
index cdc52eed6d85..80260b07f0f0 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
@@ -2000,7 +2000,7 @@ int ldlm_replay_locks(struct obd_import *imp)
 {
 	struct ldlm_namespace *ns = imp->imp_obd->obd_namespace;
 	LIST_HEAD(list);
-	struct ldlm_lock *lock, *next;
+	struct ldlm_lock *lock;
 	int rc = 0;
 
 	LASSERT(atomic_read(&imp->imp_replay_inflight) == 0);
@@ -2017,7 +2017,9 @@ int ldlm_replay_locks(struct obd_import *imp)
 
 	ldlm_namespace_foreach(ns, ldlm_chain_lock_for_replay, &list);
 
-	list_for_each_entry_safe(lock, next, &list, l_pending_chain) {
+	while (!list_empty(&list)) {
+		lock = list_first_entry(&list, struct ldlm_lock,
+					l_pending_chain);
 		list_del_init(&lock->l_pending_chain);
 		if (rc) {
 			LDLM_LOCK_RELEASE(lock);

[lustre-devel] [PATCH 21/22] lustre: ldlm_request: convert list_for_each_entry_safe() to while(!list_empty())

[James Simmons]

> These loops are removing all elements from a list.
> So using while(!list_empty()) makes the intent clearer.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lustre/ldlm/ldlm_request.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
> index cdc52eed6d85..80260b07f0f0 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
> @@ -2000,7 +2000,7 @@ int ldlm_replay_locks(struct obd_import *imp)
>  {
>  	struct ldlm_namespace *ns = imp->imp_obd->obd_namespace;
>  	LIST_HEAD(list);
> -	struct ldlm_lock *lock, *next;
> +	struct ldlm_lock *lock;
>  	int rc = 0;
>  
>  	LASSERT(atomic_read(&imp->imp_replay_inflight) == 0);
> @@ -2017,7 +2017,9 @@ int ldlm_replay_locks(struct obd_import *imp)
>  
>  	ldlm_namespace_foreach(ns, ldlm_chain_lock_for_replay, &list);
>  
> -	list_for_each_entry_safe(lock, next, &list, l_pending_chain) {
> +	while (!list_empty(&list)) {
> +		lock = list_first_entry(&list, struct ldlm_lock,
> +					l_pending_chain);
>  		list_del_init(&lock->l_pending_chain);
>  		if (rc) {
>  			LDLM_LOCK_RELEASE(lock);
> 
> 
> 

[lustre-devel] [PATCH 15/22] lustre: o2iblnd: convert list_for_each_entry_safe() to while(!list_empty())

[NeilBrown]

These loops are removing all element from a list.
So using while(!list_empty()) makes the intent clearer.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index 124870ada28b..830a5bf34c16 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -1283,11 +1283,13 @@ static void kiblnd_destroy_fmr_pool(struct kib_fmr_pool *fpo)
 		if (fpo->fmr.fpo_fmr_pool)
 			ib_destroy_fmr_pool(fpo->fmr.fpo_fmr_pool);
 	} else {
-		struct kib_fast_reg_descriptor *frd, *tmp;
+		struct kib_fast_reg_descriptor *frd;
 		int i = 0;
 
-		list_for_each_entry_safe(frd, tmp, &fpo->fast_reg.fpo_pool_list,
-					 frd_list) {
+		while (!list_empty(&fpo->fast_reg.fpo_pool_list)) {
+			frd = list_first_entry(&fpo->fast_reg.fpo_pool_list,
+					       struct kib_fast_reg_descriptor,
+					       frd_list);
 			list_del(&frd->frd_list);
 			ib_dereg_mr(frd->frd_mr);
 			kfree(frd);
@@ -1362,7 +1364,7 @@ static int kiblnd_alloc_fmr_pool(struct kib_fmr_poolset *fps, struct kib_fmr_poo
 
 static int kiblnd_alloc_freg_pool(struct kib_fmr_poolset *fps, struct kib_fmr_pool *fpo)
 {
-	struct kib_fast_reg_descriptor *frd, *tmp;
+	struct kib_fast_reg_descriptor *frd;
 	int i, rc;
 
 	INIT_LIST_HEAD(&fpo->fast_reg.fpo_pool_list);
@@ -1399,8 +1401,10 @@ static int kiblnd_alloc_freg_pool(struct kib_fmr_poolset *fps, struct kib_fmr_po
 	kfree(frd);
 
 out:
-	list_for_each_entry_safe(frd, tmp, &fpo->fast_reg.fpo_pool_list,
-				 frd_list) {
+	while (!list_empty(&fpo->fast_reg.fpo_pool_list)) {
+		frd = list_first_entry(&fpo->fast_reg.fpo_pool_list,
+				       struct kib_fast_reg_descriptor,
+				       frd_list);
 		list_del(&frd->frd_list);
 		ib_dereg_mr(frd->frd_mr);
 		kfree(frd);

[lustre-devel] [PATCH 15/22] lustre: o2iblnd: convert list_for_each_entry_safe() to while(!list_empty())

[James Simmons]

> These loops are removing all element from a list.
> So using while(!list_empty()) makes the intent clearer.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |   16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> index 124870ada28b..830a5bf34c16 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> @@ -1283,11 +1283,13 @@ static void kiblnd_destroy_fmr_pool(struct kib_fmr_pool *fpo)
>  		if (fpo->fmr.fpo_fmr_pool)
>  			ib_destroy_fmr_pool(fpo->fmr.fpo_fmr_pool);
>  	} else {
> -		struct kib_fast_reg_descriptor *frd, *tmp;
> +		struct kib_fast_reg_descriptor *frd;
>  		int i = 0;
>  
> -		list_for_each_entry_safe(frd, tmp, &fpo->fast_reg.fpo_pool_list,
> -					 frd_list) {
> +		while (!list_empty(&fpo->fast_reg.fpo_pool_list)) {
> +			frd = list_first_entry(&fpo->fast_reg.fpo_pool_list,
> +					       struct kib_fast_reg_descriptor,
> +					       frd_list);
>  			list_del(&frd->frd_list);
>  			ib_dereg_mr(frd->frd_mr);
>  			kfree(frd);
> @@ -1362,7 +1364,7 @@ static int kiblnd_alloc_fmr_pool(struct kib_fmr_poolset *fps, struct kib_fmr_poo
>  
>  static int kiblnd_alloc_freg_pool(struct kib_fmr_poolset *fps, struct kib_fmr_pool *fpo)
>  {
> -	struct kib_fast_reg_descriptor *frd, *tmp;
> +	struct kib_fast_reg_descriptor *frd;
>  	int i, rc;
>  
>  	INIT_LIST_HEAD(&fpo->fast_reg.fpo_pool_list);
> @@ -1399,8 +1401,10 @@ static int kiblnd_alloc_freg_pool(struct kib_fmr_poolset *fps, struct kib_fmr_po
>  	kfree(frd);
>  
>  out:
> -	list_for_each_entry_safe(frd, tmp, &fpo->fast_reg.fpo_pool_list,
> -				 frd_list) {
> +	while (!list_empty(&fpo->fast_reg.fpo_pool_list)) {
> +		frd = list_first_entry(&fpo->fast_reg.fpo_pool_list,
> +				       struct kib_fast_reg_descriptor,
> +				       frd_list);
>  		list_del(&frd->frd_list);
>  		ib_dereg_mr(frd->frd_mr);
>  		kfree(frd);
> 
> 
> 

[lustre-devel] [PATCH 11/22] Revert "staging: lustre: lnet: lib-move: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit 24f695909440b79b04bb75205384c9772e4c7d0a.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/lnet/lib-move.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lnet/lnet/lib-move.c b/drivers/staging/lustre/lnet/lnet/lib-move.c
index e8092b1b6b27..2756e91b34bb 100644
--- a/drivers/staging/lustre/lnet/lnet/lib-move.c
+++ b/drivers/staging/lustre/lnet/lnet/lib-move.c
@@ -49,7 +49,6 @@ int
 lnet_fail_nid(lnet_nid_t nid, unsigned int threshold)
 {
 	struct lnet_test_peer *tp;
-	struct lnet_test_peer *temp;
 	struct list_head *el;
 	struct list_head *next;
 	struct list_head cull;
@@ -88,7 +87,9 @@ lnet_fail_nid(lnet_nid_t nid, unsigned int threshold)
 
 	lnet_net_unlock(0);
 
-	list_for_each_entry_safe(tp, temp, &cull, tp_list) {
+	while (!list_empty(&cull)) {
+		tp = list_entry(cull.next, struct lnet_test_peer, tp_list);
+
 		list_del(&tp->tp_list);
 		kfree(tp);
 	}
@@ -99,7 +100,6 @@ static int
 fail_peer(lnet_nid_t nid, int outgoing)
 {
 	struct lnet_test_peer *tp;
-	struct lnet_test_peer *temp;
 	struct list_head *el;
 	struct list_head *next;
 	struct list_head cull;
@@ -146,7 +146,8 @@ fail_peer(lnet_nid_t nid, int outgoing)
 
 	lnet_net_unlock(0);
 
-	list_for_each_entry_safe(tp, temp, &cull, tp_list) {
+	while (!list_empty(&cull)) {
+		tp = list_entry(cull.next, struct lnet_test_peer, tp_list);
 		list_del(&tp->tp_list);
 
 		kfree(tp);

[lustre-devel] [PATCH 11/22] Revert "staging: lustre: lnet: lib-move: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit 24f695909440b79b04bb75205384c9772e4c7d0a.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lnet/lnet/lib-move.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/lnet/lib-move.c b/drivers/staging/lustre/lnet/lnet/lib-move.c
> index e8092b1b6b27..2756e91b34bb 100644
> --- a/drivers/staging/lustre/lnet/lnet/lib-move.c
> +++ b/drivers/staging/lustre/lnet/lnet/lib-move.c
> @@ -49,7 +49,6 @@ int
>  lnet_fail_nid(lnet_nid_t nid, unsigned int threshold)
>  {
>  	struct lnet_test_peer *tp;
> -	struct lnet_test_peer *temp;
>  	struct list_head *el;
>  	struct list_head *next;
>  	struct list_head cull;
> @@ -88,7 +87,9 @@ lnet_fail_nid(lnet_nid_t nid, unsigned int threshold)
>  
>  	lnet_net_unlock(0);
>  
> -	list_for_each_entry_safe(tp, temp, &cull, tp_list) {
> +	while (!list_empty(&cull)) {
> +		tp = list_entry(cull.next, struct lnet_test_peer, tp_list);
> +
>  		list_del(&tp->tp_list);
>  		kfree(tp);
>  	}
> @@ -99,7 +100,6 @@ static int
>  fail_peer(lnet_nid_t nid, int outgoing)
>  {
>  	struct lnet_test_peer *tp;
> -	struct lnet_test_peer *temp;
>  	struct list_head *el;
>  	struct list_head *next;
>  	struct list_head cull;
> @@ -146,7 +146,8 @@ fail_peer(lnet_nid_t nid, int outgoing)
>  
>  	lnet_net_unlock(0);
>  
> -	list_for_each_entry_safe(tp, temp, &cull, tp_list) {
> +	while (!list_empty(&cull)) {
> +		tp = list_entry(cull.next, struct lnet_test_peer, tp_list);
>  		list_del(&tp->tp_list);
>  
>  		kfree(tp);
> 
> 
> 

[lustre-devel] [PATCH 10/22] Revert "staging: lustre: lnet: conrpc: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit a9a6cb4f4693253349358f8163d826eb0cfecfbc.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/selftest/conrpc.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c
index 95cbd1a14e1b..3afde0141db5 100644
--- a/drivers/staging/lustre/lnet/selftest/conrpc.c
+++ b/drivers/staging/lustre/lnet/selftest/conrpc.c
@@ -1327,7 +1327,6 @@ lstcon_rpc_cleanup_wait(void)
 {
 	struct lstcon_rpc_trans *trans;
 	struct lstcon_rpc *crpc;
-	struct lstcon_rpc *temp;
 	struct list_head *pacer;
 	struct list_head zlist;
 
@@ -1367,7 +1366,9 @@ lstcon_rpc_cleanup_wait(void)
 
 	spin_unlock(&console_session.ses_rpc_lock);
 
-	list_for_each_entry_safe(crpc, temp, &zlist, crp_link) {
+	while (!list_empty(&zlist)) {
+		crpc = list_entry(zlist.next, lstcon_rpc_t, crp_link);
+
 		list_del(&crpc->crp_link);
 		kfree(crpc);
 	}

[lustre-devel] [PATCH 10/22] Revert "staging: lustre: lnet: conrpc: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit a9a6cb4f4693253349358f8163d826eb0cfecfbc.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lnet/selftest/conrpc.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c
> index 95cbd1a14e1b..3afde0141db5 100644
> --- a/drivers/staging/lustre/lnet/selftest/conrpc.c
> +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c
> @@ -1327,7 +1327,6 @@ lstcon_rpc_cleanup_wait(void)
>  {
>  	struct lstcon_rpc_trans *trans;
>  	struct lstcon_rpc *crpc;
> -	struct lstcon_rpc *temp;
>  	struct list_head *pacer;
>  	struct list_head zlist;
>  
> @@ -1367,7 +1366,9 @@ lstcon_rpc_cleanup_wait(void)
>  
>  	spin_unlock(&console_session.ses_rpc_lock);
>  
> -	list_for_each_entry_safe(crpc, temp, &zlist, crp_link) {
> +	while (!list_empty(&zlist)) {
> +		crpc = list_entry(zlist.next, lstcon_rpc_t, crp_link);
> +

Nak. This one needs to be updated to. The typedef lstcon_rpc_t no longer
exist. Now you need to use struct lstcon_rpc.

>  		list_del(&crpc->crp_link);
>  		kfree(crpc);
>  	}
> 
> 
> 

[lustre-devel] [PATCH 10/22] Revert "staging: lustre: lnet: conrpc: Use list_for_each_entry_safe"

[NeilBrown]

On Thu, Aug 02 2018, James Simmons wrote:

>> This reverts commit a9a6cb4f4693253349358f8163d826eb0cfecfbc.
>> 
>> These loops really want to remove everything, and using a
>>   while(!list_empty())
>> loop makes this more obvious.
>> 
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>>  drivers/staging/lustre/lnet/selftest/conrpc.c |    5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c
>> index 95cbd1a14e1b..3afde0141db5 100644
>> --- a/drivers/staging/lustre/lnet/selftest/conrpc.c
>> +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c
>> @@ -1327,7 +1327,6 @@ lstcon_rpc_cleanup_wait(void)
>>  {
>>  	struct lstcon_rpc_trans *trans;
>>  	struct lstcon_rpc *crpc;
>> -	struct lstcon_rpc *temp;
>>  	struct list_head *pacer;
>>  	struct list_head zlist;
>>  
>> @@ -1367,7 +1366,9 @@ lstcon_rpc_cleanup_wait(void)
>>  
>>  	spin_unlock(&console_session.ses_rpc_lock);
>>  
>> -	list_for_each_entry_safe(crpc, temp, &zlist, crp_link) {
>> +	while (!list_empty(&zlist)) {
>> +		crpc = list_entry(zlist.next, lstcon_rpc_t, crp_link);
>> +
>
> Nak. This one needs to be updated to. The typedef lstcon_rpc_t no longer
> exist. Now you need to use struct lstcon_rpc.

Clearly I wasn't compiled with CONFIG_LNET_SELFTEST enabled.  I am now.
Thanks - I've added all your Reviewed-bys.

NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180803/dfa2a1d9/attachment.sig>

[lustre-devel] [PATCH 10/22] Revert "staging: lustre: lnet: conrpc: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit a9a6cb4f4693253349358f8163d826eb0cfecfbc.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lnet/selftest/conrpc.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c
> index 95cbd1a14e1b..3afde0141db5 100644
> --- a/drivers/staging/lustre/lnet/selftest/conrpc.c
> +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c
> @@ -1327,7 +1327,6 @@ lstcon_rpc_cleanup_wait(void)
>  {
>  	struct lstcon_rpc_trans *trans;
>  	struct lstcon_rpc *crpc;
> -	struct lstcon_rpc *temp;
>  	struct list_head *pacer;
>  	struct list_head zlist;
>  
> @@ -1367,7 +1366,9 @@ lstcon_rpc_cleanup_wait(void)
>  
>  	spin_unlock(&console_session.ses_rpc_lock);
>  
> -	list_for_each_entry_safe(crpc, temp, &zlist, crp_link) {
> +	while (!list_empty(&zlist)) {
> +		crpc = list_entry(zlist.next, lstcon_rpc_t, crp_link);
> +
>  		list_del(&crpc->crp_link);
>  		kfree(crpc);
>  	}
> 
> 
> 

[lustre-devel] [PATCH 20/22] lustre: lov_obd: convert list_for_each_entry_safe() to while(!list_empty())

[NeilBrown]

These loops are removing all elements from a list.
So using while(!list_empty()) makes the intent clearer.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/lov/lov_obd.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/lov/lov_obd.c b/drivers/staging/lustre/lustre/lov/lov_obd.c
index ee0898cbd9c9..0dd471cfe9a1 100644
--- a/drivers/staging/lustre/lustre/lov/lov_obd.c
+++ b/drivers/staging/lustre/lustre/lov/lov_obd.c
@@ -83,7 +83,7 @@ static void lov_putref(struct obd_device *obd)
 	if (atomic_dec_and_test(&lov->lov_refcount) && lov->lov_death_row) {
 		LIST_HEAD(kill);
 		int i;
-		struct lov_tgt_desc *tgt, *n;
+		struct lov_tgt_desc *tgt;
 
 		CDEBUG(D_CONFIG, "destroying %d lov targets\n",
 		       lov->lov_death_row);
@@ -103,7 +103,8 @@ static void lov_putref(struct obd_device *obd)
 		}
 		mutex_unlock(&lov->lov_lock);
 
-		list_for_each_entry_safe(tgt, n, &kill, ltd_kill) {
+		while (!list_empty(&kill)) {
+			tgt = list_first_entry(&kill, struct lov_tgt_desc, ltd_kill);
 			list_del(&tgt->ltd_kill);
 			/* Disconnect */
 			__lov_del_obd(obd, tgt);

[lustre-devel] [PATCH 20/22] lustre: lov_obd: convert list_for_each_entry_safe() to while(!list_empty())

[James Simmons]

> These loops are removing all elements from a list.
> So using while(!list_empty()) makes the intent clearer.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lustre/lov/lov_obd.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/lov/lov_obd.c b/drivers/staging/lustre/lustre/lov/lov_obd.c
> index ee0898cbd9c9..0dd471cfe9a1 100644
> --- a/drivers/staging/lustre/lustre/lov/lov_obd.c
> +++ b/drivers/staging/lustre/lustre/lov/lov_obd.c
> @@ -83,7 +83,7 @@ static void lov_putref(struct obd_device *obd)
>  	if (atomic_dec_and_test(&lov->lov_refcount) && lov->lov_death_row) {
>  		LIST_HEAD(kill);
>  		int i;
> -		struct lov_tgt_desc *tgt, *n;
> +		struct lov_tgt_desc *tgt;
>  
>  		CDEBUG(D_CONFIG, "destroying %d lov targets\n",
>  		       lov->lov_death_row);
> @@ -103,7 +103,8 @@ static void lov_putref(struct obd_device *obd)
>  		}
>  		mutex_unlock(&lov->lov_lock);
>  
> -		list_for_each_entry_safe(tgt, n, &kill, ltd_kill) {
> +		while (!list_empty(&kill)) {
> +			tgt = list_first_entry(&kill, struct lov_tgt_desc, ltd_kill);
>  			list_del(&tgt->ltd_kill);
>  			/* Disconnect */
>  			__lov_del_obd(obd, tgt);
> 
> 
> 

[lustre-devel] [PATCH 12/22] Revert "staging: lustre: obdclass: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit 6069f756d42160e454f49286183712514db3ca6b.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lustre/obdclass/lustre_peer.c   |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
index e286a2665423..7fc62b7e95b4 100644
--- a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
@@ -146,7 +146,6 @@ int class_del_uuid(const char *uuid)
 {
 	LIST_HEAD(deathrow);
 	struct uuid_nid_data *data;
-	struct uuid_nid_data *temp;
 
 	spin_lock(&g_uuid_lock);
 	if (uuid) {
@@ -169,7 +168,9 @@ int class_del_uuid(const char *uuid)
 		return -EINVAL;
 	}
 
-	list_for_each_entry_safe(data, temp, &deathrow, un_list) {
+	while (!list_empty(&deathrow)) {
+		data = list_entry(deathrow.next, struct uuid_nid_data,
+				  un_list);
 		list_del(&data->un_list);
 
 		CDEBUG(D_INFO, "del uuid %s %s/%d\n",

[lustre-devel] [PATCH 12/22] Revert "staging: lustre: obdclass: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit 6069f756d42160e454f49286183712514db3ca6b.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  .../staging/lustre/lustre/obdclass/lustre_peer.c   |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> index e286a2665423..7fc62b7e95b4 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> @@ -146,7 +146,6 @@ int class_del_uuid(const char *uuid)
>  {
>  	LIST_HEAD(deathrow);
>  	struct uuid_nid_data *data;
> -	struct uuid_nid_data *temp;
>  
>  	spin_lock(&g_uuid_lock);
>  	if (uuid) {
> @@ -169,7 +168,9 @@ int class_del_uuid(const char *uuid)
>  		return -EINVAL;
>  	}
>  
> -	list_for_each_entry_safe(data, temp, &deathrow, un_list) {
> +	while (!list_empty(&deathrow)) {
> +		data = list_entry(deathrow.next, struct uuid_nid_data,
> +				  un_list);
>  		list_del(&data->un_list);
>  
>  		CDEBUG(D_INFO, "del uuid %s %s/%d\n",
> 
> 
> 

[lustre-devel] [PATCH 16/22] lustre: tracefile: convert list_for_each_entry_safe() to while(!list_empty())

[NeilBrown]

These loops are removing all elements from a list.
So using while(!list_empty()) makes the intent clearer.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/libcfs/tracefile.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index 07242d4ed41b..a4768e930021 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -896,11 +896,12 @@ void cfs_trace_flush_pages(void)
 {
 	struct page_collection pc;
 	struct cfs_trace_page *tage;
-	struct cfs_trace_page *tmp;
 
 	pc.pc_want_daemon_pages = 1;
 	collect_pages(&pc);
-	list_for_each_entry_safe(tage, tmp, &pc.pc_pages, linkage) {
+	while (!list_empty(&pc.pc_pages)) {
+		tage = list_first_entry(&pc.pc_pages,
+					struct cfs_trace_page, linkage);
 		__LASSERT_TAGE_INVARIANT(tage);
 
 		list_del(&tage->linkage);
@@ -1331,7 +1332,6 @@ static void trace_cleanup_on_all_cpus(void)
 {
 	struct cfs_trace_cpu_data *tcd;
 	struct cfs_trace_page *tage;
-	struct cfs_trace_page *tmp;
 	int i, cpu;
 
 	for_each_possible_cpu(cpu) {
@@ -1341,8 +1341,10 @@ static void trace_cleanup_on_all_cpus(void)
 				continue;
 			tcd->tcd_shutting_down = 1;
 
-			list_for_each_entry_safe(tage, tmp, &tcd->tcd_pages,
-						 linkage) {
+			while (!list_empty(&tcd->tcd_pages)) {
+				tage = list_first_entry(&tcd->tcd_pages,
+							struct cfs_trace_page,
+							linkage);
 				__LASSERT_TAGE_INVARIANT(tage);
 
 				list_del(&tage->linkage);

[lustre-devel] [PATCH 16/22] lustre: tracefile: convert list_for_each_entry_safe() to while(!list_empty())

[James Simmons]

> These loops are removing all elements from a list.
> So using while(!list_empty()) makes the intent clearer.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lnet/libcfs/tracefile.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
> index 07242d4ed41b..a4768e930021 100644
> --- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
> +++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
> @@ -896,11 +896,12 @@ void cfs_trace_flush_pages(void)
>  {
>  	struct page_collection pc;
>  	struct cfs_trace_page *tage;
> -	struct cfs_trace_page *tmp;
>  
>  	pc.pc_want_daemon_pages = 1;
>  	collect_pages(&pc);
> -	list_for_each_entry_safe(tage, tmp, &pc.pc_pages, linkage) {
> +	while (!list_empty(&pc.pc_pages)) {
> +		tage = list_first_entry(&pc.pc_pages,
> +					struct cfs_trace_page, linkage);
>  		__LASSERT_TAGE_INVARIANT(tage);
>  
>  		list_del(&tage->linkage);
> @@ -1331,7 +1332,6 @@ static void trace_cleanup_on_all_cpus(void)
>  {
>  	struct cfs_trace_cpu_data *tcd;
>  	struct cfs_trace_page *tage;
> -	struct cfs_trace_page *tmp;
>  	int i, cpu;
>  
>  	for_each_possible_cpu(cpu) {
> @@ -1341,8 +1341,10 @@ static void trace_cleanup_on_all_cpus(void)
>  				continue;
>  			tcd->tcd_shutting_down = 1;
>  
> -			list_for_each_entry_safe(tage, tmp, &tcd->tcd_pages,
> -						 linkage) {
> +			while (!list_empty(&tcd->tcd_pages)) {
> +				tage = list_first_entry(&tcd->tcd_pages,
> +							struct cfs_trace_page,
> +							linkage);
>  				__LASSERT_TAGE_INVARIANT(tage);
>  
>  				list_del(&tage->linkage);
> 
> 
> 

[lustre-devel] [PATCH 14/22] Revert: Staging: lustre: Iterate list using list_for_each_entry

[NeilBrown]

This reverts (what is left of) 5a2ca43fa54f561c252c2ceb986daa49e258ab13

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
index 9cf1f64e3d76..bda67d49597d 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
@@ -2106,7 +2106,6 @@ kiblnd_connreq_done(struct kib_conn *conn, int status)
 {
 	struct kib_peer *peer = conn->ibc_peer;
 	struct kib_tx *tx;
-	struct kib_tx *tmp;
 	struct list_head txs;
 	unsigned long flags;
 	int active;
@@ -2197,7 +2196,8 @@ kiblnd_connreq_done(struct kib_conn *conn, int status)
 	 * scheduled.  We won't be using round robin on this first batch.
 	 */
 	spin_lock(&conn->ibc_lock);
-	list_for_each_entry_safe(tx, tmp, &txs, tx_list) {
+	while (!list_empty(&txs)) {
+		tx = list_first_entry(&txs, struct kib_tx, tx_list);
 		list_del(&tx->tx_list);
 
 		kiblnd_queue_tx_locked(tx, conn);
@@ -3185,7 +3185,6 @@ kiblnd_check_conns(int idx)
 	struct list_head *ptmp;
 	struct kib_peer *peer;
 	struct kib_conn *conn;
-	struct kib_conn *tmp;
 	struct list_head *ctmp;
 	unsigned long flags;
 
@@ -3241,7 +3240,8 @@ kiblnd_check_conns(int idx)
 	 * connection. We can only be sure RDMA activity
 	 * has ceased once the QP has been modified.
 	 */
-	list_for_each_entry_safe(conn, tmp, &closes, ibc_connd_list) {
+	while (!list_empty(&closes)) {
+		conn = list_first_entry(&closes, struct kib_conn, ibc_connd_list);
 		list_del(&conn->ibc_connd_list);
 		kiblnd_close_conn(conn, -ETIMEDOUT);
 		kiblnd_conn_decref(conn);

[lustre-devel] [PATCH 14/22] Revert: Staging: lustre: Iterate list using list_for_each_entry

[James Simmons]

> This reverts (what is left of) 5a2ca43fa54f561c252c2ceb986daa49e258ab13
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
> index 9cf1f64e3d76..bda67d49597d 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
> @@ -2106,7 +2106,6 @@ kiblnd_connreq_done(struct kib_conn *conn, int status)
>  {
>  	struct kib_peer *peer = conn->ibc_peer;
>  	struct kib_tx *tx;
> -	struct kib_tx *tmp;
>  	struct list_head txs;
>  	unsigned long flags;
>  	int active;
> @@ -2197,7 +2196,8 @@ kiblnd_connreq_done(struct kib_conn *conn, int status)
>  	 * scheduled.  We won't be using round robin on this first batch.
>  	 */
>  	spin_lock(&conn->ibc_lock);
> -	list_for_each_entry_safe(tx, tmp, &txs, tx_list) {
> +	while (!list_empty(&txs)) {
> +		tx = list_first_entry(&txs, struct kib_tx, tx_list);
>  		list_del(&tx->tx_list);
>  
>  		kiblnd_queue_tx_locked(tx, conn);
> @@ -3185,7 +3185,6 @@ kiblnd_check_conns(int idx)
>  	struct list_head *ptmp;
>  	struct kib_peer *peer;
>  	struct kib_conn *conn;
> -	struct kib_conn *tmp;
>  	struct list_head *ctmp;
>  	unsigned long flags;
>  
> @@ -3241,7 +3240,8 @@ kiblnd_check_conns(int idx)
>  	 * connection. We can only be sure RDMA activity
>  	 * has ceased once the QP has been modified.
>  	 */
> -	list_for_each_entry_safe(conn, tmp, &closes, ibc_connd_list) {
> +	while (!list_empty(&closes)) {
> +		conn = list_first_entry(&closes, struct kib_conn, ibc_connd_list);
>  		list_del(&conn->ibc_connd_list);
>  		kiblnd_close_conn(conn, -ETIMEDOUT);
>  		kiblnd_conn_decref(conn);
> 
> 
> 

[lustre-devel] [PATCH 13/22] Revert "staging: lustre: lnet: Use list_for_each_entry_safe"

[NeilBrown]

This reverts commit 0daec763260e4f0d8038512b72971ddbcf1c63a1.

These loops really want to remove everything, and using a
  while(!list_empty())
loop makes this more obvious.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
index 3f69618ad85b..d531847305e4 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
@@ -2260,12 +2260,13 @@ static inline void
 ksocknal_flush_stale_txs(struct ksock_peer *peer)
 {
 	struct ksock_tx *tx;
-	struct ksock_tx *tmp;
 	LIST_HEAD(stale_txs);
 
 	write_lock_bh(&ksocknal_data.ksnd_global_lock);
 
-	list_for_each_entry_safe(tx, tmp, &peer->ksnp_tx_queue, tx_list) {
+	while (!list_empty(&peer->ksnp_tx_queue)) {
+		tx = list_entry(peer->ksnp_tx_queue.next, struct ksock_tx, tx_list);
+
 		if (ktime_get_seconds() < tx->tx_deadline)
 			break;
 

[lustre-devel] [PATCH 13/22] Revert "staging: lustre: lnet: Use list_for_each_entry_safe"

[James Simmons]

> This reverts commit 0daec763260e4f0d8038512b72971ddbcf1c63a1.
> 
> These loops really want to remove everything, and using a
>   while(!list_empty())
> loop makes this more obvious.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
> index 3f69618ad85b..d531847305e4 100644
> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
> @@ -2260,12 +2260,13 @@ static inline void
>  ksocknal_flush_stale_txs(struct ksock_peer *peer)
>  {
>  	struct ksock_tx *tx;
> -	struct ksock_tx *tmp;
>  	LIST_HEAD(stale_txs);
>  
>  	write_lock_bh(&ksocknal_data.ksnd_global_lock);
>  
> -	list_for_each_entry_safe(tx, tmp, &peer->ksnp_tx_queue, tx_list) {
> +	while (!list_empty(&peer->ksnp_tx_queue)) {
> +		tx = list_entry(peer->ksnp_tx_queue.next, struct ksock_tx, tx_list);
> +
>  		if (ktime_get_seconds() < tx->tx_deadline)
>  			break;
>  
> 
> 
> 

[lustre-devel] [PATCH 18/22] lustre: net_fault: convert list_for_each_entry_safe() to while(!list_empty())

[NeilBrown]

These loops are removing all elements from a list.
So using while(!list_empty()) makes the intent clearer.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lnet/lnet/net_fault.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/lnet/net_fault.c b/drivers/staging/lustre/lnet/lnet/net_fault.c
index cb1f9053ef48..41d6131ee15a 100644
--- a/drivers/staging/lustre/lnet/lnet/net_fault.c
+++ b/drivers/staging/lustre/lnet/lnet/net_fault.c
@@ -216,7 +216,8 @@ lnet_drop_rule_del(lnet_nid_t src, lnet_nid_t dst)
 	}
 	lnet_net_unlock(LNET_LOCK_EX);
 
-	list_for_each_entry_safe(rule, tmp, &zombies, dr_link) {
+	while (!list_empty(&zombies)) {
+		rule = list_first_entry(&zombies, struct lnet_drop_rule, dr_link);
 		CDEBUG(D_NET, "Remove drop rule: src %s->dst: %s (1/%d, %d)\n",
 		       libcfs_nid2str(rule->dr_attr.fa_src),
 		       libcfs_nid2str(rule->dr_attr.fa_dst),
@@ -841,7 +842,9 @@ lnet_delay_rule_del(lnet_nid_t src, lnet_nid_t dst, bool shutdown)
 		  !list_empty(&rule_list);
 	lnet_net_unlock(LNET_LOCK_EX);
 
-	list_for_each_entry_safe(rule, tmp, &rule_list, dl_link) {
+	while (!list_empty(&rule_list)) {
+		rule = list_first_entry(&rule_list,
+					struct lnet_delay_rule, dl_link);
 		list_del_init(&rule->dl_link);
 
 		del_timer_sync(&rule->dl_timer);

[lustre-devel] [PATCH 18/22] lustre: net_fault: convert list_for_each_entry_safe() to while(!list_empty())

[James Simmons]

> These loops are removing all elements from a list.
> So using while(!list_empty()) makes the intent clearer.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lnet/lnet/net_fault.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/lnet/net_fault.c b/drivers/staging/lustre/lnet/lnet/net_fault.c
> index cb1f9053ef48..41d6131ee15a 100644
> --- a/drivers/staging/lustre/lnet/lnet/net_fault.c
> +++ b/drivers/staging/lustre/lnet/lnet/net_fault.c
> @@ -216,7 +216,8 @@ lnet_drop_rule_del(lnet_nid_t src, lnet_nid_t dst)
>  	}
>  	lnet_net_unlock(LNET_LOCK_EX);
>  
> -	list_for_each_entry_safe(rule, tmp, &zombies, dr_link) {
> +	while (!list_empty(&zombies)) {
> +		rule = list_first_entry(&zombies, struct lnet_drop_rule, dr_link);
>  		CDEBUG(D_NET, "Remove drop rule: src %s->dst: %s (1/%d, %d)\n",
>  		       libcfs_nid2str(rule->dr_attr.fa_src),
>  		       libcfs_nid2str(rule->dr_attr.fa_dst),
> @@ -841,7 +842,9 @@ lnet_delay_rule_del(lnet_nid_t src, lnet_nid_t dst, bool shutdown)
>  		  !list_empty(&rule_list);
>  	lnet_net_unlock(LNET_LOCK_EX);
>  
> -	list_for_each_entry_safe(rule, tmp, &rule_list, dl_link) {
> +	while (!list_empty(&rule_list)) {
> +		rule = list_first_entry(&rule_list,
> +					struct lnet_delay_rule, dl_link);
>  		list_del_init(&rule->dl_link);
>  
>  		del_timer_sync(&rule->dl_timer);
> 
> 
> 

[lustre-devel] [PATCH 19/22] lustre: fld_request: convert list_for_each_entry_safe() to while(!list_empty())

[NeilBrown]

These loops are removing all elements from a list.
So using while(!list_empty()) makes the intent clearer.


Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/fld/fld_request.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/fld/fld_request.c b/drivers/staging/lustre/lustre/fld/fld_request.c
index 97f7ea632346..7b0365b3e413 100644
--- a/drivers/staging/lustre/lustre/fld/fld_request.c
+++ b/drivers/staging/lustre/lustre/fld/fld_request.c
@@ -280,10 +280,12 @@ EXPORT_SYMBOL(fld_client_init);
 
 void fld_client_fini(struct lu_client_fld *fld)
 {
-	struct lu_fld_target *target, *tmp;
+	struct lu_fld_target *target;
 
 	spin_lock(&fld->lcf_lock);
-	list_for_each_entry_safe(target, tmp, &fld->lcf_targets, ft_chain) {
+	while (!list_empty(&fld->lcf_targets)) {
+		target = list_first_entry(&fld->lcf_targets,
+					  struct lu_fld_target, ft_chain);
 		fld->lcf_count--;
 		list_del(&target->ft_chain);
 		if (target->ft_exp)

[lustre-devel] [PATCH 19/22] lustre: fld_request: convert list_for_each_entry_safe() to while(!list_empty())

[James Simmons]

> These loops are removing all elements from a list.
> So using while(!list_empty()) makes the intent clearer.
> 

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lustre/fld/fld_request.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/fld/fld_request.c b/drivers/staging/lustre/lustre/fld/fld_request.c
> index 97f7ea632346..7b0365b3e413 100644
> --- a/drivers/staging/lustre/lustre/fld/fld_request.c
> +++ b/drivers/staging/lustre/lustre/fld/fld_request.c
> @@ -280,10 +280,12 @@ EXPORT_SYMBOL(fld_client_init);
>  
>  void fld_client_fini(struct lu_client_fld *fld)
>  {
> -	struct lu_fld_target *target, *tmp;
> +	struct lu_fld_target *target;
>  
>  	spin_lock(&fld->lcf_lock);
> -	list_for_each_entry_safe(target, tmp, &fld->lcf_targets, ft_chain) {
> +	while (!list_empty(&fld->lcf_targets)) {
> +		target = list_first_entry(&fld->lcf_targets,
> +					  struct lu_fld_target, ft_chain);
>  		fld->lcf_count--;
>  		list_del(&target->ft_chain);
>  		if (target->ft_exp)
> 
> 
> 

[lustre-devel] [PATCH 00/22] Lustre: use while loop when emptying a list

[Andreas Dilger]

On Jul 29, 2018, at 21:37, NeilBrown <neilb@suse.com> wrote:
> 
> If you have a list that needs to be emptied, it is best to have a loop
> like
>  while (!list_empty(...))
> 
> because then it is obvious what the purpose of the loop is.
> Many places in lustre use
>  list_for_each_entry_safe()
> instead, which obscures the purpose.
> Several of these were from patches which deliberately converted from
> the while loop the list list_for_each_entry_safe() loop, at least some
> of which introduced real bugs.
> 
> This series reverts all those patches, and then makes other
> conversions.
> There are still several places which could be converted, but I got
> bored...
> I've particularly converted all where the list_head is a local
> variable. In these cases it is obviously wrong not to empty the
> list completely.
> 
> For those conversions that I did manual (not reverts) I use
>  list_first_entry()
> to get the first entry, rather then list_entry(head->next).
> Others could be converted as well.
> 
> NeilBrown

Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

> ---
> 
> NeilBrown (22):
>      Revert "staging: lustre: lnet: api-ni: Use list_for_each_entry_safe"
>      Revert "staging: lustre: o2iblnd: Use list_for_each_entry_safe in kiblnd_destroy_fmr_pool_list"
>      Revert "staging: lustre: lnet: o2iblnd: Use list_for_each_entry_safe"
>      Revert "staging: lustre: lnet: socklnd: Use list_for_each_entry_safe"
>      Revert "staging: lustre: lnet: socklnd_proto: Use list_for_each_entry_safe"
>      Revert "staging: lustre: osc_cache: Use list_for_each_entry_safe"
>      Revert "staging: lustre: lnet: peer: Use list_for_each_entry_safe"
>      Revert "staging: lustre: lnet: config: Use list_for_each_entry_safe"
>      Revert "staging: lustre: lnet: router: Use list_for_each_entry_safe"
>      Revert "staging: lustre: lnet: conrpc: Use list_for_each_entry_safe"
>      Revert "staging: lustre: lnet: lib-move: Use list_for_each_entry_safe"
>      Revert "staging: lustre: obdclass: Use list_for_each_entry_safe"
>      Revert "staging: lustre: lnet: Use list_for_each_entry_safe"
>      Revert: Staging: lustre: Iterate list using list_for_each_entry
>      lustre: o2iblnd: convert list_for_each_entry_safe() to while(!list_empty())
>      lustre: tracefile: convert list_for_each_entry_safe() to while(!list_empty())
>      lustre: lib-move:  convert list_for_each_entry_safe() to while(!list_empty())
>      lustre: net_fault: convert list_for_each_entry_safe() to while(!list_empty())
>      lustre: fld_request: convert list_for_each_entry_safe() to while(!list_empty())
>      lustre: lov_obd: convert list_for_each_entry_safe() to while(!list_empty())
>      lustre: ldlm_request: convert list_for_each_entry_safe() to while(!list_empty())
>      lustre: sec_config: convert list_for_each_entry_safe() to while(!list_empty())
> 
> 
> .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |   21 ++++++++++++--------
> .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |   12 ++++++-----
> .../staging/lustre/lnet/klnds/socklnd/socklnd.c    |    9 +++++----
> .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |    5 +++--
> .../lustre/lnet/klnds/socklnd/socklnd_proto.c      |    4 ++--
> drivers/staging/lustre/lnet/libcfs/tracefile.c     |   12 +++++++----
> drivers/staging/lustre/lnet/lnet/api-ni.c          |   10 ++++++----
> drivers/staging/lustre/lnet/lnet/config.c          |    5 +++--
> drivers/staging/lustre/lnet/lnet/lib-move.c        |   13 +++++++-----
> drivers/staging/lustre/lnet/lnet/net_fault.c       |    7 +++++--
> drivers/staging/lustre/lnet/lnet/peer.c            |    4 ++--
> drivers/staging/lustre/lnet/lnet/router.c          |    4 ++--
> drivers/staging/lustre/lnet/selftest/conrpc.c      |    5 +++--
> drivers/staging/lustre/lustre/fld/fld_request.c    |    6 ++++--
> drivers/staging/lustre/lustre/ldlm/ldlm_request.c  |    6 ++++--
> drivers/staging/lustre/lustre/lov/lov_obd.c        |    5 +++--
> .../staging/lustre/lustre/obdclass/lustre_peer.c   |    5 +++--
> drivers/staging/lustre/lustre/osc/osc_cache.c      |    9 +++++----
> drivers/staging/lustre/lustre/ptlrpc/sec_config.c  |    7 ++++---
> 19 files changed, 87 insertions(+), 62 deletions(-)
> 
> --
> Signature
> 

Cheers, Andreas
---
Andreas Dilger
CTO Whamcloud




-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 235 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180730/d82a84f4/attachment-0001.sig>

[lustre-devel] [PATCH 00/22] Lustre: use while loop when emptying a list

[James Simmons]

> If you have a list that needs to be emptied, it is best to have a loop
> like
>   while (!list_empty(...))
> 
> because then it is obvious what the purpose of the loop is.
> Many places in lustre use
>   list_for_each_entry_safe()
> instead, which obscures the purpose.
> Several of these were from patches which deliberately converted from
> the while loop the list list_for_each_entry_safe() loop, at least some
> of which introduced real bugs.
> 
> This series reverts all those patches, and then makes other
> conversions.
> There are still several places which could be converted, but I got
> bored...
> I've particularly converted all where the list_head is a local
> variable. In these cases it is obviously wrong not to empty the
> list completely.
> 
> For those conversions that I did manual (not reverts) I use
>   list_first_entry()
> to get the first entry, rather then list_entry(head->next).
> Others could be converted as well.

So much heart burn with these patches. Nice to see them reverted.
I have tested this patch series but I like Doug or another Lnet
to review these patches as well.

> 
> NeilBrown
> 
> 
> ---
> 
> NeilBrown (22):
>       Revert "staging: lustre: lnet: api-ni: Use list_for_each_entry_safe"
>       Revert "staging: lustre: o2iblnd: Use list_for_each_entry_safe in kiblnd_destroy_fmr_pool_list"
>       Revert "staging: lustre: lnet: o2iblnd: Use list_for_each_entry_safe"
>       Revert "staging: lustre: lnet: socklnd: Use list_for_each_entry_safe"
>       Revert "staging: lustre: lnet: socklnd_proto: Use list_for_each_entry_safe"
>       Revert "staging: lustre: osc_cache: Use list_for_each_entry_safe"
>       Revert "staging: lustre: lnet: peer: Use list_for_each_entry_safe"
>       Revert "staging: lustre: lnet: config: Use list_for_each_entry_safe"
>       Revert "staging: lustre: lnet: router: Use list_for_each_entry_safe"
>       Revert "staging: lustre: lnet: conrpc: Use list_for_each_entry_safe"
>       Revert "staging: lustre: lnet: lib-move: Use list_for_each_entry_safe"
>       Revert "staging: lustre: obdclass: Use list_for_each_entry_safe"
>       Revert "staging: lustre: lnet: Use list_for_each_entry_safe"
>       Revert: Staging: lustre: Iterate list using list_for_each_entry
>       lustre: o2iblnd: convert list_for_each_entry_safe() to while(!list_empty())
>       lustre: tracefile: convert list_for_each_entry_safe() to while(!list_empty())
>       lustre: lib-move:  convert list_for_each_entry_safe() to while(!list_empty())
>       lustre: net_fault: convert list_for_each_entry_safe() to while(!list_empty())
>       lustre: fld_request: convert list_for_each_entry_safe() to while(!list_empty())
>       lustre: lov_obd: convert list_for_each_entry_safe() to while(!list_empty())
>       lustre: ldlm_request: convert list_for_each_entry_safe() to while(!list_empty())
>       lustre: sec_config: convert list_for_each_entry_safe() to while(!list_empty())
> 
> 
>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |   21 ++++++++++++--------
>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |   12 ++++++-----
>  .../staging/lustre/lnet/klnds/socklnd/socklnd.c    |    9 +++++----
>  .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |    5 +++--
>  .../lustre/lnet/klnds/socklnd/socklnd_proto.c      |    4 ++--
>  drivers/staging/lustre/lnet/libcfs/tracefile.c     |   12 +++++++----
>  drivers/staging/lustre/lnet/lnet/api-ni.c          |   10 ++++++----
>  drivers/staging/lustre/lnet/lnet/config.c          |    5 +++--
>  drivers/staging/lustre/lnet/lnet/lib-move.c        |   13 +++++++-----
>  drivers/staging/lustre/lnet/lnet/net_fault.c       |    7 +++++--
>  drivers/staging/lustre/lnet/lnet/peer.c            |    4 ++--
>  drivers/staging/lustre/lnet/lnet/router.c          |    4 ++--
>  drivers/staging/lustre/lnet/selftest/conrpc.c      |    5 +++--
>  drivers/staging/lustre/lustre/fld/fld_request.c    |    6 ++++--
>  drivers/staging/lustre/lustre/ldlm/ldlm_request.c  |    6 ++++--
>  drivers/staging/lustre/lustre/lov/lov_obd.c        |    5 +++--
>  .../staging/lustre/lustre/obdclass/lustre_peer.c   |    5 +++--
>  drivers/staging/lustre/lustre/osc/osc_cache.c      |    9 +++++----
>  drivers/staging/lustre/lustre/ptlrpc/sec_config.c  |    7 ++++---
>  19 files changed, 87 insertions(+), 62 deletions(-)
> 
> --
> Signature
> 
>