[PATCH] RDS: Simplify code

Subsystems: networking [general], rds - reliable datagram sockets, the rest

STALE3696d

5 messages, 3 authors, 2016-09-06 · open the first message on its own page

[PATCH] RDS: Simplify code

From: Christophe JAILLET <hidden>
Date: 2016-09-03 05:34:22

Calling 'list_splice' followed by 'INIT_LIST_HEAD' is equivalent to
'list_splice_init'.

This has been spotted with the following coccinelle script:
/////
@@
expression y,z;
@@

-   list_splice(y,z);
-   INIT_LIST_HEAD(y);
+   list_splice_init(y,z);

Signed-off-by: Christophe JAILLET <redacted>
---
 net/rds/loop.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/rds/loop.c b/net/rds/loop.c
index f2bf78de5688..c3e6da4fdf97 100644
--- a/net/rds/loop.c
+++ b/net/rds/loop.c
@@ -167,8 +167,7 @@ void rds_loop_exit(void)
 
 	/* avoid calling conn_destroy with irqs off */
 	spin_lock_irq(&loop_conns_lock);
-	list_splice(&loop_conns, &tmp_list);
-	INIT_LIST_HEAD(&loop_conns);
+	list_splice_init(&loop_conns, &tmp_list);
 	spin_unlock_irq(&loop_conns_lock);
 
 	list_for_each_entry_safe(lc, _lc, &tmp_list, loop_node) {
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] RDS: Simplify code

From: Leon Romanovsky <leon@kernel.org>
Date: 2016-09-04 12:21:01

On Sat, Sep 03, 2016 at 07:33:29AM +0200, Christophe JAILLET wrote:
Calling 'list_splice' followed by 'INIT_LIST_HEAD' is equivalent to
'list_splice_init'.
It is not 100% accurate

list_splice(y, z)
INIT_LIST_HEAD(y)

==>

if (!list_empty(y))
     __list_splice(y, z, z>next);
INIT_LIST_HEAD(y)

and not

if (!list_empty(y)) {
     __list_splice(y, z, z>next);
     INIT_LIST_HEAD(y)
}

as list_splice_init will do.

quoted hunk
This has been spotted with the following coccinelle script:
/////
@@
expression y,z;
@@

-   list_splice(y,z);
-   INIT_LIST_HEAD(y);
+   list_splice_init(y,z);

Signed-off-by: Christophe JAILLET <redacted>
---
 net/rds/loop.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/rds/loop.c b/net/rds/loop.c
index f2bf78de5688..c3e6da4fdf97 100644
--- a/net/rds/loop.c
+++ b/net/rds/loop.c
@@ -167,8 +167,7 @@ void rds_loop_exit(void)

 	/* avoid calling conn_destroy with irqs off */
 	spin_lock_irq(&loop_conns_lock);
-	list_splice(&loop_conns, &tmp_list);
-	INIT_LIST_HEAD(&loop_conns);
+	list_splice_init(&loop_conns, &tmp_list);
 	spin_unlock_irq(&loop_conns_lock);

 	list_for_each_entry_safe(lc, _lc, &tmp_list, loop_node) {
--
2.7.4

Re: [PATCH] RDS: Simplify code

From: Christophe JAILLET <hidden>
Date: 2016-09-04 16:06:21

Le 04/09/2016 à 14:20, Leon Romanovsky a écrit :
On Sat, Sep 03, 2016 at 07:33:29AM +0200, Christophe JAILLET wrote:
quoted
Calling 'list_splice' followed by 'INIT_LIST_HEAD' is equivalent to
'list_splice_init'.
It is not 100% accurate

list_splice(y, z)
INIT_LIST_HEAD(y)

==>

if (!list_empty(y))
      __list_splice(y, z, z>next);
INIT_LIST_HEAD(y)

and not

if (!list_empty(y)) {
      __list_splice(y, z, z>next);
      INIT_LIST_HEAD(y)
}

as list_splice_init will do.
You are right but if you dig further you will see that calling INIT_LIST_HEAD on an empty list is a no-op (AFAIK).
And if this list was not already correctly initialized, then you would have some other troubles.

CJ

Re: [PATCH] RDS: Simplify code

From: Leon Romanovsky <leon@kernel.org>
Date: 2016-09-04 18:25:05

On Sun, Sep 04, 2016 at 05:57:20PM +0200, Christophe JAILLET wrote:
Le 04/09/2016 à 14:20, Leon Romanovsky a écrit :
quoted
On Sat, Sep 03, 2016 at 07:33:29AM +0200, Christophe JAILLET wrote:
quoted
Calling 'list_splice' followed by 'INIT_LIST_HEAD' is equivalent to
'list_splice_init'.
It is not 100% accurate

list_splice(y, z)
INIT_LIST_HEAD(y)

==>

if (!list_empty(y))
     __list_splice(y, z, z>next);
INIT_LIST_HEAD(y)

and not

if (!list_empty(y)) {
     __list_splice(y, z, z>next);
     INIT_LIST_HEAD(y)
}

as list_splice_init will do.
You are right but if you dig further you will see that calling
INIT_LIST_HEAD on an empty list is a no-op (AFAIK).
And if this list was not already correctly initialized, then you would have
some other troubles.
Thank you for the suggestion,
It looks like the code after that can be skipped in case of loop_conns
list is empty, the tmp_list will be empty too.

174         list_for_each_entry_safe(lc, _lc, &tmp_list, loop_node) {
175                 WARN_ON(lc->conn->c_passive);
176                 rds_conn_destroy(lc->conn);
177         }
CJ

Re: [PATCH] RDS: Simplify code

From: santosh.shilimkar@oracle.com <hidden>
Date: 2016-09-06 00:22:17

On 9/4/16 11:23 AM, Leon Romanovsky wrote:
On Sun, Sep 04, 2016 at 05:57:20PM +0200, Christophe JAILLET wrote:
quoted
Le 04/09/2016 à 14:20, Leon Romanovsky a écrit :
quoted
On Sat, Sep 03, 2016 at 07:33:29AM +0200, Christophe JAILLET wrote:
quoted
Calling 'list_splice' followed by 'INIT_LIST_HEAD' is equivalent to
'list_splice_init'.
It is not 100% accurate

list_splice(y, z)
INIT_LIST_HEAD(y)

==>

if (!list_empty(y))
     __list_splice(y, z, z>next);
INIT_LIST_HEAD(y)

and not

if (!list_empty(y)) {
     __list_splice(y, z, z>next);
     INIT_LIST_HEAD(y)
}

as list_splice_init will do.
You are right but if you dig further you will see that calling
INIT_LIST_HEAD on an empty list is a no-op (AFAIK).
And if this list was not already correctly initialized, then you would have
some other troubles.
Thank you for the suggestion,
It looks like the code after that can be skipped in case of loop_conns
list is empty, the tmp_list will be empty too.

174         list_for_each_entry_safe(lc, _lc, &tmp_list, loop_node) {
175                 WARN_ON(lc->conn->c_passive);
176                 rds_conn_destroy(lc->conn);
177         }
Thanks for trying. As already pointed, your change doesn't simplify
much rather change the behavior. The loop cursor already takes care
of list empty case. I don't see any reason to change that code.

Regards,
Santosh

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help