Use list_for_each_entry_continue_reverse in setup_net

PATCH In review Pavel Emelyanov 3 participants 4 messages Sep 17, 2007 – Sep 18, 2007

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Pavel Emelyanov

author

2007-09-17 11:05 UTC #0 lore raw

PATCH 1/1 Use list_for_each_entry_continue_reverse in setup_net

I proposed introducing a list_for_each_entry_continue_reverse macro to be used in setup_net() when unrolling the failed ->init callback.

Here is the macro and some more cleanup in the setup_net() itself to remove one variable from the stack :) Minor, but the code looks nicer.

Signed-off-by Pavel Emelyanov <[email protected]>

Acked-by "Eric W. Biederman" <[email protected]>

---

 include/linux/list.h     |   14 ++++++++++++++
 net/core/net_namespace.c |    8 +++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

include/linux/list.h +14 -0

index f29fc9c..ad9dcb9 100644
@@ -525,6 +525,20 @@ static inline void list_splice_init_rcu(
 	     pos = list_entry(pos->member.next, typeof(*pos), member))
 
 /**
+ * list_for_each_entry_continue_reverse - iterate backwards from the given point
+ * @pos:	the type * to use as a loop cursor.
+ * @head:	the head for your list.
+ * @member:	the name of the list_struct within the struct.
+ *
+ * Start to iterate over list of given type backwards, continuing after
+ * the current position.
+ */
+#define list_for_each_entry_continue_reverse(pos, head, member)		\
+	for (pos = list_entry(pos->member.prev, typeof(*pos), member);	\
+	     prefetch(pos->member.prev), &pos->member != (head);	\
+	     pos = list_entry(pos->member.prev, typeof(*pos), member))
+
+/**
  * list_for_each_entry_from - iterate over list of given type from the current point
  * @pos:	the type * to use as a loop cursor.
  * @head:	the head for your list.

net/core/net_namespace.c +3 -5

index 1fc513c..a9dd261 100644
@@ -102,7 +102,6 @@ static int setup_net(struct net *net)
 {
 	/* Must be called with net_mutex held */
 	struct pernet_operations *ops;
-	struct list_head *ptr;
 	int error;
 
 	memset(net, 0, sizeof(struct net));

@@ -110,8 +109,7 @@ static int setup_net(struct net *net)
 	atomic_set(&net->use_count, 0);
 
 	error = 0;
