* linux-next: build warning after merge of the net-next tree
@ 2020-09-08 3:00 Stephen Rothwell
2020-09-08 3:49 ` Jakub Kicinski
2020-09-08 7:17 ` [PATCH net-next] net: bridge: mcast: fix unused br var when lockdep isn't defined Nikolay Aleksandrov
0 siblings, 2 replies; 15+ messages in thread
From: Stephen Rothwell @ 2020-09-08 3:00 UTC (permalink / raw)
To: David Miller, Networking
Cc: Jakub Kicinski, Nikolay Aleksandrov, Linux Next Mailing List,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 575 bytes --]
Hi all,
After merging the net-next tree, today's linux-next build (powerpc
ppc64_defconfig) produced this warning:
net/bridge/br_multicast.c: In function 'br_multicast_find_port':
net/bridge/br_multicast.c:1818:21: warning: unused variable 'br' [-Wunused-variable]
1818 | struct net_bridge *br = mp->br;
| ^~
Introduced by commit
0436862e417e ("net: bridge: mcast: support for IGMPv3/MLDv2 ALLOW_NEW_SOURCES report")
Maybe turning mlock_dereference into a static inline function would help.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: linux-next: build warning after merge of the net-next tree 2020-09-08 3:00 linux-next: build warning after merge of the net-next tree Stephen Rothwell @ 2020-09-08 3:49 ` Jakub Kicinski 2020-09-08 7:17 ` [PATCH net-next] net: bridge: mcast: fix unused br var when lockdep isn't defined Nikolay Aleksandrov 1 sibling, 0 replies; 15+ messages in thread From: Jakub Kicinski @ 2020-09-08 3:49 UTC (permalink / raw) To: Stephen Rothwell, Nikolay Aleksandrov Cc: David Miller, Networking, Linux Next Mailing List, Linux Kernel Mailing List On Tue, 8 Sep 2020 13:00:00 +1000 Stephen Rothwell wrote: > Hi all, > > After merging the net-next tree, today's linux-next build (powerpc > ppc64_defconfig) produced this warning: > > net/bridge/br_multicast.c: In function 'br_multicast_find_port': > net/bridge/br_multicast.c:1818:21: warning: unused variable 'br' [-Wunused-variable] > 1818 | struct net_bridge *br = mp->br; > | ^~ > > Introduced by commit > > 0436862e417e ("net: bridge: mcast: support for IGMPv3/MLDv2 ALLOW_NEW_SOURCES report") > > Maybe turning mlock_dereference into a static inline function would help. Or perhaps provide a better definition of whatever is making the reference disappear? RCU_LOCKDEP_WARN()? Thanks for the report! ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next] net: bridge: mcast: fix unused br var when lockdep isn't defined 2020-09-08 3:00 linux-next: build warning after merge of the net-next tree Stephen Rothwell 2020-09-08 3:49 ` Jakub Kicinski @ 2020-09-08 7:17 ` Nikolay Aleksandrov 2020-09-08 16:00 ` Jakub Kicinski 1 sibling, 1 reply; 15+ messages in thread From: Nikolay Aleksandrov @ 2020-09-08 7:17 UTC (permalink / raw) To: netdev; +Cc: kuba, davem, roopa, Nikolay Aleksandrov, Stephen Rothwell Stephen reported the following warning: net/bridge/br_multicast.c: In function 'br_multicast_find_port': net/bridge/br_multicast.c:1818:21: warning: unused variable 'br' [-Wunused-variable] 1818 | struct net_bridge *br = mp->br; | ^~ It happens due to bridge's mlock_dereference() when lockdep isn't defined. Silence the warning by annotating the variable as __maybe_unused. Fixes: 0436862e417e ("net: bridge: mcast: support for IGMPv3/MLDv2 ALLOW_NEW_SOURCES report") Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> --- net/bridge/br_multicast.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index b83f11228948..33adf44ef7ec 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1814,8 +1814,8 @@ br_multicast_find_port(struct net_bridge_mdb_entry *mp, struct net_bridge_port *p, const unsigned char *src) { + struct net_bridge *br __maybe_unused = mp->br; struct net_bridge_port_group *pg; - struct net_bridge *br = mp->br; for (pg = mlock_dereference(mp->ports, br); pg; -- 2.25.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] net: bridge: mcast: fix unused br var when lockdep isn't defined 2020-09-08 7:17 ` [PATCH net-next] net: bridge: mcast: fix unused br var when lockdep isn't defined Nikolay Aleksandrov @ 2020-09-08 16:00 ` Jakub Kicinski 2020-09-08 16:09 ` nikolay 2020-09-08 17:36 ` [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition Jakub Kicinski 0 siblings, 2 replies; 15+ messages in thread From: Jakub Kicinski @ 2020-09-08 16:00 UTC (permalink / raw) To: Nikolay Aleksandrov; +Cc: netdev, davem, roopa, Stephen Rothwell On Tue, 8 Sep 2020 10:17:13 +0300 Nikolay Aleksandrov wrote: > Stephen reported the following warning: > net/bridge/br_multicast.c: In function 'br_multicast_find_port': > net/bridge/br_multicast.c:1818:21: warning: unused variable 'br' [-Wunused-variable] > 1818 | struct net_bridge *br = mp->br; > | ^~ > > It happens due to bridge's mlock_dereference() when lockdep isn't defined. > Silence the warning by annotating the variable as __maybe_unused. > > Fixes: 0436862e417e ("net: bridge: mcast: support for IGMPv3/MLDv2 ALLOW_NEW_SOURCES report") > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > --- > net/bridge/br_multicast.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > index b83f11228948..33adf44ef7ec 100644 > --- a/net/bridge/br_multicast.c > +++ b/net/bridge/br_multicast.c > @@ -1814,8 +1814,8 @@ br_multicast_find_port(struct net_bridge_mdb_entry *mp, > struct net_bridge_port *p, > const unsigned char *src) > { > + struct net_bridge *br __maybe_unused = mp->br; > struct net_bridge_port_group *pg; > - struct net_bridge *br = mp->br; > > for (pg = mlock_dereference(mp->ports, br); > pg; That's a lazy fix :( Is everyone using lockdep annotations going to sprinkle __maybe_unused throughout the code? Macros should also always evaluate their arguments. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] net: bridge: mcast: fix unused br var when lockdep isn't defined 2020-09-08 16:00 ` Jakub Kicinski @ 2020-09-08 16:09 ` nikolay 2020-09-08 17:36 ` [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition Jakub Kicinski 1 sibling, 0 replies; 15+ messages in thread From: nikolay @ 2020-09-08 16:09 UTC (permalink / raw) To: Jakub Kicinski; +Cc: netdev, davem, roopa, Stephen Rothwell On 8 September 2020 19:00:49 EEST, Jakub Kicinski <kuba@kernel.org> wrote: >On Tue, 8 Sep 2020 10:17:13 +0300 Nikolay Aleksandrov wrote: >> Stephen reported the following warning: >> net/bridge/br_multicast.c: In function 'br_multicast_find_port': >> net/bridge/br_multicast.c:1818:21: warning: unused variable 'br' >[-Wunused-variable] >> 1818 | struct net_bridge *br = mp->br; >> | ^~ >> >> It happens due to bridge's mlock_dereference() when lockdep isn't >defined. >> Silence the warning by annotating the variable as __maybe_unused. >> >> Fixes: 0436862e417e ("net: bridge: mcast: support for IGMPv3/MLDv2 >ALLOW_NEW_SOURCES report") >> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> >> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> >> --- >> net/bridge/br_multicast.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c >> index b83f11228948..33adf44ef7ec 100644 >> --- a/net/bridge/br_multicast.c >> +++ b/net/bridge/br_multicast.c >> @@ -1814,8 +1814,8 @@ br_multicast_find_port(struct >net_bridge_mdb_entry *mp, >> struct net_bridge_port *p, >> const unsigned char *src) >> { >> + struct net_bridge *br __maybe_unused = mp->br; >> struct net_bridge_port_group *pg; >> - struct net_bridge *br = mp->br; >> >> for (pg = mlock_dereference(mp->ports, br); >> pg; > >That's a lazy fix :( Is everyone using lockdep annotations going to >sprinkle __maybe_unused throughout the code? Macros should also always >evaluate their arguments. When the local variable's only used for lockdep, I guess. :) Here we don't actually need it at all, alternatively we can just drop it and use mp->br. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition 2020-09-08 16:00 ` Jakub Kicinski 2020-09-08 16:09 ` nikolay @ 2020-09-08 17:36 ` Jakub Kicinski 2020-09-08 18:15 ` nikolay 2020-09-09 3:12 ` David Miller 1 sibling, 2 replies; 15+ messages in thread From: Jakub Kicinski @ 2020-09-08 17:36 UTC (permalink / raw) To: davem Cc: netdev, paulmck, joel, josh, peterz, christian.brauner, rcu, linux-kernel, nikolay, sfr, roopa, Jakub Kicinski We run into a unused variable warning in bridge code when variable is only used inside the condition of rcu_dereference_protected(). #define mlock_dereference(X, br) \ rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock)) Since on builds with CONFIG_PROVE_RCU=n rcu_dereference_protected() compiles to nothing the compiler doesn't see the variable use. Prevent the warning by adding the condition as dead code. We need to un-hide the declaration of lockdep_tasklist_lock_is_held() and fix a bug the crept into a net/sched header. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- include/linux/rcupdate.h | 2 +- include/linux/sched/task.h | 2 -- include/net/sch_generic.h | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index d15d46db61f7..cf3d3ba3f3e4 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -320,7 +320,7 @@ static inline void rcu_preempt_sleep_check(void) { } #else /* #ifdef CONFIG_PROVE_RCU */ -#define RCU_LOCKDEP_WARN(c, s) do { } while (0) +#define RCU_LOCKDEP_WARN(c, s) do { } while (0 && (c)) #define rcu_sleep_check() do { } while (0) #endif /* #else #ifdef CONFIG_PROVE_RCU */ diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h index a98965007eef..9f943c391df9 100644 --- a/include/linux/sched/task.h +++ b/include/linux/sched/task.h @@ -47,9 +47,7 @@ extern spinlock_t mmlist_lock; extern union thread_union init_thread_union; extern struct task_struct init_task; -#ifdef CONFIG_PROVE_RCU extern int lockdep_tasklist_lock_is_held(void); -#endif /* #ifdef CONFIG_PROVE_RCU */ extern asmlinkage void schedule_tail(struct task_struct *prev); extern void init_idle(struct task_struct *idle, int cpu); diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index d60e7c39d60c..eb68cc6e4e79 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -443,7 +443,7 @@ static inline bool lockdep_tcf_proto_is_locked(struct tcf_proto *tp) return lockdep_is_held(&tp->lock); } #else -static inline bool lockdep_tcf_chain_is_locked(struct tcf_block *chain) +static inline bool lockdep_tcf_chain_is_locked(struct tcf_chain *chain) { return true; } -- 2.24.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition 2020-09-08 17:36 ` [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition Jakub Kicinski @ 2020-09-08 18:15 ` nikolay 2020-09-09 0:27 ` Jakub Kicinski 2020-09-09 3:12 ` David Miller 1 sibling, 1 reply; 15+ messages in thread From: nikolay @ 2020-09-08 18:15 UTC (permalink / raw) To: Jakub Kicinski, davem Cc: netdev, paulmck, joel, josh, peterz, christian.brauner, rcu, linux-kernel, sfr, roopa On 8 September 2020 20:36:24 EEST, Jakub Kicinski <kuba@kernel.org> wrote: >We run into a unused variable warning in bridge code when >variable is only used inside the condition of >rcu_dereference_protected(). > > #define mlock_dereference(X, br) \ > rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock)) > >Since on builds with CONFIG_PROVE_RCU=n rcu_dereference_protected() >compiles to nothing the compiler doesn't see the variable use. > >Prevent the warning by adding the condition as dead code. >We need to un-hide the declaration of lockdep_tasklist_lock_is_held() >and fix a bug the crept into a net/sched header. > >Signed-off-by: Jakub Kicinski <kuba@kernel.org> >--- > include/linux/rcupdate.h | 2 +- > include/linux/sched/task.h | 2 -- > include/net/sch_generic.h | 2 +- > 3 files changed, 2 insertions(+), 4 deletions(-) > >diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h >index d15d46db61f7..cf3d3ba3f3e4 100644 >--- a/include/linux/rcupdate.h >+++ b/include/linux/rcupdate.h >@@ -320,7 +320,7 @@ static inline void rcu_preempt_sleep_check(void) { >} > > #else /* #ifdef CONFIG_PROVE_RCU */ > >-#define RCU_LOCKDEP_WARN(c, s) do { } while (0) >+#define RCU_LOCKDEP_WARN(c, s) do { } while (0 && (c)) > #define rcu_sleep_check() do { } while (0) > > #endif /* #else #ifdef CONFIG_PROVE_RCU */ >diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h >index a98965007eef..9f943c391df9 100644 >--- a/include/linux/sched/task.h >+++ b/include/linux/sched/task.h >@@ -47,9 +47,7 @@ extern spinlock_t mmlist_lock; > extern union thread_union init_thread_union; > extern struct task_struct init_task; > >-#ifdef CONFIG_PROVE_RCU > extern int lockdep_tasklist_lock_is_held(void); >-#endif /* #ifdef CONFIG_PROVE_RCU */ > > extern asmlinkage void schedule_tail(struct task_struct *prev); > extern void init_idle(struct task_struct *idle, int cpu); >diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h >index d60e7c39d60c..eb68cc6e4e79 100644 >--- a/include/net/sch_generic.h >+++ b/include/net/sch_generic.h >@@ -443,7 +443,7 @@ static inline bool >lockdep_tcf_proto_is_locked(struct tcf_proto *tp) > return lockdep_is_held(&tp->lock); > } > #else >-static inline bool lockdep_tcf_chain_is_locked(struct tcf_block >*chain) >+static inline bool lockdep_tcf_chain_is_locked(struct tcf_chain >*chain) > { > return true; > } Ah, you want to solve it for all. :) Looks and sounds good to me, Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition 2020-09-08 18:15 ` nikolay @ 2020-09-09 0:27 ` Jakub Kicinski 2020-09-14 20:21 ` Joel Fernandes 0 siblings, 1 reply; 15+ messages in thread From: Jakub Kicinski @ 2020-09-09 0:27 UTC (permalink / raw) To: nikolay Cc: davem, netdev, paulmck, joel, josh, peterz, christian.brauner, rcu, linux-kernel, sfr, roopa On Tue, 08 Sep 2020 21:15:56 +0300 nikolay@cumulusnetworks.com wrote: > Ah, you want to solve it for all. :) > Looks and sounds good to me, > Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Actually, I give up, lockdep_is_held() is not defined without CONFIG_LOCKDEP, let's just go with your patch.. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition 2020-09-09 0:27 ` Jakub Kicinski @ 2020-09-14 20:21 ` Joel Fernandes 2020-09-14 22:47 ` Jakub Kicinski 0 siblings, 1 reply; 15+ messages in thread From: Joel Fernandes @ 2020-09-14 20:21 UTC (permalink / raw) To: Jakub Kicinski Cc: nikolay, davem, netdev, paulmck, josh, peterz, christian.brauner, rcu, linux-kernel, sfr, roopa On Tue, Sep 08, 2020 at 05:27:51PM -0700, Jakub Kicinski wrote: > On Tue, 08 Sep 2020 21:15:56 +0300 nikolay@cumulusnetworks.com wrote: > > Ah, you want to solve it for all. :) > > Looks and sounds good to me, > > Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > Actually, I give up, lockdep_is_held() is not defined without > CONFIG_LOCKDEP, let's just go with your patch.. Care to send a patch just for the RCU macro then? Not sure what Dave is applying but if the net-next tree is not taking the RCU macro change, then send another one with my tag: Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org> thanks! - Joel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition 2020-09-14 20:21 ` Joel Fernandes @ 2020-09-14 22:47 ` Jakub Kicinski 2020-09-15 0:20 ` Paul E. McKenney 2020-09-15 1:34 ` Joel Fernandes 0 siblings, 2 replies; 15+ messages in thread From: Jakub Kicinski @ 2020-09-14 22:47 UTC (permalink / raw) To: Joel Fernandes Cc: nikolay, davem, netdev, paulmck, josh, peterz, christian.brauner, rcu, linux-kernel, sfr, roopa On Mon, 14 Sep 2020 16:21:22 -0400 Joel Fernandes wrote: > On Tue, Sep 08, 2020 at 05:27:51PM -0700, Jakub Kicinski wrote: > > On Tue, 08 Sep 2020 21:15:56 +0300 nikolay@cumulusnetworks.com wrote: > > > Ah, you want to solve it for all. :) > > > Looks and sounds good to me, > > > Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > > > Actually, I give up, lockdep_is_held() is not defined without > > CONFIG_LOCKDEP, let's just go with your patch.. > > Care to send a patch just for the RCU macro then? Not sure what Dave is > applying but if the net-next tree is not taking the RCU macro change, then > send another one with my tag: Seems like quite a few places depend on the macro disappearing its argument. I was concerned that it's going to be had to pick out whether !LOCKDEP builds should return true or false from LOCKDEP helpers, but perhaps relying on the linker errors even more is not such poor taste? Does the patch below look acceptable to you? --->8------------ rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition We run into a unused variable warning in bridge code when variable is only used inside the condition of rcu_dereference_protected(). #define mlock_dereference(X, br) \ rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock)) Since on builds with CONFIG_PROVE_RCU=n rcu_dereference_protected() compiles to nothing the compiler doesn't see the variable use. Prevent the warning by adding the condition as dead code. We need to un-hide the declaration of lockdep_tasklist_lock_is_held(), lockdep_sock_is_held(), RCU lock maps and remove some declarations in net/sched header, because they have a wrong type. Add forward declarations of lockdep_is_held(), lock_is_held() which will cause a linker errors if actually used with !LOCKDEP. At least RCU expects some locks _not_ to be held so it's hard to pick true/false for a dummy implementation. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- include/linux/lockdep.h | 6 ++++++ include/linux/rcupdate.h | 11 ++++++----- include/linux/rcupdate_trace.h | 4 ++-- include/linux/sched/task.h | 2 -- include/net/sch_generic.h | 12 ------------ include/net/sock.h | 2 -- 6 files changed, 14 insertions(+), 23 deletions(-) diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index 6a584b3e5c74..c4b6225ee320 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -371,6 +371,12 @@ static inline void lockdep_unregister_key(struct lock_class_key *key) #define lockdep_depth(tsk) (0) +/* + * Dummy forward declarations, allow users to write less ifdef-y code + * and depend on dead code elimination. + */ +int lock_is_held(const void *); +int lockdep_is_held(const void *); #define lockdep_is_held_type(l, r) (1) #define lockdep_assert_held(l) do { (void)(l); } while (0) diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index d15d46db61f7..50d45781fa99 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -234,6 +234,11 @@ bool rcu_lockdep_current_cpu_online(void); static inline bool rcu_lockdep_current_cpu_online(void) { return true; } #endif /* #else #if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PROVE_RCU) */ +extern struct lockdep_map rcu_lock_map; +extern struct lockdep_map rcu_bh_lock_map; +extern struct lockdep_map rcu_sched_lock_map; +extern struct lockdep_map rcu_callback_map; + #ifdef CONFIG_DEBUG_LOCK_ALLOC static inline void rcu_lock_acquire(struct lockdep_map *map) @@ -246,10 +251,6 @@ static inline void rcu_lock_release(struct lockdep_map *map) lock_release(map, _THIS_IP_); } -extern struct lockdep_map rcu_lock_map; -extern struct lockdep_map rcu_bh_lock_map; -extern struct lockdep_map rcu_sched_lock_map; -extern struct lockdep_map rcu_callback_map; int debug_lockdep_rcu_enabled(void); int rcu_read_lock_held(void); int rcu_read_lock_bh_held(void); @@ -320,7 +321,7 @@ static inline void rcu_preempt_sleep_check(void) { } #else /* #ifdef CONFIG_PROVE_RCU */ -#define RCU_LOCKDEP_WARN(c, s) do { } while (0) +#define RCU_LOCKDEP_WARN(c, s) do { } while (0 && (c)) #define rcu_sleep_check() do { } while (0) #endif /* #else #ifdef CONFIG_PROVE_RCU */ diff --git a/include/linux/rcupdate_trace.h b/include/linux/rcupdate_trace.h index aaaac8ac927c..25cdef506cae 100644 --- a/include/linux/rcupdate_trace.h +++ b/include/linux/rcupdate_trace.h @@ -11,10 +11,10 @@ #include <linux/sched.h> #include <linux/rcupdate.h> -#ifdef CONFIG_DEBUG_LOCK_ALLOC - extern struct lockdep_map rcu_trace_lock_map; +#ifdef CONFIG_DEBUG_LOCK_ALLOC + static inline int rcu_read_lock_trace_held(void) { return lock_is_held(&rcu_trace_lock_map); diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h index a98965007eef..9f943c391df9 100644 --- a/include/linux/sched/task.h +++ b/include/linux/sched/task.h @@ -47,9 +47,7 @@ extern spinlock_t mmlist_lock; extern union thread_union init_thread_union; extern struct task_struct init_task; -#ifdef CONFIG_PROVE_RCU extern int lockdep_tasklist_lock_is_held(void); -#endif /* #ifdef CONFIG_PROVE_RCU */ extern asmlinkage void schedule_tail(struct task_struct *prev); extern void init_idle(struct task_struct *idle, int cpu); diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index d60e7c39d60c..1aaa9e3d2e9c 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -432,7 +432,6 @@ struct tcf_block { struct mutex proto_destroy_lock; /* Lock for proto_destroy hashtable. */ }; -#ifdef CONFIG_PROVE_LOCKING static inline bool lockdep_tcf_chain_is_locked(struct tcf_chain *chain) { return lockdep_is_held(&chain->filter_chain_lock); @@ -442,17 +441,6 @@ static inline bool lockdep_tcf_proto_is_locked(struct tcf_proto *tp) { return lockdep_is_held(&tp->lock); } -#else -static inline bool lockdep_tcf_chain_is_locked(struct tcf_block *chain) -{ - return true; -} - -static inline bool lockdep_tcf_proto_is_locked(struct tcf_proto *tp) -{ - return true; -} -#endif /* #ifdef CONFIG_PROVE_LOCKING */ #define tcf_chain_dereference(p, chain) \ rcu_dereference_protected(p, lockdep_tcf_chain_is_locked(chain)) diff --git a/include/net/sock.h b/include/net/sock.h index eaa5cac5e836..1c67b1297a72 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -1566,13 +1566,11 @@ do { \ lockdep_init_map(&(sk)->sk_lock.dep_map, (name), (key), 0); \ } while (0) -#ifdef CONFIG_LOCKDEP static inline bool lockdep_sock_is_held(const struct sock *sk) { return lockdep_is_held(&sk->sk_lock) || lockdep_is_held(&sk->sk_lock.slock); } -#endif void lock_sock_nested(struct sock *sk, int subclass); -- 2.24.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition 2020-09-14 22:47 ` Jakub Kicinski @ 2020-09-15 0:20 ` Paul E. McKenney 2020-09-15 0:30 ` Jakub Kicinski 2020-09-15 1:34 ` Joel Fernandes 1 sibling, 1 reply; 15+ messages in thread From: Paul E. McKenney @ 2020-09-15 0:20 UTC (permalink / raw) To: Jakub Kicinski Cc: Joel Fernandes, nikolay, davem, netdev, josh, peterz, christian.brauner, rcu, linux-kernel, sfr, roopa On Mon, Sep 14, 2020 at 03:47:38PM -0700, Jakub Kicinski wrote: > On Mon, 14 Sep 2020 16:21:22 -0400 Joel Fernandes wrote: > > On Tue, Sep 08, 2020 at 05:27:51PM -0700, Jakub Kicinski wrote: > > > On Tue, 08 Sep 2020 21:15:56 +0300 nikolay@cumulusnetworks.com wrote: > > > > Ah, you want to solve it for all. :) > > > > Looks and sounds good to me, > > > > Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > > > > > Actually, I give up, lockdep_is_held() is not defined without > > > CONFIG_LOCKDEP, let's just go with your patch.. > > > > Care to send a patch just for the RCU macro then? Not sure what Dave is > > applying but if the net-next tree is not taking the RCU macro change, then > > send another one with my tag: > > Seems like quite a few places depend on the macro disappearing its > argument. I was concerned that it's going to be had to pick out whether > !LOCKDEP builds should return true or false from LOCKDEP helpers, but > perhaps relying on the linker errors even more is not such poor taste? > > Does the patch below look acceptable to you? The thing to check would be whether all compilers do sufficient dead-code elimination (it used to be that they did not). One way to get a quick sniff test of this would be to make sure that a dead-code lockdep_is_held() is in common code, and then expose this patch to kbuild test robot. Seem reasonable? Thanx, Paul > --->8------------ > > rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition > > We run into a unused variable warning in bridge code when > variable is only used inside the condition of > rcu_dereference_protected(). > > #define mlock_dereference(X, br) \ > rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock)) > > Since on builds with CONFIG_PROVE_RCU=n rcu_dereference_protected() > compiles to nothing the compiler doesn't see the variable use. > > Prevent the warning by adding the condition as dead code. > We need to un-hide the declaration of lockdep_tasklist_lock_is_held(), > lockdep_sock_is_held(), RCU lock maps and remove some declarations > in net/sched header, because they have a wrong type. > > Add forward declarations of lockdep_is_held(), lock_is_held() which > will cause a linker errors if actually used with !LOCKDEP. > At least RCU expects some locks _not_ to be held so it's hard to > pick true/false for a dummy implementation. > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > include/linux/lockdep.h | 6 ++++++ > include/linux/rcupdate.h | 11 ++++++----- > include/linux/rcupdate_trace.h | 4 ++-- > include/linux/sched/task.h | 2 -- > include/net/sch_generic.h | 12 ------------ > include/net/sock.h | 2 -- > 6 files changed, 14 insertions(+), 23 deletions(-) > > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h > index 6a584b3e5c74..c4b6225ee320 100644 > --- a/include/linux/lockdep.h > +++ b/include/linux/lockdep.h > @@ -371,6 +371,12 @@ static inline void lockdep_unregister_key(struct lock_class_key *key) > > #define lockdep_depth(tsk) (0) > > +/* > + * Dummy forward declarations, allow users to write less ifdef-y code > + * and depend on dead code elimination. > + */ > +int lock_is_held(const void *); > +int lockdep_is_held(const void *); > #define lockdep_is_held_type(l, r) (1) > > #define lockdep_assert_held(l) do { (void)(l); } while (0) > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > index d15d46db61f7..50d45781fa99 100644 > --- a/include/linux/rcupdate.h > +++ b/include/linux/rcupdate.h > @@ -234,6 +234,11 @@ bool rcu_lockdep_current_cpu_online(void); > static inline bool rcu_lockdep_current_cpu_online(void) { return true; } > #endif /* #else #if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PROVE_RCU) */ > > +extern struct lockdep_map rcu_lock_map; > +extern struct lockdep_map rcu_bh_lock_map; > +extern struct lockdep_map rcu_sched_lock_map; > +extern struct lockdep_map rcu_callback_map; > + > #ifdef CONFIG_DEBUG_LOCK_ALLOC > > static inline void rcu_lock_acquire(struct lockdep_map *map) > @@ -246,10 +251,6 @@ static inline void rcu_lock_release(struct lockdep_map *map) > lock_release(map, _THIS_IP_); > } > > -extern struct lockdep_map rcu_lock_map; > -extern struct lockdep_map rcu_bh_lock_map; > -extern struct lockdep_map rcu_sched_lock_map; > -extern struct lockdep_map rcu_callback_map; > int debug_lockdep_rcu_enabled(void); > int rcu_read_lock_held(void); > int rcu_read_lock_bh_held(void); > @@ -320,7 +321,7 @@ static inline void rcu_preempt_sleep_check(void) { } > > #else /* #ifdef CONFIG_PROVE_RCU */ > > -#define RCU_LOCKDEP_WARN(c, s) do { } while (0) > +#define RCU_LOCKDEP_WARN(c, s) do { } while (0 && (c)) > #define rcu_sleep_check() do { } while (0) > > #endif /* #else #ifdef CONFIG_PROVE_RCU */ > diff --git a/include/linux/rcupdate_trace.h b/include/linux/rcupdate_trace.h > index aaaac8ac927c..25cdef506cae 100644 > --- a/include/linux/rcupdate_trace.h > +++ b/include/linux/rcupdate_trace.h > @@ -11,10 +11,10 @@ > #include <linux/sched.h> > #include <linux/rcupdate.h> > > -#ifdef CONFIG_DEBUG_LOCK_ALLOC > - > extern struct lockdep_map rcu_trace_lock_map; > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > + > static inline int rcu_read_lock_trace_held(void) > { > return lock_is_held(&rcu_trace_lock_map); > diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h > index a98965007eef..9f943c391df9 100644 > --- a/include/linux/sched/task.h > +++ b/include/linux/sched/task.h > @@ -47,9 +47,7 @@ extern spinlock_t mmlist_lock; > extern union thread_union init_thread_union; > extern struct task_struct init_task; > > -#ifdef CONFIG_PROVE_RCU > extern int lockdep_tasklist_lock_is_held(void); > -#endif /* #ifdef CONFIG_PROVE_RCU */ > > extern asmlinkage void schedule_tail(struct task_struct *prev); > extern void init_idle(struct task_struct *idle, int cpu); > diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h > index d60e7c39d60c..1aaa9e3d2e9c 100644 > --- a/include/net/sch_generic.h > +++ b/include/net/sch_generic.h > @@ -432,7 +432,6 @@ struct tcf_block { > struct mutex proto_destroy_lock; /* Lock for proto_destroy hashtable. */ > }; > > -#ifdef CONFIG_PROVE_LOCKING > static inline bool lockdep_tcf_chain_is_locked(struct tcf_chain *chain) > { > return lockdep_is_held(&chain->filter_chain_lock); > @@ -442,17 +441,6 @@ static inline bool lockdep_tcf_proto_is_locked(struct tcf_proto *tp) > { > return lockdep_is_held(&tp->lock); > } > -#else > -static inline bool lockdep_tcf_chain_is_locked(struct tcf_block *chain) > -{ > - return true; > -} > - > -static inline bool lockdep_tcf_proto_is_locked(struct tcf_proto *tp) > -{ > - return true; > -} > -#endif /* #ifdef CONFIG_PROVE_LOCKING */ > > #define tcf_chain_dereference(p, chain) \ > rcu_dereference_protected(p, lockdep_tcf_chain_is_locked(chain)) > diff --git a/include/net/sock.h b/include/net/sock.h > index eaa5cac5e836..1c67b1297a72 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -1566,13 +1566,11 @@ do { \ > lockdep_init_map(&(sk)->sk_lock.dep_map, (name), (key), 0); \ > } while (0) > > -#ifdef CONFIG_LOCKDEP > static inline bool lockdep_sock_is_held(const struct sock *sk) > { > return lockdep_is_held(&sk->sk_lock) || > lockdep_is_held(&sk->sk_lock.slock); > } > -#endif > > void lock_sock_nested(struct sock *sk, int subclass); > > -- > 2.24.1 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition 2020-09-15 0:20 ` Paul E. McKenney @ 2020-09-15 0:30 ` Jakub Kicinski 2020-09-15 17:02 ` Paul E. McKenney 0 siblings, 1 reply; 15+ messages in thread From: Jakub Kicinski @ 2020-09-15 0:30 UTC (permalink / raw) To: Paul E. McKenney Cc: Joel Fernandes, nikolay, davem, netdev, josh, peterz, christian.brauner, rcu, linux-kernel, sfr, roopa On Mon, 14 Sep 2020 17:20:11 -0700 Paul E. McKenney wrote: > > Seems like quite a few places depend on the macro disappearing its > > argument. I was concerned that it's going to be had to pick out whether > > !LOCKDEP builds should return true or false from LOCKDEP helpers, but > > perhaps relying on the linker errors even more is not such poor taste? > > > > Does the patch below look acceptable to you? > > The thing to check would be whether all compilers do sufficient > dead-code elimination (it used to be that they did not). One way to > get a quick sniff test of this would be to make sure that a dead-code > lockdep_is_held() is in common code, and then expose this patch to kbuild > test robot. I'm pretty sure it's in common code because kbuild bot complaints were the reason I gave up the first time around ;) I'll expose this to kbuild bot via my kernel.org tree in case it doesn't consider scissored patches and report back! ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition 2020-09-15 0:30 ` Jakub Kicinski @ 2020-09-15 17:02 ` Paul E. McKenney 0 siblings, 0 replies; 15+ messages in thread From: Paul E. McKenney @ 2020-09-15 17:02 UTC (permalink / raw) To: Jakub Kicinski Cc: Joel Fernandes, nikolay, davem, netdev, josh, peterz, christian.brauner, rcu, linux-kernel, sfr, roopa On Mon, Sep 14, 2020 at 05:30:29PM -0700, Jakub Kicinski wrote: > On Mon, 14 Sep 2020 17:20:11 -0700 Paul E. McKenney wrote: > > > Seems like quite a few places depend on the macro disappearing its > > > argument. I was concerned that it's going to be had to pick out whether > > > !LOCKDEP builds should return true or false from LOCKDEP helpers, but > > > perhaps relying on the linker errors even more is not such poor taste? > > > > > > Does the patch below look acceptable to you? > > > > The thing to check would be whether all compilers do sufficient > > dead-code elimination (it used to be that they did not). One way to > > get a quick sniff test of this would be to make sure that a dead-code > > lockdep_is_held() is in common code, and then expose this patch to kbuild > > test robot. > > I'm pretty sure it's in common code because kbuild bot complaints were > the reason I gave up the first time around ;) > > I'll expose this to kbuild bot via my kernel.org tree in case it > doesn't consider scissored patches and report back! Sounds good, thank you! Thanx, Paul ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition 2020-09-14 22:47 ` Jakub Kicinski 2020-09-15 0:20 ` Paul E. McKenney @ 2020-09-15 1:34 ` Joel Fernandes 1 sibling, 0 replies; 15+ messages in thread From: Joel Fernandes @ 2020-09-15 1:34 UTC (permalink / raw) To: Jakub Kicinski Cc: nikolay, davem, netdev, paulmck, josh, peterz, christian.brauner, rcu, linux-kernel, sfr, roopa On Mon, Sep 14, 2020 at 03:47:38PM -0700, Jakub Kicinski wrote: > On Mon, 14 Sep 2020 16:21:22 -0400 Joel Fernandes wrote: > > On Tue, Sep 08, 2020 at 05:27:51PM -0700, Jakub Kicinski wrote: > > > On Tue, 08 Sep 2020 21:15:56 +0300 nikolay@cumulusnetworks.com wrote: > > > > Ah, you want to solve it for all. :) > > > > Looks and sounds good to me, > > > > Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > > > > > Actually, I give up, lockdep_is_held() is not defined without > > > CONFIG_LOCKDEP, let's just go with your patch.. > > > > Care to send a patch just for the RCU macro then? Not sure what Dave is > > applying but if the net-next tree is not taking the RCU macro change, then > > send another one with my tag: > > Seems like quite a few places depend on the macro disappearing its > argument. I was concerned that it's going to be had to pick out whether > !LOCKDEP builds should return true or false from LOCKDEP helpers, but > perhaps relying on the linker errors even more is not such poor taste? > > Does the patch below look acceptable to you? > > --->8------------ > > rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition > > We run into a unused variable warning in bridge code when > variable is only used inside the condition of > rcu_dereference_protected(). > > #define mlock_dereference(X, br) \ > rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock)) > > Since on builds with CONFIG_PROVE_RCU=n rcu_dereference_protected() > compiles to nothing the compiler doesn't see the variable use. > > Prevent the warning by adding the condition as dead code. > We need to un-hide the declaration of lockdep_tasklist_lock_is_held(), > lockdep_sock_is_held(), RCU lock maps and remove some declarations > in net/sched header, because they have a wrong type. > > Add forward declarations of lockdep_is_held(), lock_is_held() which > will cause a linker errors if actually used with !LOCKDEP. > At least RCU expects some locks _not_ to be held so it's hard to > pick true/false for a dummy implementation. > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > include/linux/lockdep.h | 6 ++++++ > include/linux/rcupdate.h | 11 ++++++----- > include/linux/rcupdate_trace.h | 4 ++-- > include/linux/sched/task.h | 2 -- > include/net/sch_generic.h | 12 ------------ > include/net/sock.h | 2 -- Would it make sense to split it into individual patches? So 1 for rcu, 1 for lockdep and then 1 for networking. The lockdep ones may need PeterZ's ack. thanks, - Joel > 6 files changed, 14 insertions(+), 23 deletions(-) > > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h > index 6a584b3e5c74..c4b6225ee320 100644 > --- a/include/linux/lockdep.h > +++ b/include/linux/lockdep.h > @@ -371,6 +371,12 @@ static inline void lockdep_unregister_key(struct lock_class_key *key) > > #define lockdep_depth(tsk) (0) > > +/* > + * Dummy forward declarations, allow users to write less ifdef-y code > + * and depend on dead code elimination. > + */ > +int lock_is_held(const void *); > +int lockdep_is_held(const void *); > #define lockdep_is_held_type(l, r) (1) > > #define lockdep_assert_held(l) do { (void)(l); } while (0) > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > index d15d46db61f7..50d45781fa99 100644 > --- a/include/linux/rcupdate.h > +++ b/include/linux/rcupdate.h > @@ -234,6 +234,11 @@ bool rcu_lockdep_current_cpu_online(void); > static inline bool rcu_lockdep_current_cpu_online(void) { return true; } > #endif /* #else #if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PROVE_RCU) */ > > +extern struct lockdep_map rcu_lock_map; > +extern struct lockdep_map rcu_bh_lock_map; > +extern struct lockdep_map rcu_sched_lock_map; > +extern struct lockdep_map rcu_callback_map; > + > #ifdef CONFIG_DEBUG_LOCK_ALLOC > > static inline void rcu_lock_acquire(struct lockdep_map *map) > @@ -246,10 +251,6 @@ static inline void rcu_lock_release(struct lockdep_map *map) > lock_release(map, _THIS_IP_); > } > > -extern struct lockdep_map rcu_lock_map; > -extern struct lockdep_map rcu_bh_lock_map; > -extern struct lockdep_map rcu_sched_lock_map; > -extern struct lockdep_map rcu_callback_map; > int debug_lockdep_rcu_enabled(void); > int rcu_read_lock_held(void); > int rcu_read_lock_bh_held(void); > @@ -320,7 +321,7 @@ static inline void rcu_preempt_sleep_check(void) { } > > #else /* #ifdef CONFIG_PROVE_RCU */ > > -#define RCU_LOCKDEP_WARN(c, s) do { } while (0) > +#define RCU_LOCKDEP_WARN(c, s) do { } while (0 && (c)) > #define rcu_sleep_check() do { } while (0) > > #endif /* #else #ifdef CONFIG_PROVE_RCU */ > diff --git a/include/linux/rcupdate_trace.h b/include/linux/rcupdate_trace.h > index aaaac8ac927c..25cdef506cae 100644 > --- a/include/linux/rcupdate_trace.h > +++ b/include/linux/rcupdate_trace.h > @@ -11,10 +11,10 @@ > #include <linux/sched.h> > #include <linux/rcupdate.h> > > -#ifdef CONFIG_DEBUG_LOCK_ALLOC > - > extern struct lockdep_map rcu_trace_lock_map; > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > + > static inline int rcu_read_lock_trace_held(void) > { > return lock_is_held(&rcu_trace_lock_map); > diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h > index a98965007eef..9f943c391df9 100644 > --- a/include/linux/sched/task.h > +++ b/include/linux/sched/task.h > @@ -47,9 +47,7 @@ extern spinlock_t mmlist_lock; > extern union thread_union init_thread_union; > extern struct task_struct init_task; > > -#ifdef CONFIG_PROVE_RCU > extern int lockdep_tasklist_lock_is_held(void); > -#endif /* #ifdef CONFIG_PROVE_RCU */ > > extern asmlinkage void schedule_tail(struct task_struct *prev); > extern void init_idle(struct task_struct *idle, int cpu); > diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h > index d60e7c39d60c..1aaa9e3d2e9c 100644 > --- a/include/net/sch_generic.h > +++ b/include/net/sch_generic.h > @@ -432,7 +432,6 @@ struct tcf_block { > struct mutex proto_destroy_lock; /* Lock for proto_destroy hashtable. */ > }; > > -#ifdef CONFIG_PROVE_LOCKING > static inline bool lockdep_tcf_chain_is_locked(struct tcf_chain *chain) > { > return lockdep_is_held(&chain->filter_chain_lock); > @@ -442,17 +441,6 @@ static inline bool lockdep_tcf_proto_is_locked(struct tcf_proto *tp) > { > return lockdep_is_held(&tp->lock); > } > -#else > -static inline bool lockdep_tcf_chain_is_locked(struct tcf_block *chain) > -{ > - return true; > -} > - > -static inline bool lockdep_tcf_proto_is_locked(struct tcf_proto *tp) > -{ > - return true; > -} > -#endif /* #ifdef CONFIG_PROVE_LOCKING */ > > #define tcf_chain_dereference(p, chain) \ > rcu_dereference_protected(p, lockdep_tcf_chain_is_locked(chain)) > diff --git a/include/net/sock.h b/include/net/sock.h > index eaa5cac5e836..1c67b1297a72 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -1566,13 +1566,11 @@ do { \ > lockdep_init_map(&(sk)->sk_lock.dep_map, (name), (key), 0); \ > } while (0) > > -#ifdef CONFIG_LOCKDEP > static inline bool lockdep_sock_is_held(const struct sock *sk) > { > return lockdep_is_held(&sk->sk_lock) || > lockdep_is_held(&sk->sk_lock.slock); > } > -#endif > > void lock_sock_nested(struct sock *sk, int subclass); > > -- > 2.24.1 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition 2020-09-08 17:36 ` [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition Jakub Kicinski 2020-09-08 18:15 ` nikolay @ 2020-09-09 3:12 ` David Miller 1 sibling, 0 replies; 15+ messages in thread From: David Miller @ 2020-09-09 3:12 UTC (permalink / raw) To: kuba Cc: netdev, paulmck, joel, josh, peterz, christian.brauner, rcu, linux-kernel, nikolay, sfr, roopa From: Jakub Kicinski <kuba@kernel.org> Date: Tue, 8 Sep 2020 10:36:24 -0700 > We run into a unused variable warning in bridge code when > variable is only used inside the condition of > rcu_dereference_protected(). > > #define mlock_dereference(X, br) \ > rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock)) > > Since on builds with CONFIG_PROVE_RCU=n rcu_dereference_protected() > compiles to nothing the compiler doesn't see the variable use. > > Prevent the warning by adding the condition as dead code. > We need to un-hide the declaration of lockdep_tasklist_lock_is_held() > and fix a bug the crept into a net/sched header. > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> I ended up applying Nikolay's fix, but this situation with the rcu macros needs to be addressed. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-09-15 19:02 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-08 3:00 linux-next: build warning after merge of the net-next tree Stephen Rothwell 2020-09-08 3:49 ` Jakub Kicinski 2020-09-08 7:17 ` [PATCH net-next] net: bridge: mcast: fix unused br var when lockdep isn't defined Nikolay Aleksandrov 2020-09-08 16:00 ` Jakub Kicinski 2020-09-08 16:09 ` nikolay 2020-09-08 17:36 ` [PATCH net-next] rcu: prevent RCU_LOCKDEP_WARN() from swallowing the condition Jakub Kicinski 2020-09-08 18:15 ` nikolay 2020-09-09 0:27 ` Jakub Kicinski 2020-09-14 20:21 ` Joel Fernandes 2020-09-14 22:47 ` Jakub Kicinski 2020-09-15 0:20 ` Paul E. McKenney 2020-09-15 0:30 ` Jakub Kicinski 2020-09-15 17:02 ` Paul E. McKenney 2020-09-15 1:34 ` Joel Fernandes 2020-09-09 3:12 ` David Miller
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.