staging: lustre: lnet: Replace list_for_each with list_for_each_entry

PATCH In review Haneen Mohammed 2 participants 6 messages Sep 16, 2017 Staging

Share & tools

Haneen Mohammed

author

2017-09-16 00:41 UTC #0 lore raw

PATCH 1/1 staging: lustre: lnet: Replace list_for_each with list_for_each_entry

Replace use of the combination of list_for_each and list_entry with list_for_each_entry to simplify the code and remove variables that are used only in list_for_each.
Issue found and corrected using Coccinelle script:

@r@
expression head, member, e;
type T1, T2, T3;
iterator name list_for_each, list_for_each_entry;
identifier pos, var;
@@

-T1 *pos;
...when!=pos=e;

-list_for_each(pos, head)
+list_for_each_entry(var, head, member)
{
...when!=pos=e;
when!=T3 *var;
-var = list_entry(pos, T2, member);
...when!=pos=e;
}
...when!=pos=e;

Signed-off-by Haneen Mohammed <[email protected]>

---
 drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

drivers/staging/lustre/lnet/lnet/router.c +2 -7

index 3df101b..b8eba33 100644
@@ -222,15 +222,12 @@ struct lnet_remotenet *
 lnet_find_net_locked(__u32 net)
 {
 	struct lnet_remotenet *rnet;
-	struct list_head *tmp;
 	struct list_head *rn_list;
 
 	LASSERT(!the_lnet.ln_shutdown);
 
 	rn_list = lnet_net2rnethash(net);
-	list_for_each(tmp, rn_list) {
-		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
-
+	list_for_each_entry(rnet, rn_list, lrn_list) {
 		if (rnet->lrn_net == net)
 			return rnet;
 	}

@@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
 	__u32 lnd_type, seed[2];
 	struct timespec64 ts;
 	struct lnet_ni *ni;
-	struct list_head *tmp;
 
 	if (seeded)
 		return;

@@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
 	 * Nodes with small feet have little entropy
 	 * the NID for this node gives the most entropy in the low bits
 	 */
-	list_for_each(tmp, &the_lnet.ln_nis) {
-		ni = list_entry(tmp, struct lnet_ni, ni_list);
+	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
 		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
 
 		if (lnd_type != LOLND)

Signature · 1 line

2.7.4

Julia Lawall

Acked-by on v1 1/1

2017-09-16 11:23 UTC #1 lore raw

Quoted 18 lines · Haneen Mohammed

Replace use of the combination of list_for_each and list_entry with list_for_each_entry to simplify the code and remove variables that are used only in list_for_each.
Issue found and corrected using Coccinelle script:
@r@
expression head, member, e;
type T1, T2, T3;
iterator name list_for_each, list_for_each_entry;
identifier pos, var;
@@
-T1 *pos;
...when!=pos=e;
(quoted diff) +1 -1
-list_for_each(pos, head)
+list_for_each_entry(var, head, member)
{

Quoted 6 lines

...when!=pos=e;
   when!=T3 *var;
-var = list_entry(pos, T2, member);
...when!=pos=e;
}
...when!=pos=e;

Actually, one could consider that there should be when != pos, not when != pos=e, because you need that there are no references at all to pos to be able to delete it. But it's true that if pos is not initialized then there should be no other kinds of references either. The other possibility for initialization would be eg

f(...,&pos,...)

You could be suspicious of &pos in general. But anyway there is no &pos in this code.

julia

Quoted 1 line

Signed-off-by Haneen Mohammed <[email protected]>

Acked-by Julia Lawall <[email protected]>

Quoted 53 lines

---
 drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

drivers/staging/lustre/lnet/lnet/router.c +2 -8

index 3df101b..b8eba33 100644
@@ -222,15 +222,12 @@ struct lnet_remotenet *
 lnet_find_net_locked(__u32 net)
 {
 	struct lnet_remotenet *rnet;
-	struct list_head *tmp;
 	struct list_head *rn_list;
 
 	LASSERT(!the_lnet.ln_shutdown);
 
 	rn_list = lnet_net2rnethash(net);
-	list_for_each(tmp, rn_list) {
-		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
-
+	list_for_each_entry(rnet, rn_list, lrn_list) {
 		if (rnet->lrn_net == net)
 			return rnet;
 	}

@@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
 	__u32 lnd_type, seed[2];
 	struct timespec64 ts;
 	struct lnet_ni *ni;
-	struct list_head *tmp;
 
 	if (seeded)
 		return;

@@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
 	 * Nodes with small feet have little entropy
 	 * the NID for this node gives the most entropy in the low bits
 	 */
-	list_for_each(tmp, &the_lnet.ln_nis) {
-		ni = list_entry(tmp, struct lnet_ni, ni_list);
+	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
 		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
 
 		if (lnd_type != LOLND)
--

2.7.4

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe at googlegroups.com. To post to this group, send email to outreachy-kernel at googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170916004148.GA25693%40Haneen.
For more options, visit https://groups.google.com/d/optout.

Haneen Mohammed

author

2017-09-16 16:10 UTC #2 lore raw

Quoted 35 lines · Julia Lawall

Quoted 18 lines · Haneen Mohammed
Replace use of the combination of list_for_each and list_entry with list_for_each_entry to simplify the code and remove variables that are used only in list_for_each.
Issue found and corrected using Coccinelle script:
@r@
expression head, member, e;
type T1, T2, T3;
iterator name list_for_each, list_for_each_entry;
identifier pos, var;
@@
-T1 *pos;
...when!=pos=e;
(quoted diff) +1 -1
-list_for_each(pos, head)
+list_for_each_entry(var, head, member)
{
Quoted 6 lines
...when!=pos=e;
   when!=T3 *var;
-var = list_entry(pos, T2, member);
...when!=pos=e;
}
...when!=pos=e;
Actually, one could consider that there should be when != pos, not when != pos=e, because you need that there are no references at all to pos to be able to delete it. But it's true that if pos is not initialized then there should be no other kinds of references either. The other possibility for initialization would be eg

Quoted 7 lines

f(...,&pos,...)
You could be suspicious of &pos in general. But anyway there is no &pos in this code.
julia

I have tried "when != pos;" first and switched to "when !=pos=e;" becuase I have found cases where pos was still used. I only realised I should have used "when != pos" without the semicolon after I've submitted the patch.

There is still case where even with "when!=pos" the script fails to match pos within an if statement. Is there away around that?

Quoted 57 lines

Quoted 1 line

Signed-off-by Haneen Mohammed <[email protected]>

Acked-by Julia Lawall <[email protected]>

Quoted 53 lines

---
 drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

drivers/staging/lustre/lnet/lnet/router.c +2 -8

index 3df101b..b8eba33 100644
@@ -222,15 +222,12 @@ struct lnet_remotenet *
 lnet_find_net_locked(__u32 net)
 {
 	struct lnet_remotenet *rnet;
-	struct list_head *tmp;
 	struct list_head *rn_list;
 
 	LASSERT(!the_lnet.ln_shutdown);
 
 	rn_list = lnet_net2rnethash(net);
-	list_for_each(tmp, rn_list) {
-		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
-
+	list_for_each_entry(rnet, rn_list, lrn_list) {
 		if (rnet->lrn_net == net)
 			return rnet;
 	}

@@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
 	__u32 lnd_type, seed[2];
 	struct timespec64 ts;
 	struct lnet_ni *ni;
-	struct list_head *tmp;
 
 	if (seeded)
 		return;

@@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
 	 * Nodes with small feet have little entropy
 	 * the NID for this node gives the most entropy in the low bits
 	 */
-	list_for_each(tmp, &the_lnet.ln_nis) {
-		ni = list_entry(tmp, struct lnet_ni, ni_list);
+	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
 		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
 
 		if (lnd_type != LOLND)
--

2.7.4

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170916004148.GA25693%40Haneen.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2017-09-16 16:14 UTC #3 lore raw

Quoted 44 lines · Haneen Mohammed

Quoted 35 lines · Julia Lawall
Quoted 18 lines · Haneen Mohammed
Replace use of the combination of list_for_each and list_entry with list_for_each_entry to simplify the code and remove variables that are used only in list_for_each.
Issue found and corrected using Coccinelle script:
@r@
expression head, member, e;
type T1, T2, T3;
iterator name list_for_each, list_for_each_entry;
identifier pos, var;
@@
-T1 *pos;
...when!=pos=e;
(quoted diff) +1 -1
-list_for_each(pos, head)
+list_for_each_entry(var, head, member)
{
Quoted 6 lines
...when!=pos=e;
   when!=T3 *var;
-var = list_entry(pos, T2, member);
...when!=pos=e;
}
...when!=pos=e;
Actually, one could consider that there should be when != pos, not when != pos=e, because you need that there are no references at all to pos to be able to delete it. But it's true that if pos is not initialized then there should be no other kinds of references either. The other possibility for initialization would be eg
Quoted 7 lines
f(...,&pos,...)
You could be suspicious of &pos in general. But anyway there is no &pos in this code.
julia

Quoted 7 lines

I have tried "when != pos;" first and switched to "when !=pos=e;" becuase I have found cases where pos was still used. I only realised I should have used "when != pos" without the semicolon after I've submitted the patch.
There is still case where even with "when!=pos" the script fails to match pos within an if statement. Is there away around that?

Could you send an example?

julia

Quoted 66 lines

Quoted 51 lines
Quoted 2 lines
Signed-off-by Haneen Mohammed <[email protected]>
Acked-by Julia Lawall <[email protected]>
Quoted 46 lines
---
 drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/staging/lustre/lnet/lnet/router.c +2 -8
index 3df101b..b8eba33 100644
@@ -222,15 +222,12 @@ struct lnet_remotenet *
 lnet_find_net_locked(__u32 net)
 {
 	struct lnet_remotenet *rnet;
-	struct list_head *tmp;
 	struct list_head *rn_list;
 
 	LASSERT(!the_lnet.ln_shutdown);
 
 	rn_list = lnet_net2rnethash(net);
-	list_for_each(tmp, rn_list) {
-		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
-
+	list_for_each_entry(rnet, rn_list, lrn_list) {
 		if (rnet->lrn_net == net)
 			return rnet;
 	}
@@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
 	__u32 lnd_type, seed[2];
 	struct timespec64 ts;
 	struct lnet_ni *ni;
-	struct list_head *tmp;
 
 	if (seeded)
 		return;
@@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
 	 * Nodes with small feet have little entropy
 	 * the NID for this node gives the most entropy in the low bits
 	 */
-	list_for_each(tmp, &the_lnet.ln_nis) {
-		ni = list_entry(tmp, struct lnet_ni, ni_list);
+	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
 		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
 
 		if (lnd_type != LOLND)
--
2.7.4
Quoted 7 lines
Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170916004148.GA25693%40Haneen.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170916161048.GA6020%40Haneen.
For more options, visit https://groups.google.com/d/optout.

Haneen Mohammed

author

2017-09-16 16:21 UTC #4 lore raw

Quoted 53 lines · Julia Lawall

Quoted 49 lines · Haneen Mohammed
Quoted 35 lines · Julia Lawall
Quoted 18 lines · Haneen Mohammed
Replace use of the combination of list_for_each and list_entry with list_for_each_entry to simplify the code and remove variables that are used only in list_for_each.
Issue found and corrected using Coccinelle script:
@r@
expression head, member, e;
type T1, T2, T3;
iterator name list_for_each, list_for_each_entry;
identifier pos, var;
@@
-T1 *pos;
...when!=pos=e;
(quoted diff) +1 -1
-list_for_each(pos, head)
+list_for_each_entry(var, head, member)
{
Quoted 6 lines
...when!=pos=e;
   when!=T3 *var;
-var = list_entry(pos, T2, member);
...when!=pos=e;
}
...when!=pos=e;
Actually, one could consider that there should be when != pos, not when != pos=e, because you need that there are no references at all to pos to be able to delete it. But it's true that if pos is not initialized then there should be no other kinds of references either. The other possibility for initialization would be eg
Quoted 7 lines
f(...,&pos,...)
You could be suspicious of &pos in general. But anyway there is no &pos in this code.
julia
I have tried "when != pos;" first and switched to "when !=pos=e;" becuase I have found cases where pos was still used. I only realised I should have used "when != pos" without the semicolon after I've submitted the patch.

Quoted 8 lines

Quoted 2 lines
There is still case where even with "when!=pos" the script fails to match pos within an if statement. Is there away around that?
Could you send an example?
julia

in staging/lustre/lustre/ldlm/ldm_lock.c, line 898:

list_for_each(tmp, queue) {
	lock = list_entry(tmp, struct ldlm_lock, l_res_link);

	mode_end = list_prev_entry(lock, l_sl_mode);

	if (lock->l_req_mode != req->l_req_mode) {
		/* jump to last lock of mode group */
		tmp = &mode_end->l_res_link;
		continue;
	}

Quoted 66 lines

Quoted 66 lines
Quoted 51 lines
Quoted 2 lines
Signed-off-by Haneen Mohammed <[email protected]>
Acked-by Julia Lawall <[email protected]>
Quoted 46 lines
---
 drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/staging/lustre/lnet/lnet/router.c +2 -8
index 3df101b..b8eba33 100644
@@ -222,15 +222,12 @@ struct lnet_remotenet *
 lnet_find_net_locked(__u32 net)
 {
 	struct lnet_remotenet *rnet;
-	struct list_head *tmp;
 	struct list_head *rn_list;
 
 	LASSERT(!the_lnet.ln_shutdown);
 
 	rn_list = lnet_net2rnethash(net);
-	list_for_each(tmp, rn_list) {
-		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
-
+	list_for_each_entry(rnet, rn_list, lrn_list) {
 		if (rnet->lrn_net == net)
 			return rnet;
 	}
@@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
 	__u32 lnd_type, seed[2];
 	struct timespec64 ts;
 	struct lnet_ni *ni;
-	struct list_head *tmp;
 
 	if (seeded)
 		return;
@@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
 	 * Nodes with small feet have little entropy
 	 * the NID for this node gives the most entropy in the low bits
 	 */
-	list_for_each(tmp, &the_lnet.ln_nis) {
-		ni = list_entry(tmp, struct lnet_ni, ni_list);
+	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
 		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
 
 		if (lnd_type != LOLND)
--
2.7.4
Quoted 7 lines
Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170916004148.GA25693%40Haneen.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170916161048.GA6020%40Haneen.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2017-09-16 17:04 UTC #5 lore raw

Quoted 65 lines · Haneen Mohammed

Quoted 53 lines · Julia Lawall
Quoted 49 lines · Haneen Mohammed
Quoted 35 lines · Julia Lawall
Quoted 18 lines · Haneen Mohammed
Replace use of the combination of list_for_each and list_entry with list_for_each_entry to simplify the code and remove variables that are used only in list_for_each.
Issue found and corrected using Coccinelle script:
@r@
expression head, member, e;
type T1, T2, T3;
iterator name list_for_each, list_for_each_entry;
identifier pos, var;
@@
-T1 *pos;
...when!=pos=e;
(quoted diff) +1 -1
-list_for_each(pos, head)
+list_for_each_entry(var, head, member)
{
Quoted 6 lines
...when!=pos=e;
   when!=T3 *var;
-var = list_entry(pos, T2, member);
...when!=pos=e;
}
...when!=pos=e;
Actually, one could consider that there should be when != pos, not when != pos=e, because you need that there are no references at all to pos to be able to delete it. But it's true that if pos is not initialized then there should be no other kinds of references either. The other possibility for initialization would be eg
Quoted 7 lines
f(...,&pos,...)
You could be suspicious of &pos in general. But anyway there is no &pos in this code.
julia
I have tried "when != pos;" first and switched to "when !=pos=e;" becuase I have found cases where pos was still used. I only realised I should have used "when != pos" without the semicolon after I've submitted the patch.
Quoted 8 lines
Quoted 2 lines
There is still case where even with "when!=pos" the script fails to match pos within an if statement. Is there away around that?
Could you send an example?
julia
in staging/lustre/lustre/ldlm/ldm_lock.c, line 898:

Quoted 10 lines

list_for_each(tmp, queue) {
	lock = list_entry(tmp, struct ldlm_lock, l_res_link);

	mode_end = list_prev_entry(lock, l_sl_mode);

	if (lock->l_req_mode != req->l_req_mode) {
		/* jump to last lock of mode group */
		tmp = &mode_end->l_res_link;
		continue;
	}

Indeed, it should not match that. Perhaps the problem is because of the continue. Thanks for the report.

julia

Quoted 74 lines

Quoted 51 lines
Quoted 51 lines
Quoted 51 lines
Quoted 2 lines
Signed-off-by Haneen Mohammed <[email protected]>
Acked-by Julia Lawall <[email protected]>
Quoted 46 lines
---
 drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/staging/lustre/lnet/lnet/router.c +2 -8
index 3df101b..b8eba33 100644
@@ -222,15 +222,12 @@ struct lnet_remotenet *
 lnet_find_net_locked(__u32 net)
 {
 	struct lnet_remotenet *rnet;
-	struct list_head *tmp;
 	struct list_head *rn_list;
 
 	LASSERT(!the_lnet.ln_shutdown);
 
 	rn_list = lnet_net2rnethash(net);
-	list_for_each(tmp, rn_list) {
-		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
-
+	list_for_each_entry(rnet, rn_list, lrn_list) {
 		if (rnet->lrn_net == net)
 			return rnet;
 	}
@@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
 	__u32 lnd_type, seed[2];
 	struct timespec64 ts;
 	struct lnet_ni *ni;
-	struct list_head *tmp;
 
 	if (seeded)
 		return;
@@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
 	 * Nodes with small feet have little entropy
 	 * the NID for this node gives the most entropy in the low bits
 	 */
-	list_for_each(tmp, &the_lnet.ln_nis) {
-		ni = list_entry(tmp, struct lnet_ni, ni_list);
+	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
 		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
 
 		if (lnd_type != LOLND)
--
2.7.4
Quoted 15 lines
Quoted 8 lines
Quoted 7 lines
Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170916004148.GA25693%40Haneen.
For more options, visit https://groups.google.com/d/optout.
Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170916161048.GA6020%40Haneen.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170916162124.GA6808%40Haneen.
For more options, visit https://groups.google.com/d/optout.