Skip to contentSkip to conversation

list: fix prefetch dummy

PATCH In review Arturo Borrero Gonzalez 6 participants 9 messages Apr 6, 2015 – Apr 9, 2015 netfilter

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 list: fix prefetch dummy

linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value]

  for (pos = list_entry((head)->next, typeof(*pos), member), \
                                                           ^

libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry'
list_for_each_entry(c, &h->chains, list) {
^

[ Patch copied from one similar of Patrick McHardy on libnftnl ]

---
 libiptc/linux_list.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
libiptc/linux_list.h +1 -1
index abdcf88..559e33c 100644
@@ -27,7 +27,7 @@
 	1; \
 })
 
-#define prefetch(x)		1
+#define prefetch(x)		((void)0)
 
 /* empty define to make this work in userspace -HW */
 #define smp_wmb()
#1 lore raw
Quoted 20 lines · Arturo Borrero Gonzalez

linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value]

  for (pos = list_entry((head)->next, typeof(*pos), member), \
                                                           ^

libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry'
list_for_each_entry(c, &h->chains, list) {
^

[ Patch copied from one similar of Patrick McHardy on libnftnl ]

---
 libiptc/linux_list.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
libiptc/linux_list.h +0 -0
index abdcf88..559e33c 100644

libiptc/linux_list.h line 33

Quoted 8 lines
libiptc/linux_list.h +1 -1
@@ -27,7 +27,7 @@
 	1; \
 })
 
-#define prefetch(x)		1
+#define prefetch(x)		((void)0)
 
 /* empty define to make this work in userspace -HW */
 #define smp_wmb()

Why not just use "do {} while (0)"? I know that is what is used in the kernel for functions that don't do anything.

- Alex

#2 lore raw
Quoted 26 lines · Alexander Duyck
Quoted 25 lines · Arturo Borrero Gonzalez

linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value]

  for (pos = list_entry((head)->next, typeof(*pos), member), \
                                                           ^

libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry'
list_for_each_entry(c, &h->chains, list) {
^

[ Patch copied from one similar of Patrick McHardy on libnftnl ]

---
 libiptc/linux_list.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
libiptc/linux_list.h +1 -1
index abdcf88..559e33c 100644
@@ -27,7 +27,7 @@
 	1; \
 })
 
-#define prefetch(x)		1
+#define prefetch(x)		((void)0)
 
Quoted 6 lines

libiptc/linux_list.h line 33

Quoted 3 lines
libiptc/linux_list.h +0 -0
 /* empty define to make this work in userspace -HW */
 #define smp_wmb()

Why not just use "do {} while (0)"? I know that is what is used in the kernel for functions that don't do anything.

I may be getting the terms wrong, but:
do{}while(0) is not an expression, it is a (block) control statement. In particular, do{}while(0) won't evaluate to an rvalue.

#3 lore raw
Quoted 28 lines · Jan Engelhardt
Quoted 26 lines · Alexander Duyck
Quoted 25 lines · Arturo Borrero Gonzalez

linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value]

   for (pos = list_entry((head)->next, typeof(*pos), member), \
                                                            ^
libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry'
   list_for_each_entry(c, &h->chains, list) {
   ^

[ Patch copied from one similar of Patrick McHardy on libnftnl ]

---
  libiptc/linux_list.h |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
libiptc/linux_list.h +1 -1
index abdcf88..559e33c 100644
@@ -27,7 +27,7 @@
  	1; \
  })
 
-#define prefetch(x)		1
+#define prefetch(x)		((void)0)
 
Quoted 8 lines
Quoted 5 lines

libiptc/linux_list.h line 33

Quoted 3 lines
libiptc/linux_list.h +0 -0
  /* empty define to make this work in userspace -HW */
  #define smp_wmb()

Why not just use "do {} while (0)"? I know that is what is used in the kernel for functions that don't do anything.

I may be getting the terms wrong, but:
do{}while(0) is not an expression, it is a (block) control statement. In particular, do{}while(0) won't evaluate to an rvalue.

Right. That is the point in this case. I am assuming what Arturo is trying to accomplish since you shouldn't be able to evaluate ((void)0) as an rvalue either.

The prefetch(x) is an empty statement which is being cast as a void to avoid a compiler warning, so it falls into the first case as defined in: http://kernelnewbies.org/FAQ/DoWhile0

