All of lore.kernel.org
 help / color / mirror / Atom feed
* 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 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.