* [net-next bugfix] net, rps: fix build failure when CONFIG_RPS isn't set
@ 2013-12-31 20:31 Zhi Yong Wu
2013-12-31 20:54 ` David Miller
2013-12-31 22:03 ` Sergei Shtylyov
0 siblings, 2 replies; 7+ messages in thread
From: Zhi Yong Wu @ 2013-12-31 20:31 UTC (permalink / raw)
To: netdev; +Cc: davem, therbert, fengguang.wu, Zhi Yong Wu
From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
include/net/sock.h | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 8ee90ad..bd716b6 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -846,12 +846,16 @@ static inline void sock_rps_reset_flow_hash(__u32 hash)
static inline void sock_rps_record_flow(const struct sock *sk)
{
+#ifdef CONFIG_RPS
sock_rps_record_flow_hash(sk->sk_rxhash);
+#endif
}
static inline void sock_rps_reset_flow(const struct sock *sk)
{
+#ifdef CONFIG_RPS
sock_rps_reset_flow_hash(sk->sk_rxhash);
+#endif
}
static inline void sock_rps_save_rxhash(struct sock *sk,
--
1.7.6.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [net-next bugfix] net, rps: fix build failure when CONFIG_RPS isn't set
2013-12-31 20:31 [net-next bugfix] net, rps: fix build failure when CONFIG_RPS isn't set Zhi Yong Wu
@ 2013-12-31 20:54 ` David Miller
2013-12-31 20:58 ` David Miller
2013-12-31 22:03 ` Sergei Shtylyov
1 sibling, 1 reply; 7+ messages in thread
From: David Miller @ 2013-12-31 20:54 UTC (permalink / raw)
To: zwu.kernel; +Cc: netdev, therbert, fengguang.wu, wuzhy
From: Zhi Yong Wu <zwu.kernel@gmail.com>
Date: Wed, 1 Jan 2014 04:31:01 +0800
> @@ -846,12 +846,16 @@ static inline void sock_rps_reset_flow_hash(__u32 hash)
>
> static inline void sock_rps_record_flow(const struct sock *sk)
> {
> +#ifdef CONFIG_RPS
> sock_rps_record_flow_hash(sk->sk_rxhash);
> +#endif
> }
Why is this necessary? sock_rps_record_flow_hash() is provided
unconditionally, regardless of CONFIG_RPS.
I'm not applying this, it doesn't make any sense. Either there is
no build failure, or the existing CONFIG_RPS ifdef inside of the
implementation of sock_rps_record_flow_hash() is bogus.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [net-next bugfix] net, rps: fix build failure when CONFIG_RPS isn't set
2013-12-31 20:54 ` David Miller
@ 2013-12-31 20:58 ` David Miller
2014-01-01 6:36 ` Zhi Yong Wu
0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2013-12-31 20:58 UTC (permalink / raw)
To: zwu.kernel; +Cc: netdev, therbert, fengguang.wu, wuzhy
From: David Miller <davem@davemloft.net>
Date: Tue, 31 Dec 2013 15:54:59 -0500 (EST)
> From: Zhi Yong Wu <zwu.kernel@gmail.com>
> Date: Wed, 1 Jan 2014 04:31:01 +0800
>
>> @@ -846,12 +846,16 @@ static inline void sock_rps_reset_flow_hash(__u32 hash)
>>
>> static inline void sock_rps_record_flow(const struct sock *sk)
>> {
>> +#ifdef CONFIG_RPS
>> sock_rps_record_flow_hash(sk->sk_rxhash);
>> +#endif
>> }
>
> Why is this necessary? sock_rps_record_flow_hash() is provided
> unconditionally, regardless of CONFIG_RPS.
>
> I'm not applying this, it doesn't make any sense. Either there is
> no build failure, or the existing CONFIG_RPS ifdef inside of the
> implementation of sock_rps_record_flow_hash() is bogus.
Ok, if you had actually provided the build failure log message I'd be
able to evaluate this patch much better.
The real issue is the lack of sk_rxhash when RPS is not enabled.
I'm going to apply this with the build failure message mentioned.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [net-next bugfix] net, rps: fix build failure when CONFIG_RPS isn't set
2013-12-31 22:03 ` Sergei Shtylyov
@ 2013-12-31 21:08 ` David Miller
2013-12-31 21:08 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2013-12-31 21:08 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: zwu.kernel, netdev, therbert, fengguang.wu, wuzhy
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 01 Jan 2014 01:03:18 +0300
> Hello.
>
> On 12/31/2013 11:31 PM, Zhi Yong Wu wrote:
>
>> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>
>> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> ---
>> include/net/sock.h | 4 ++++
>> 1 files changed, 4 insertions(+), 0 deletions(-)
>
>> diff --git a/include/net/sock.h b/include/net/sock.h
>> index 8ee90ad..bd716b6 100644
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -846,12 +846,16 @@ static inline void
>> sock_rps_reset_flow_hash(__u32 hash)
>>
>> static inline void sock_rps_record_flow(const struct sock *sk)
>> {
>> +#ifdef CONFIG_RPS
>> sock_rps_record_flow_hash(sk->sk_rxhash);
>> +#endif
>> }
>>
>> static inline void sock_rps_reset_flow(const struct sock *sk)
>> {
>> +#ifdef CONFIG_RPS
>> sock_rps_reset_flow_hash(sk->sk_rxhash);
>> +#endif
>> }
>
> A lot better style would be:
No, the functions have to be available, but have a NOP implementation.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [net-next bugfix] net, rps: fix build failure when CONFIG_RPS isn't set
2013-12-31 22:03 ` Sergei Shtylyov
2013-12-31 21:08 ` David Miller
@ 2013-12-31 21:08 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2013-12-31 21:08 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: zwu.kernel, netdev, therbert, fengguang.wu, wuzhy
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 01 Jan 2014 01:03:18 +0300
> Hello.
>
> On 12/31/2013 11:31 PM, Zhi Yong Wu wrote:
>
>> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>
>> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> ---
>> include/net/sock.h | 4 ++++
>> 1 files changed, 4 insertions(+), 0 deletions(-)
>
>> diff --git a/include/net/sock.h b/include/net/sock.h
>> index 8ee90ad..bd716b6 100644
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -846,12 +846,16 @@ static inline void
>> sock_rps_reset_flow_hash(__u32 hash)
>>
>> static inline void sock_rps_record_flow(const struct sock *sk)
>> {
>> +#ifdef CONFIG_RPS
>> sock_rps_record_flow_hash(sk->sk_rxhash);
>> +#endif
>> }
>>
>> static inline void sock_rps_reset_flow(const struct sock *sk)
>> {
>> +#ifdef CONFIG_RPS
>> sock_rps_reset_flow_hash(sk->sk_rxhash);
>> +#endif
>> }
>
> A lot better style would be:
Sorry, I meant to say, this is consistent with how the other functions
dependent upon RPS are handled.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [net-next bugfix] net, rps: fix build failure when CONFIG_RPS isn't set
2013-12-31 20:31 [net-next bugfix] net, rps: fix build failure when CONFIG_RPS isn't set Zhi Yong Wu
2013-12-31 20:54 ` David Miller
@ 2013-12-31 22:03 ` Sergei Shtylyov
2013-12-31 21:08 ` David Miller
2013-12-31 21:08 ` David Miller
1 sibling, 2 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2013-12-31 22:03 UTC (permalink / raw)
To: Zhi Yong Wu, netdev; +Cc: davem, therbert, fengguang.wu, Zhi Yong Wu
Hello.
On 12/31/2013 11:31 PM, Zhi Yong Wu wrote:
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> ---
> include/net/sock.h | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 8ee90ad..bd716b6 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -846,12 +846,16 @@ static inline void sock_rps_reset_flow_hash(__u32 hash)
>
> static inline void sock_rps_record_flow(const struct sock *sk)
> {
> +#ifdef CONFIG_RPS
> sock_rps_record_flow_hash(sk->sk_rxhash);
> +#endif
> }
>
> static inline void sock_rps_reset_flow(const struct sock *sk)
> {
> +#ifdef CONFIG_RPS
> sock_rps_reset_flow_hash(sk->sk_rxhash);
> +#endif
> }
A lot better style would be:
+#ifdef CONFIG_RPS
static inline void sock_rps_reset_flow(const struct sock *sk)
{
sock_rps_reset_flow_hash(sk->sk_rxhash);
}
#else
static inline void sock_rps_reset_flow(const struct sock *sk) {}
+#endif
WBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [net-next bugfix] net, rps: fix build failure when CONFIG_RPS isn't set
2013-12-31 20:58 ` David Miller
@ 2014-01-01 6:36 ` Zhi Yong Wu
0 siblings, 0 replies; 7+ messages in thread
From: Zhi Yong Wu @ 2014-01-01 6:36 UTC (permalink / raw)
To: David Miller, Sergei Shtylyov
Cc: Linux Netdev List, Tom Herbert, Fengguang Wu, Zhi Yong Wu
On Wed, Jan 1, 2014 at 4:58 AM, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 31 Dec 2013 15:54:59 -0500 (EST)
>
>> From: Zhi Yong Wu <zwu.kernel@gmail.com>
>> Date: Wed, 1 Jan 2014 04:31:01 +0800
>>
>>> @@ -846,12 +846,16 @@ static inline void sock_rps_reset_flow_hash(__u32 hash)
>>>
>>> static inline void sock_rps_record_flow(const struct sock *sk)
>>> {
>>> +#ifdef CONFIG_RPS
>>> sock_rps_record_flow_hash(sk->sk_rxhash);
>>> +#endif
>>> }
>>
>> Why is this necessary? sock_rps_record_flow_hash() is provided
>> unconditionally, regardless of CONFIG_RPS.
>>
>> I'm not applying this, it doesn't make any sense. Either there is
>> no build failure, or the existing CONFIG_RPS ifdef inside of the
>> implementation of sock_rps_record_flow_hash() is bogus.
>
> Ok, if you had actually provided the build failure log message I'd be
> able to evaluate this patch much better.
>
> The real issue is the lack of sk_rxhash when RPS is not enabled.
>
> I'm going to apply this with the build failure message mentioned.
I noticed that you have added the build failure log message in your
net-next tree.
That is, i needn't do anything now, thanks.
--
Regards,
Zhi Yong Wu
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-01-01 6:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-31 20:31 [net-next bugfix] net, rps: fix build failure when CONFIG_RPS isn't set Zhi Yong Wu
2013-12-31 20:54 ` David Miller
2013-12-31 20:58 ` David Miller
2014-01-01 6:36 ` Zhi Yong Wu
2013-12-31 22:03 ` Sergei Shtylyov
2013-12-31 21:08 ` David Miller
2013-12-31 21:08 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).