-	list_for_each(ptr, &pernet_list) {
-		ops = list_entry(ptr, struct pernet_operations, list);
+	list_for_each_entry(ops, &pernet_list, list) {
 		if (ops->init) {
 			error = ops->init(net);
 			if (error < 0)

@@ -120,12 +118,12 @@ static int setup_net(struct net *net)
 	}
 out:
 	return error;
+
 out_undo:
 	/* Walk through the list backwards calling the exit functions
 	 * for the pernet modules whose init functions did not fail.
 	 */
-	for (ptr = ptr->prev; ptr != &pernet_list; ptr = ptr->prev) {
-		ops = list_entry(ptr, struct pernet_operations, list);
+	list_for_each_entry_continue_reverse(ops, &pernet_list, list) {
 		if (ops->exit)
 			ops->exit(net);
 	}

Eric W. Biederman

2007-09-17 13:07 UTC #1 lore raw

Quoted 10 lines · Pavel Emelyanov

I proposed introducing a list_for_each_entry_continue_reverse macro to be used in setup_net() when unrolling the failed ->init callback.
Here is the macro and some more cleanup in the setup_net() itself to remove one variable from the stack :) Minor, but the code looks nicer.
Signed-off-by Pavel Emelyanov <[email protected]>
Acked-by "Eric W. Biederman" <[email protected]>

Pavel if you are going down this route. Could you look at cleanup_net as well. The reverse walk there could probably benefit from being list_for_each_entry_reverse.

Eric

Quoted 71 lines

---

 include/linux/list.h     |   14 ++++++++++++++
 net/core/net_namespace.c |    8 +++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

include/linux/list.h +1 -0

index f29fc9c..ad9dcb9 100644
@@ -525,6 +525,20 @@ static inline void list_splice_init_rcu(
 	     pos = list_entry(pos->member.next, typeof(*pos), member))
 
 /**
+ * list_for_each_entry_continue_reverse - iterate backwards from the given

point
+ * @pos: the type * to use as a loop cursor.
+ * @head: the head for your list.
+ * @member: the name of the list_struct within the struct.
+ *
+ * Start to iterate over list of given type backwards, continuing after
+ * the current position.
+ */

include/linux/list.h +6 -0

+#define list_for_each_entry_continue_reverse(pos, head, member) \
+	for (pos = list_entry(pos->member.prev, typeof(*pos), member);	\
+	     prefetch(pos->member.prev), &pos->member != (head);	\
+	     pos = list_entry(pos->member.prev, typeof(*pos), member))
+
+/**
  * list_for_each_entry_from - iterate over list of given type from the current

point
* @pos: the type * to use as a loop cursor.
* @head: the head for your list.

net/core/net_namespace.c +3 -5

index 1fc513c..a9dd261 100644
@@ -102,7 +102,6 @@ static int setup_net(struct net *net)
 {
 	/* Must be called with net_mutex held */
 	struct pernet_operations *ops;
-	struct list_head *ptr;
 	int error;
 
 	memset(net, 0, sizeof(struct net));

@@ -110,8 +109,7 @@ static int setup_net(struct net *net)
 	atomic_set(&net->use_count, 0);
 
 	error = 0;
-	list_for_each(ptr, &pernet_list) {
-		ops = list_entry(ptr, struct pernet_operations, list);
+	list_for_each_entry(ops, &pernet_list, list) {
 		if (ops->init) {
 			error = ops->init(net);
 			if (error < 0)

@@ -120,12 +118,12 @@ static int setup_net(struct net *net)
 	}
 out:
 	return error;
+
 out_undo:
 	/* Walk through the list backwards calling the exit functions
 	 * for the pernet modules whose init functions did not fail.
 	 */
-	for (ptr = ptr->prev; ptr != &pernet_list; ptr = ptr->prev) {
-		ops = list_entry(ptr, struct pernet_operations, list);
+	list_for_each_entry_continue_reverse(ops, &pernet_list, list) {
 		if (ops->exit)
 			ops->exit(net);
 	}

Pavel Emelyanov

author

2007-09-17 13:57 UTC #2 lore raw

PATCH 1/1 Use list_for_each_entry_continue_reverse in setup_net

[snip]

Quoted 3 lines

Pavel if you are going down this route. Could you look at cleanup_net as well. The reverse walk there could probably benefit from being list_for_each_entry_reverse.

Oh! Thanks a lot ;) Here's the new patch.

Log:

I proposed introducing a list_for_each_entry_continue_reverse macro to be used in setup_net() when unrolling the failed ->init callback.

Here is the macro and some more cleanup in the setup_net() itself to remove one variable from the stack :) Minor, but the code looks nicer.

Signed-off-by Pavel Emelyanov <[email protected]>

Acked-by "Eric W. Biederman" <[email protected]>

---

include/linux/list.h +14 -0

index f29fc9c..ad9dcb9 100644
@@ -525,6 +525,20 @@ static inline void list_splice_init_rcu(
 	     pos = list_entry(pos->member.next, typeof(*pos), member))
 
 /**
+ * list_for_each_entry_continue_reverse - iterate backwards from the given point
+ * @pos:	the type * to use as a loop cursor.
+ * @head:	the head for your list.
+ * @member:	the name of the list_struct within the struct.
+ *
+ * Start to iterate over list of given type backwards, continuing after
+ * the current position.
+ */
+#define list_for_each_entry_continue_reverse(pos, head, member)		\
+	for (pos = list_entry(pos->member.prev, typeof(*pos), member);	\
+	     prefetch(pos->member.prev), &pos->member != (head);	\
+	     pos = list_entry(pos->member.prev, typeof(*pos), member))
+
+/**
  * list_for_each_entry_from - iterate over list of given type from the current point
  * @pos:	the type * to use as a loop cursor.
  * @head:	the head for your list.

net/core/net_namespace.c +4 -8

index 1fc513c..0e6cb02 100644
@@ -56,7 +56,6 @@ static void net_free(struct net *net)
 static void cleanup_net(struct work_struct *work)
 {
 	struct pernet_operations *ops;
-	struct list_head *ptr;
 	struct net *net;
 
 	net = container_of(work, struct net, work);

@@ -69,8 +68,7 @@ static void cleanup_net(struct work_stru
 	net_unlock();
 
 	/* Run all of the network namespace exit methods */
-	list_for_each_prev(ptr, &pernet_list) {
-		ops = list_entry(ptr, struct pernet_operations, list);
+	list_for_each_entry_reverse(ops, &pernet_list, list) {
 		if (ops->exit)
 			ops->exit(net);
 	}

@@ -102,7 +100,6 @@ static int setup_net(struct net *net)
 {
 	/* Must be called with net_mutex held */
 	struct pernet_operations *ops;
-	struct list_head *ptr;
 	int error;
 
 	memset(net, 0, sizeof(struct net));

@@ -110,8 +107,7 @@ static int setup_net(struct net *net)
 	atomic_set(&net->use_count, 0);
 
 	error = 0;
-	list_for_each(ptr, &pernet_list) {
-		ops = list_entry(ptr, struct pernet_operations, list);
+	list_for_each_entry(ops, &pernet_list, list) {
 		if (ops->init) {
 			error = ops->init(net);
 			if (error < 0)

@@ -120,12 +116,12 @@ static int setup_net(struct net *net)
 	}
 out:
 	return error;
+
 out_undo:
 	/* Walk through the list backwards calling the exit functions
 	 * for the pernet modules whose init functions did not fail.
 	 */
-	for (ptr = ptr->prev; ptr != &pernet_list; ptr = ptr->prev) {
-		ops = list_entry(ptr, struct pernet_operations, list);
+	list_for_each_entry_continue_reverse(ops, &pernet_list, list) {
 		if (ops->exit)
 			ops->exit(net);
 	}

David Miller

maintainer

2007-09-18 01:58 UTC #3 lore raw

In reply to Eric W. Biederman

From: [email protected] (Eric W. Biederman)
Date: Mon, 17 Sep 2007 07:07:42 -0600

Quoted 3 lines

Pavel if you are going down this route. Could you look at cleanup_net as well. The reverse walk there could probably benefit from being list_for_each_entry_reverse.

Pavel please resubmit this work after everything has been resolved, thanks.

If you have already, please resend as I don't have a copy.