- Alex

#4 lore raw
Quoted 5 lines · Alexander Duyck
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

-#define prefetch(x) 1
+#define prefetch(x) ((void)0)

Why not just use "do {} while (0)"? I know that is what is used in the kernel for functions that don't do anything.

Quoted 7 lines
Quoted 3 lines

I may be getting the terms wrong, but:
do{}while(0) is not an expression, it is a (block) control statement. In particular, do{}while(0) won't evaluate to an rvalue.

Right. That is the point in this case. I am assuming what Arturo is trying to accomplish since you shouldn't be able to evaluate ((void)0) as an rvalue either.

The difference is that

	int i;
	i = 1, (void)0;

compiles, while

	i = 1, do{}while(0);

will not. Even less so with

	int i = 1, (void)0 vs
	int i = 1, do{}while(0);

[that looks so obscure that even I need to go figure out why that is even permitted at this point].

#5 lore raw
Quoted 15 lines · Jan Engelhardt
Quoted 5 lines · Alexander Duyck
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

-#define prefetch(x) 1
+#define prefetch(x) ((void)0)

Why not just use "do {} while (0)"? I know that is what is used in the kernel for functions that don't do anything.

Quoted 7 lines
Quoted 3 lines

I may be getting the terms wrong, but:
do{}while(0) is not an expression, it is a (block) control statement. In particular, do{}while(0) won't evaluate to an rvalue.

Right. That is the point in this case. I am assuming what Arturo is trying to accomplish since you shouldn't be able to evaluate ((void)0) as an rvalue either.

Quoted 9 lines

The difference is that

	int i;
	i = 1, (void)0;

compiles, while

	i = 1, do{}while(0);

Okay, that explains it. It is being used inside the init, condition, and post process sections of a for loop. Thanks for explaining it.

- Alex

#6 lore raw
In reply to Arturo Borrero Gonzalez
Quoted 8 lines · Arturo Borrero Gonzalez

linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value]

  for (pos = list_entry((head)->next, typeof(*pos), member), \
                                                           ^

libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry'
list_for_each_entry(c, &h->chains, list) {
^

[ Patch copied from one similar of Patrick McHardy on libnftnl ]

Also applied, thanks.

#7 lore raw
In reply to Arturo Borrero Gonzalez
Quoted 29 lines · On Mon, 06 Apr 2015 20:05:41 +0200 Arturo Borrero Gonzalez…

linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value]

  for (pos = list_entry((head)->next, typeof(*pos), member), \
                                                           ^

libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry'
list_for_each_entry(c, &h->chains, list) {
^

[ Patch copied from one similar of Patrick McHardy on libnftnl ]

---
 libiptc/linux_list.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
libiptc/linux_list.h +1 -2
index abdcf88..559e33c 100644
@@ -27,7 +27,7 @@
 	1; \
 })
 
-#define prefetch(x)		1
+#define prefetch(x)		((void)0)
 
 /* empty define to make this work in userspace -HW */
 #define smp_wmb()
 
--
Quoted 3 lines

To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

The kernel dropped the prefetch() in the list macros a couple of years ago. The prefetch() had no performance gain and hurt in several cases.

#8 lore raw
Quoted 31 lines · Stephen Hemminger
Quoted 28 lines · On Mon, 06 Apr 2015 20:05:41 +0200 Arturo Borrero Gonzalez…

linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value]

  for (pos = list_entry((head)->next, typeof(*pos), member), \
                                                           ^

libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry'
list_for_each_entry(c, &h->chains, list) {
^

[ Patch copied from one similar of Patrick McHardy on libnftnl ]

---
 libiptc/linux_list.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
libiptc/linux_list.h +1 -1
index abdcf88..559e33c 100644
@@ -27,7 +27,7 @@
 	1; \
 })
 
-#define prefetch(x)		1
+#define prefetch(x)		((void)0)
 
 /* empty define to make this work in userspace -HW */
 #define smp_wmb()
Quoted 8 lines
Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

The kernel dropped the prefetch() in the list macros a couple of years ago. The prefetch() had no performance gain and hurt in several cases.

We have an old copy of linux's list.h in our userspace trees. We can probably refresh those to get them in sync with current at some point.