* mesh RCU issues
@ 2011-05-12 12:25 Johannes Berg
2011-05-12 13:03 ` [PATCH] mac80211: remove pointless mesh path timer RCU code Johannes Berg
2011-05-12 22:26 ` mesh RCU issues Javier Cardona
0 siblings, 2 replies; 9+ messages in thread
From: Johannes Berg @ 2011-05-12 12:25 UTC (permalink / raw)
To: linux-wireless; +Cc: Javier Cardona
I was reviewing sparse RCU warnings in mac80211...
This seems to be some kind of bad joke:
void mesh_path_timer(unsigned long data)
{
struct ieee80211_sub_if_data *sdata;
struct mesh_path *mpath;
rcu_read_lock();
mpath = (struct mesh_path *) data;
mpath = rcu_dereference(mpath);
if (!mpath)
goto endmpathtimer;
????
And indeed I don't see a del_timer_sync() when the mesh path is freed.
But this is _clearly_ totally bogus. Somebody please fix ASAP.
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] mac80211: remove pointless mesh path timer RCU code 2011-05-12 12:25 mesh RCU issues Johannes Berg @ 2011-05-12 13:03 ` Johannes Berg 2011-05-12 22:26 ` mesh RCU issues Javier Cardona 1 sibling, 0 replies; 9+ messages in thread From: Johannes Berg @ 2011-05-12 13:03 UTC (permalink / raw) To: linux-wireless; +Cc: Javier Cardona, John W. Linville From: Johannes Berg <johannes.berg@intel.com> The code here to RCU-dereference a pointer that's on the stack is totally pointless, RCU isn't magic (like say Java's weak references are), so the code can't work like whoever wrote it thought it might. Remove it so readers don't get confused. Note that it seems that a bug is there anyway: I don't see any code that cancels the timer when a mesh path struct is destroyed. Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- net/mac80211/mesh_hwmp.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) --- a/net/mac80211/mesh_hwmp.c 2011-05-12 15:01:20.000000000 +0200 +++ b/net/mac80211/mesh_hwmp.c 2011-05-12 15:01:24.000000000 +0200 @@ -967,20 +967,11 @@ endlookup: void mesh_path_timer(unsigned long data) { - struct ieee80211_sub_if_data *sdata; - struct mesh_path *mpath; + struct mesh_path *mpath = (void *) data; + struct ieee80211_sub_if_data *sdata = mpath->sdata; - rcu_read_lock(); - mpath = (struct mesh_path *) data; - mpath = rcu_dereference(mpath); - if (!mpath) - goto endmpathtimer; - sdata = mpath->sdata; - - if (sdata->local->quiescing) { - rcu_read_unlock(); + if (sdata->local->quiescing) return; - } spin_lock_bh(&mpath->state_lock); if (mpath->flags & MESH_PATH_RESOLVED || @@ -997,8 +988,6 @@ void mesh_path_timer(unsigned long data) } spin_unlock_bh(&mpath->state_lock); -endmpathtimer: - rcu_read_unlock(); } void ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: mesh RCU issues 2011-05-12 12:25 mesh RCU issues Johannes Berg 2011-05-12 13:03 ` [PATCH] mac80211: remove pointless mesh path timer RCU code Johannes Berg @ 2011-05-12 22:26 ` Javier Cardona 2011-05-13 7:13 ` Johannes Berg 1 sibling, 1 reply; 9+ messages in thread From: Javier Cardona @ 2011-05-12 22:26 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, devel Johannes, On Thu, May 12, 2011 at 5:25 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > I was reviewing sparse RCU warnings in mac80211... > > This seems to be some kind of bad joke: > > void mesh_path_timer(unsigned long data) > { > struct ieee80211_sub_if_data *sdata; > struct mesh_path *mpath; > > rcu_read_lock(); > mpath = (struct mesh_path *) data; > mpath = rcu_dereference(mpath); > if (!mpath) > goto endmpathtimer; > > ???? Thanks for cleaning that up. > And indeed I don't see a del_timer_sync() when the mesh path is freed. Isn't the call to del_timer_sync() you are looking for in mesh_path_node_reclaim() ? > But this is _clearly_ totally bogus. Somebody please fix ASAP. I'll run sparse and see if I see other rcu warnings that I can fix. Thanks, j > johannes > > -- Javier Cardona cozybit Inc. http://www.cozybit.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: mesh RCU issues 2011-05-12 22:26 ` mesh RCU issues Javier Cardona @ 2011-05-13 7:13 ` Johannes Berg 2011-05-13 20:28 ` Javier Cardona 0 siblings, 1 reply; 9+ messages in thread From: Johannes Berg @ 2011-05-13 7:13 UTC (permalink / raw) To: Javier Cardona; +Cc: linux-wireless, devel On Thu, 2011-05-12 at 15:26 -0700, Javier Cardona wrote: > > And indeed I don't see a del_timer_sync() when the mesh path is freed. > > Isn't the call to del_timer_sync() you are looking for in > mesh_path_node_reclaim() ? Hmm, indeed, but it looks like mesh_path_node_free() also frees a node, no? I'd only found the latter function freeing it and got worried. > > But this is _clearly_ totally bogus. Somebody please fix ASAP. > > I'll run sparse and see if I see other rcu warnings that I can fix. Thanks. I think most of the use is probably OK or "just" missing rcu_dereference() wrappers. The global mesh_paths and mpp_paths variables should also be __rcu I think, but that caused so many warnings that I gave up for now, and I didn't quite understand what was going on. You'll want to apply http://johannes.sipsolutions.net/patches/kernel/all/LATEST/NNN-mac80211-rcu-annotations.patch to get rid of all the other spurious RCU warnings with CONFIG_SPARSE_RCU. Thanks, Johannes ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: mesh RCU issues 2011-05-13 7:13 ` Johannes Berg @ 2011-05-13 20:28 ` Javier Cardona 2011-05-13 23:30 ` Johannes Berg 2011-05-14 0:00 ` Johannes Berg 0 siblings, 2 replies; 9+ messages in thread From: Javier Cardona @ 2011-05-13 20:28 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, devel On Fri, May 13, 2011 at 12:13 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Thu, 2011-05-12 at 15:26 -0700, Javier Cardona wrote: > >> > And indeed I don't see a del_timer_sync() when the mesh path is freed. >> >> Isn't the call to del_timer_sync() you are looking for in >> mesh_path_node_reclaim() ? > > Hmm, indeed, but it looks like mesh_path_node_free() also frees a node, > no? I'd only found the latter function freeing it and got worried. Ah, I see. Yes, looks like the timer should be deleted there as well. Patch will follow. >> > But this is _clearly_ totally bogus. Somebody please fix ASAP. >> >> I'll run sparse and see if I see other rcu warnings that I can fix. > > Thanks. I think most of the use is probably OK or "just" missing > rcu_dereference() wrappers. The global mesh_paths and mpp_paths > variables should also be __rcu I think, but that caused so many warnings > that I gave up for now, and I didn't quite understand what was going on. > > You'll want to apply > http://johannes.sipsolutions.net/patches/kernel/all/LATEST/NNN-mac80211-rcu-annotations.patch to get rid of all the other spurious RCU warnings with CONFIG_SPARSE_RCU. (You probably meant CONFIG_SPARSE_RCU_POINTER) I must be doing something wrong because I don't see the RCU warnings after making {mesh,mpp}_paths __rcu. I only see two "different address spaces" errors that are fixed in the patch below. I do see a bunch of 'warning' errors: /home/javier/dev/wireless-testing/arch/x86/include/asm/uaccess_32.h:199:2: error: attribute 'warning': unknown attribute diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c index 83ce48e..1db8bba 100644 --- a/net/mac80211/mesh_pathtbl.c +++ b/net/mac80211/mesh_pathtbl.c @@ -36,8 +36,8 @@ struct mpath_node { struct mesh_path *mpath; }; -static struct mesh_table *mesh_paths; -static struct mesh_table *mpp_paths; /* Store paths for MPP&MAP */ +static struct mesh_table __rcu *mesh_paths; +static struct mesh_table __rcu *mpp_paths; /* Store paths for MPP&MAP */ int mesh_paths_generation; @@ -513,7 +513,7 @@ void mesh_plink_broken(struct sta_info *sta) for_each_mesh_entry(mesh_paths, p, node, i) { mpath = node->mpath; spin_lock_bh(&mpath->state_lock); - if (mpath->next_hop == sta && + if (rcu_dereference(mpath->next_hop) == sta && mpath->flags & MESH_PATH_ACTIVE && !(mpath->flags & MESH_PATH_FIXED)) { mpath->flags &= ~MESH_PATH_ACTIVE; @@ -549,7 +549,7 @@ void mesh_path_flush_by_nexthop(struct sta_info *sta) for_each_mesh_entry(mesh_paths, p, node, i) { mpath = node->mpath; - if (mpath->next_hop == sta) + if (rcu_dereference(mpath->next_hop) == sta) mesh_path_del(mpath->dst, mpath->sdata); } } Thoughts? j -- Javier Cardona cozybit Inc. http://www.cozybit.com ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: mesh RCU issues 2011-05-13 20:28 ` Javier Cardona @ 2011-05-13 23:30 ` Johannes Berg 2011-05-14 0:00 ` Johannes Berg 1 sibling, 0 replies; 9+ messages in thread From: Johannes Berg @ 2011-05-13 23:30 UTC (permalink / raw) To: Javier Cardona; +Cc: linux-wireless, devel On Fri, 2011-05-13 at 13:28 -0700, Javier Cardona wrote: > >> Isn't the call to del_timer_sync() you are looking for in > >> mesh_path_node_reclaim() ? > > > > Hmm, indeed, but it looks like mesh_path_node_free() also frees a node, > > no? I'd only found the latter function freeing it and got worried. > > Ah, I see. Yes, looks like the timer should be deleted there as well. > Patch will follow. Thanks! > > You'll want to apply > > http://johannes.sipsolutions.net/patches/kernel/all/LATEST/NNN-mac80211-rcu-annotations.patch to get rid of all the other spurious RCU warnings with CONFIG_SPARSE_RCU. > > (You probably meant CONFIG_SPARSE_RCU_POINTER) Yeah, sorry. > I must be doing something wrong because I don't see the RCU warnings > after making {mesh,mpp}_paths __rcu. I only see two "different > address spaces" errors that are fixed in the patch below. > I do see a bunch of 'warning' errors: > > /home/javier/dev/wireless-testing/arch/x86/include/asm/uaccess_32.h:199:2: > error: attribute 'warning': unknown attribute Oh, damn. You're running into a sparse issue: upon encountering an error, it aborts parsing/checking. So since you're getting an error from an include file, you never see any warnings from the rest of the file. > > > diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c > index 83ce48e..1db8bba 100644 > --- a/net/mac80211/mesh_pathtbl.c > +++ b/net/mac80211/mesh_pathtbl.c > @@ -36,8 +36,8 @@ struct mpath_node { > struct mesh_path *mpath; > }; > > -static struct mesh_table *mesh_paths; > -static struct mesh_table *mpp_paths; /* Store paths for MPP&MAP */ > +static struct mesh_table __rcu *mesh_paths; > +static struct mesh_table __rcu *mpp_paths; /* Store paths for MPP&MAP */ > > int mesh_paths_generation; > > @@ -513,7 +513,7 @@ void mesh_plink_broken(struct sta_info *sta) > for_each_mesh_entry(mesh_paths, p, node, i) { > mpath = node->mpath; > spin_lock_bh(&mpath->state_lock); > - if (mpath->next_hop == sta && > + if (rcu_dereference(mpath->next_hop) == sta && > mpath->flags & MESH_PATH_ACTIVE && > !(mpath->flags & MESH_PATH_FIXED)) { > mpath->flags &= ~MESH_PATH_ACTIVE; > @@ -549,7 +549,7 @@ void mesh_path_flush_by_nexthop(struct sta_info *sta) > > for_each_mesh_entry(mesh_paths, p, node, i) { > mpath = node->mpath; > - if (mpath->next_hop == sta) > + if (rcu_dereference(mpath->next_hop) == sta) > mesh_path_del(mpath->dst, mpath->sdata); > } > } > > Thoughts? I'll try if this is sufficient but it seemed there were more warnings when I tried. johannes ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: mesh RCU issues 2011-05-13 20:28 ` Javier Cardona 2011-05-13 23:30 ` Johannes Berg @ 2011-05-14 0:00 ` Johannes Berg 2011-05-14 3:52 ` Javier Cardona 1 sibling, 1 reply; 9+ messages in thread From: Johannes Berg @ 2011-05-14 0:00 UTC (permalink / raw) To: Javier Cardona; +Cc: linux-wireless, devel On Fri, 2011-05-13 at 13:28 -0700, Javier Cardona wrote: > diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c > index 83ce48e..1db8bba 100644 > --- a/net/mac80211/mesh_pathtbl.c > +++ b/net/mac80211/mesh_pathtbl.c [snip] With this patch, I get the warnings below. The locking ones are definitely genuine bugs, but the missing rcu_dereference() ones you already fixed were bugs too, albeit only on the Alpha architecture (and maybe elsewhere due to compiler optimisations?) johannes CHECK /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:335:48: warning: incorrect type in argument 3 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:335:48: expected struct mesh_table *tbl /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:335:48: got struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] mesh_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:348:13: warning: incorrect type in argument 2 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:348:13: expected struct atomic_t [usertype] *v /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:348:13: got struct atomic_t [noderef] <asn:4>*<noident> /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:382:16: warning: incorrect type in assignment (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:382:16: expected struct mesh_table *oldtbl /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:382:16: got struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] mesh_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:383:29: warning: incorrect type in argument 1 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:383:29: expected struct mesh_table *oldtbl /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:383:29: got struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] mesh_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:406:16: warning: incorrect type in assignment (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:406:16: expected struct mesh_table *oldtbl /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:406:16: got struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] mpp_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:407:29: warning: incorrect type in argument 1 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:407:29: expected struct mesh_table *oldtbl /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:407:29: got struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] mpp_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:459:48: warning: incorrect type in argument 3 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:459:48: expected struct mesh_table *tbl /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:459:48: got struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] [assigned] mpp_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:472:13: warning: incorrect type in argument 2 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:472:13: expected struct atomic_t [usertype] *v /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:472:13: got struct atomic_t [noderef] <asn:4>*<noident> /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:600:49: warning: incorrect type in argument 3 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:600:49: expected struct mesh_table *tbl /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:600:49: got struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] [assigned] mesh_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:612:37: warning: incorrect type in argument 1 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:612:37: expected struct atomic_t [usertype] *v /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:612:37: got struct atomic_t [noderef] <asn:4>*<noident> /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:748:20: warning: incorrect type in assignment (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:748:20: expected struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] [assigned] mesh_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:748:20: got struct mesh_table * /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:755:19: warning: incorrect type in assignment (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:755:19: expected struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] [assigned] mpp_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:755:19: got struct mesh_table * /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:757:33: warning: incorrect type in argument 1 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:757:33: expected struct mesh_table *tbl /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:757:33: got struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] [assigned] mesh_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:794:25: warning: incorrect type in argument 1 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:794:25: expected struct mesh_table *tbl /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:794:25: got struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] [assigned] mesh_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:795:25: warning: incorrect type in argument 1 (different address spaces) /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:795:25: expected struct mesh_table *tbl /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:795:25: got struct mesh_table [noderef] <asn:4>*static [addressable] [toplevel] [assigned] mpp_paths /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:266:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:266:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:266:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:266:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:336:19: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:338:23: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:349:17: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:349:47: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:354:25: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:363:25: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:373:6: warning: context imbalance in 'mesh_mpath_table_grow' - different lock contexts for basic block /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:397:6: warning: context imbalance in 'mesh_mpp_table_grow' - different lock contexts for basic block /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:460:19: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:462:23: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:473:17: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:473:46: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:476:25: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:485:25: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:513:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:513:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:513:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:513:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:550:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:550:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:550:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:550:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:564:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:564:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:564:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:564:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:601:19: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:603:23: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:621:25: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:751:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:752:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:753:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:760:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:761:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:762:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:775:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:775:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:775:9: warning: dereference of noderef expression /home/johannes/sys/wireless-testing/net/mac80211/mesh_pathtbl.c:775:9: warning: dereference of noderef expression ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: mesh RCU issues 2011-05-14 0:00 ` Johannes Berg @ 2011-05-14 3:52 ` Javier Cardona 2011-05-14 8:24 ` Johannes Berg 0 siblings, 1 reply; 9+ messages in thread From: Javier Cardona @ 2011-05-14 3:52 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, devel On Fri, May 13, 2011 at 5:00 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Fri, 2011-05-13 at 13:28 -0700, Javier Cardona wrote: > >> diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c >> index 83ce48e..1db8bba 100644 >> --- a/net/mac80211/mesh_pathtbl.c >> +++ b/net/mac80211/mesh_pathtbl.c > [snip] > > With this patch, I get the warnings below. Thanks. Any ideas on how to fix sparse? I'd like to see those too :) > The locking ones are definitely genuine bugs, I'm not sure I fully understand the sparse message... would this fix the two locking warnings? diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c index 83ce48e..fbf0c28 100644 --- a/net/mac80211/mesh_pathtbl.c +++ b/net/mac80211/mesh_pathtbl.c @@ -376,8 +376,10 @@ void mesh_mpath_table_grow(void) rcu_read_lock(); newtbl = mesh_table_alloc(rcu_dereference(mesh_paths)->size_order + 1); - if (!newtbl) + if (!newtbl) { + rcu_read_unlock(); return; + } write_lock_bh(&pathtbl_resize_lock); oldtbl = mesh_paths; if (mesh_table_grow(mesh_paths, newtbl) < 0) { @@ -400,8 +402,10 @@ void mesh_mpp_table_grow(void) rcu_read_lock(); newtbl = mesh_table_alloc(rcu_dereference(mpp_paths)->size_order + 1); - if (!newtbl) + if (!newtbl) { + rcu_read_unlock(); return; + } write_lock_bh(&pathtbl_resize_lock); oldtbl = mpp_paths; if (mesh_table_grow(mpp_paths, newtbl) < 0) { Javier ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: mesh RCU issues 2011-05-14 3:52 ` Javier Cardona @ 2011-05-14 8:24 ` Johannes Berg 0 siblings, 0 replies; 9+ messages in thread From: Johannes Berg @ 2011-05-14 8:24 UTC (permalink / raw) To: Javier Cardona; +Cc: linux-wireless, devel On Fri, 2011-05-13 at 20:52 -0700, Javier Cardona wrote: > > With this patch, I get the warnings below. > > Thanks. Any ideas on how to fix sparse? I'd like to see those too :) No, not really, what version are you using? I'm on 64 bit so I don't have uaccess_32.h included I guess, I suppose you could just go to that file and comment out the contents of the copy_from_user_overflow() function that seems to be causing it grief. > > The locking ones are definitely genuine bugs, > > I'm not sure I fully understand the sparse message... would this fix > the two locking warnings? [snip] Sure, those locking fixes should fix the bugs and get rid of the warnings unless there was another code path leaving w/o unlock. johannes ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-05-14 8:24 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-05-12 12:25 mesh RCU issues Johannes Berg 2011-05-12 13:03 ` [PATCH] mac80211: remove pointless mesh path timer RCU code Johannes Berg 2011-05-12 22:26 ` mesh RCU issues Javier Cardona 2011-05-13 7:13 ` Johannes Berg 2011-05-13 20:28 ` Javier Cardona 2011-05-13 23:30 ` Johannes Berg 2011-05-14 0:00 ` Johannes Berg 2011-05-14 3:52 ` Javier Cardona 2011-05-14 8:24 ` Johannes Berg
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).