* [iptables PATCH] list: fix prefetch dummy
@ 2015-04-06 18:05 Arturo Borrero Gonzalez
2015-04-07 0:38 ` Alexander Duyck
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Arturo Borrero Gonzalez @ 2015-04-06 18:05 UTC (permalink / raw)
To: netfilter-devel; +Cc: kaber, pablo
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 ]
Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
libiptc/linux_list.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h
index abdcf88..559e33c 100644
--- a/libiptc/linux_list.h
+++ b/libiptc/linux_list.h
@@ -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()
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [iptables PATCH] list: fix prefetch dummy
2015-04-06 18:05 [iptables PATCH] list: fix prefetch dummy Arturo Borrero Gonzalez
@ 2015-04-07 0:38 ` Alexander Duyck
2015-04-07 8:37 ` Jan Engelhardt
2015-04-08 17:04 ` Pablo Neira Ayuso
2015-04-09 0:18 ` Stephen Hemminger
2 siblings, 1 reply; 9+ messages in thread
From: Alexander Duyck @ 2015-04-07 0:38 UTC (permalink / raw)
To: Arturo Borrero Gonzalez, netfilter-devel; +Cc: kaber, pablo
On 04/06/2015 11:05 AM, Arturo Borrero Gonzalez wrote:
> 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 ]
>
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
> libiptc/linux_list.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h
> index abdcf88..559e33c 100644
> --- a/libiptc/linux_list.h
> +++ b/libiptc/linux_list.h
> @@ -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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [iptables PATCH] list: fix prefetch dummy
2015-04-07 0:38 ` Alexander Duyck
@ 2015-04-07 8:37 ` Jan Engelhardt
2015-04-07 17:45 ` Alexander Duyck
0 siblings, 1 reply; 9+ messages in thread
From: Jan Engelhardt @ 2015-04-07 8:37 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Arturo Borrero Gonzalez, netfilter-devel, kaber, pablo
On Tuesday 2015-04-07 02:38, Alexander Duyck wrote:
>On 04/06/2015 11:05 AM, Arturo Borrero Gonzalez wrote:
>> 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 ]
>>
>> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
>> ---
>> libiptc/linux_list.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h
>> index abdcf88..559e33c 100644
>> --- a/libiptc/linux_list.h
>> +++ b/libiptc/linux_list.h
>> @@ -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.
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [iptables PATCH] list: fix prefetch dummy
2015-04-07 8:37 ` Jan Engelhardt
@ 2015-04-07 17:45 ` Alexander Duyck
2015-04-07 18:17 ` Jan Engelhardt
0 siblings, 1 reply; 9+ messages in thread
From: Alexander Duyck @ 2015-04-07 17:45 UTC (permalink / raw)
To: Jan Engelhardt, Alexander Duyck
Cc: Arturo Borrero Gonzalez, netfilter-devel, kaber, pablo
On 04/07/2015 01:37 AM, Jan Engelhardt wrote:
> On Tuesday 2015-04-07 02:38, Alexander Duyck wrote:
>
>> On 04/06/2015 11:05 AM, Arturo Borrero Gonzalez wrote:
>>> 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 ]
>>>
>>> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
>>> ---
>>> libiptc/linux_list.h | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h
>>> index abdcf88..559e33c 100644
>>> --- a/libiptc/linux_list.h
>>> +++ b/libiptc/linux_list.h
>>> @@ -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.
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [iptables PATCH] list: fix prefetch dummy
2015-04-07 17:45 ` Alexander Duyck
@ 2015-04-07 18:17 ` Jan Engelhardt
2015-04-07 19:29 ` Alexander Duyck
0 siblings, 1 reply; 9+ messages in thread
From: Jan Engelhardt @ 2015-04-07 18:17 UTC (permalink / raw)
To: Alexander Duyck
Cc: Alexander Duyck, Arturo Borrero Gonzalez, netfilter-devel, kaber, pablo
On Tuesday 2015-04-07 19:45, Alexander Duyck wrote:
>>>> -#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.
>> 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].
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [iptables PATCH] list: fix prefetch dummy
2015-04-07 18:17 ` Jan Engelhardt
@ 2015-04-07 19:29 ` Alexander Duyck
0 siblings, 0 replies; 9+ messages in thread
From: Alexander Duyck @ 2015-04-07 19:29 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Alexander Duyck, Arturo Borrero Gonzalez, netfilter-devel, kaber, pablo
On 04/07/2015 11:17 AM, Jan Engelhardt wrote:
>
> On Tuesday 2015-04-07 19:45, Alexander Duyck wrote:
>>>>> -#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.
>>> 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);
>
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [iptables PATCH] list: fix prefetch dummy
2015-04-06 18:05 [iptables PATCH] list: fix prefetch dummy Arturo Borrero Gonzalez
2015-04-07 0:38 ` Alexander Duyck
@ 2015-04-08 17:04 ` Pablo Neira Ayuso
2015-04-09 0:18 ` Stephen Hemminger
2 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2015-04-08 17:04 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: netfilter-devel, kaber
On Mon, Apr 06, 2015 at 08:05:41PM +0200, Arturo Borrero Gonzalez wrote:
> 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [iptables PATCH] list: fix prefetch dummy
2015-04-06 18:05 [iptables PATCH] list: fix prefetch dummy Arturo Borrero Gonzalez
2015-04-07 0:38 ` Alexander Duyck
2015-04-08 17:04 ` Pablo Neira Ayuso
@ 2015-04-09 0:18 ` Stephen Hemminger
2015-04-09 10:45 ` Pablo Neira Ayuso
2 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2015-04-09 0:18 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: netfilter-devel, kaber, pablo
On Mon, 06 Apr 2015 20:05:41 +0200
Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote:
> 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 ]
>
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
> libiptc/linux_list.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h
> index abdcf88..559e33c 100644
> --- a/libiptc/linux_list.h
> +++ b/libiptc/linux_list.h
> @@ -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()
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [iptables PATCH] list: fix prefetch dummy
2015-04-09 0:18 ` Stephen Hemminger
@ 2015-04-09 10:45 ` Pablo Neira Ayuso
0 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2015-04-09 10:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Arturo Borrero Gonzalez, netfilter-devel, kaber
On Wed, Apr 08, 2015 at 05:18:25PM -0700, Stephen Hemminger wrote:
> On Mon, 06 Apr 2015 20:05:41 +0200
> Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote:
>
> > 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 ]
> >
> > Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> > ---
> > libiptc/linux_list.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h
> > index abdcf88..559e33c 100644
> > --- a/libiptc/linux_list.h
> > +++ b/libiptc/linux_list.h
> > @@ -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()
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> > the body of a message to majordomo@vger.kernel.org
> > 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-04-09 10:40 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-06 18:05 [iptables PATCH] list: fix prefetch dummy Arturo Borrero Gonzalez
2015-04-07 0:38 ` Alexander Duyck
2015-04-07 8:37 ` Jan Engelhardt
2015-04-07 17:45 ` Alexander Duyck
2015-04-07 18:17 ` Jan Engelhardt
2015-04-07 19:29 ` Alexander Duyck
2015-04-08 17:04 ` Pablo Neira Ayuso
2015-04-09 0:18 ` Stephen Hemminger
2015-04-09 10:45 ` Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.