* suspicious RCU usage in net/ipv4/ip_tunnel.c:80
@ 2014-01-11 0:37 Cong Wang
2014-01-11 1:21 ` Eric Dumazet
0 siblings, 1 reply; 10+ messages in thread
From: Cong Wang @ 2014-01-11 0:37 UTC (permalink / raw)
To: Tom Herbert; +Cc: netdev
Hi, Tom
I saw:
[ 65.609535] [ INFO: suspicious RCU usage. ]
[ 65.609537] 3.13.0-rc6+ #139 Not tainted
[ 65.609538] -------------------------------
[ 65.609539] net/ipv4/ip_tunnel.c:80 suspicious rcu_dereference_check() usage!
[ 65.609540]
[ 65.609540] other info that might help us debug this:
[ 65.609540]
[ 65.609541]
[ 65.609541] rcu_scheduler_active = 1, debug_locks = 0
[ 65.609542] 5 locks held by kworker/u8:0/6:
[ 65.609553] #0: (%s#4){.+.+.+}, at: [<ffffffff8106d56b>]
process_one_work+0x173/0x41e
[ 65.609557] #1: (net_cleanup_work){+.+.+.}, at:
[<ffffffff8106d56b>] process_one_work+0x173/0x41e
[ 65.609563] #2: (net_mutex){+.+.+.}, at: [<ffffffff817816c9>]
cleanup_net+0x85/0x18a
[ 65.609569] #3: (rtnl_mutex){+.+.+.}, at: [<ffffffff81796058>]
rtnl_lock+0x17/0x19
[ 65.609575] #4: (&(&idst->lock)->rlock){+.....}, at:
[<ffffffff818820fe>] __tunnel_dst_set+0x30/0x8b
[ 65.609575]
[ 65.609575] stack backtrace:
[ 65.609577] CPU: 1 PID: 6 Comm: kworker/u8:0 Not tainted 3.13.0-rc6+ #139
[ 65.609579] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 65.609582] Workqueue: netns cleanup_net
[ 65.609585] 0000000000000000 ffff880118205bb0 ffffffff819894b6
ffff8801181ea550
[ 65.609587] ffff880118205be0 ffffffff81095f34 0000000000000000
ffffe8ffff606df0
[ 65.609589] ffffe8ffff606df8 0000000000000000 ffff880118205c10
ffffffff8188213c
[ 65.609590] Call Trace:
[ 65.609595] [<ffffffff819894b6>] dump_stack+0x4d/0x66
[ 65.609598] [<ffffffff81095f34>] lockdep_rcu_suspicious+0x107/0x110
[ 65.609600] [<ffffffff8188213c>] __tunnel_dst_set+0x6e/0x8b
[ 65.609602] [<ffffffff81882203>] tunnel_dst_reset_all.isra.25+0x40/0x47
[ 65.609605] [<ffffffff81883e28>] ip_tunnel_uninit+0x61/0x64
[ 65.609608] [<ffffffff8178aeb2>] rollback_registered_many+0x188/0x21f
[ 65.609610] [<ffffffff8178b02f>] unregister_netdevice_many+0x19/0x63
[ 65.609612] [<ffffffff81883a17>] ip_tunnel_delete_net+0xd1/0xe5
[ 65.609615] [<ffffffff8188bc68>] vti_exit_net+0x2a/0x2c
[ 65.609617] [<ffffffff81780f21>] ops_exit_list+0x44/0x55
[ 65.609619] [<ffffffff8178173a>] cleanup_net+0xf6/0x18a
[ 65.609621] [<ffffffff8106d612>] process_one_work+0x21a/0x41e
[ 65.609623] [<ffffffff8106d56b>] ? process_one_work+0x173/0x41e
[ 65.609625] [<ffffffff8106dc1b>] worker_thread+0x149/0x1f5
[ 65.609628] [<ffffffff8106dad2>] ? rescuer_thread+0x28d/0x28d
[ 65.609630] [<ffffffff81073f6f>] kthread+0xd2/0xda
[ 65.609634] [<ffffffff8107d0ed>] ? finish_task_switch+0x3a/0xbe
[ 65.609636] [<ffffffff81073e9d>] ? __kthread_parkme+0x61/0x61
[ 65.609640] [<ffffffff819961ac>] ret_from_fork+0x7c/0xb0
[ 65.609642] [<ffffffff81073e9d>] ? __kthread_parkme+0x61/0x61
I think the patch below could fix it but not sure if we really need
synchronize_rcu() here...
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index d3929a6..eb50b9a 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -77,10 +77,11 @@ static inline void __tunnel_dst_set(struct
ip_tunnel_dst *idst,
dst = NULL;
spin_lock_bh(&idst->lock);
- old_dst = rcu_dereference(idst->dst);
+ old_dst = idst->dst;
rcu_assign_pointer(idst->dst, dst);
- dst_release(old_dst);
spin_unlock_bh(&idst->lock);
+ synchronize_rcu();
+ dst_release(old_dst);
}
static inline void tunnel_dst_set(struct ip_tunnel *t, struct dst_entry *dst)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: suspicious RCU usage in net/ipv4/ip_tunnel.c:80
2014-01-11 0:37 suspicious RCU usage in net/ipv4/ip_tunnel.c:80 Cong Wang
@ 2014-01-11 1:21 ` Eric Dumazet
2014-01-11 19:15 ` Cong Wang
2014-01-13 19:35 ` Cong Wang
0 siblings, 2 replies; 10+ messages in thread
From: Eric Dumazet @ 2014-01-11 1:21 UTC (permalink / raw)
To: Cong Wang; +Cc: Tom Herbert, netdev
On Fri, 2014-01-10 at 16:37 -0800, Cong Wang wrote:
> Hi, Tom
>
> I saw:
>
> [ 65.609535] [ INFO: suspicious RCU usage. ]
> [ 65.609537] 3.13.0-rc6+ #139 Not tainted
> [ 65.609538] -------------------------------
> [ 65.609539] net/ipv4/ip_tunnel.c:80 suspicious rcu_dereference_check() usage!
> [ 65.609540]
> [ 65.609540] other info that might help us debug this:
> [ 65.609540]
> [ 65.609541]
> [ 65.609541] rcu_scheduler_active = 1, debug_locks = 0
> [ 65.609542] 5 locks held by kworker/u8:0/6:
> [ 65.609553] #0: (%s#4){.+.+.+}, at: [<ffffffff8106d56b>]
> process_one_work+0x173/0x41e
> [ 65.609557] #1: (net_cleanup_work){+.+.+.}, at:
> [<ffffffff8106d56b>] process_one_work+0x173/0x41e
> [ 65.609563] #2: (net_mutex){+.+.+.}, at: [<ffffffff817816c9>]
> cleanup_net+0x85/0x18a
> [ 65.609569] #3: (rtnl_mutex){+.+.+.}, at: [<ffffffff81796058>]
> rtnl_lock+0x17/0x19
> [ 65.609575] #4: (&(&idst->lock)->rlock){+.....}, at:
> [<ffffffff818820fe>] __tunnel_dst_set+0x30/0x8b
> [ 65.609575]
> [ 65.609575] stack backtrace:
> [ 65.609577] CPU: 1 PID: 6 Comm: kworker/u8:0 Not tainted 3.13.0-rc6+ #139
> [ 65.609579] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [ 65.609582] Workqueue: netns cleanup_net
> [ 65.609585] 0000000000000000 ffff880118205bb0 ffffffff819894b6
> ffff8801181ea550
> [ 65.609587] ffff880118205be0 ffffffff81095f34 0000000000000000
> ffffe8ffff606df0
> [ 65.609589] ffffe8ffff606df8 0000000000000000 ffff880118205c10
> ffffffff8188213c
> [ 65.609590] Call Trace:
> [ 65.609595] [<ffffffff819894b6>] dump_stack+0x4d/0x66
> [ 65.609598] [<ffffffff81095f34>] lockdep_rcu_suspicious+0x107/0x110
> [ 65.609600] [<ffffffff8188213c>] __tunnel_dst_set+0x6e/0x8b
> [ 65.609602] [<ffffffff81882203>] tunnel_dst_reset_all.isra.25+0x40/0x47
> [ 65.609605] [<ffffffff81883e28>] ip_tunnel_uninit+0x61/0x64
> [ 65.609608] [<ffffffff8178aeb2>] rollback_registered_many+0x188/0x21f
> [ 65.609610] [<ffffffff8178b02f>] unregister_netdevice_many+0x19/0x63
> [ 65.609612] [<ffffffff81883a17>] ip_tunnel_delete_net+0xd1/0xe5
> [ 65.609615] [<ffffffff8188bc68>] vti_exit_net+0x2a/0x2c
> [ 65.609617] [<ffffffff81780f21>] ops_exit_list+0x44/0x55
> [ 65.609619] [<ffffffff8178173a>] cleanup_net+0xf6/0x18a
> [ 65.609621] [<ffffffff8106d612>] process_one_work+0x21a/0x41e
> [ 65.609623] [<ffffffff8106d56b>] ? process_one_work+0x173/0x41e
> [ 65.609625] [<ffffffff8106dc1b>] worker_thread+0x149/0x1f5
> [ 65.609628] [<ffffffff8106dad2>] ? rescuer_thread+0x28d/0x28d
> [ 65.609630] [<ffffffff81073f6f>] kthread+0xd2/0xda
> [ 65.609634] [<ffffffff8107d0ed>] ? finish_task_switch+0x3a/0xbe
> [ 65.609636] [<ffffffff81073e9d>] ? __kthread_parkme+0x61/0x61
> [ 65.609640] [<ffffffff819961ac>] ret_from_fork+0x7c/0xb0
> [ 65.609642] [<ffffffff81073e9d>] ? __kthread_parkme+0x61/0x61
>
>
> I think the patch below could fix it but not sure if we really need
> synchronize_rcu() here...
>
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index d3929a6..eb50b9a 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -77,10 +77,11 @@ static inline void __tunnel_dst_set(struct
> ip_tunnel_dst *idst,
> dst = NULL;
>
> spin_lock_bh(&idst->lock);
> - old_dst = rcu_dereference(idst->dst);
> + old_dst = idst->dst;
> rcu_assign_pointer(idst->dst, dst);
> - dst_release(old_dst);
> spin_unlock_bh(&idst->lock);
> + synchronize_rcu();
> + dst_release(old_dst);
> }
Nope, the synchronize_rcu() is not needed here.
Please use following sparse ready patch, thanks :
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index d3929a69f008..6eda759b5c4b 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -77,10 +77,11 @@ static inline void __tunnel_dst_set(struct ip_tunnel_dst *idst,
dst = NULL;
spin_lock_bh(&idst->lock);
- old_dst = rcu_dereference(idst->dst);
+ old_dst = rcu_dereference_protected(idst->dst,
+ lockdep_is_held(&idst->lock));
rcu_assign_pointer(idst->dst, dst);
- dst_release(old_dst);
spin_unlock_bh(&idst->lock);
+ dst_release(old_dst);
}
static inline void tunnel_dst_set(struct ip_tunnel *t, struct dst_entry *dst)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: suspicious RCU usage in net/ipv4/ip_tunnel.c:80
2014-01-11 1:21 ` Eric Dumazet
@ 2014-01-11 19:15 ` Cong Wang
2014-01-12 17:44 ` Eric Dumazet
2014-01-13 19:35 ` Cong Wang
1 sibling, 1 reply; 10+ messages in thread
From: Cong Wang @ 2014-01-11 19:15 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Tom Herbert, netdev
On Fri, Jan 10, 2014 at 5:21 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Nope, the synchronize_rcu() is not needed here.
OK.
>
> Please use following sparse ready patch, thanks :
>
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index d3929a69f008..6eda759b5c4b 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -77,10 +77,11 @@ static inline void __tunnel_dst_set(struct ip_tunnel_dst *idst,
> dst = NULL;
>
> spin_lock_bh(&idst->lock);
> - old_dst = rcu_dereference(idst->dst);
> + old_dst = rcu_dereference_protected(idst->dst,
> + lockdep_is_held(&idst->lock));
> rcu_assign_pointer(idst->dst, dst);
> - dst_release(old_dst);
> spin_unlock_bh(&idst->lock);
> + dst_release(old_dst);
Do you really need a rcu_dereference*() here? We don't dereference
it inside spin_lock protection.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: suspicious RCU usage in net/ipv4/ip_tunnel.c:80
2014-01-11 19:15 ` Cong Wang
@ 2014-01-12 17:44 ` Eric Dumazet
2014-01-13 6:36 ` Cong Wang
0 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2014-01-12 17:44 UTC (permalink / raw)
To: Cong Wang; +Cc: Tom Herbert, netdev
On Sat, 2014-01-11 at 11:15 -0800, Cong Wang wrote:
> On Fri, Jan 10, 2014 at 5:21 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > Nope, the synchronize_rcu() is not needed here.
>
> OK.
>
> >
> > Please use following sparse ready patch, thanks :
> >
> > diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> > index d3929a69f008..6eda759b5c4b 100644
> > --- a/net/ipv4/ip_tunnel.c
> > +++ b/net/ipv4/ip_tunnel.c
> > @@ -77,10 +77,11 @@ static inline void __tunnel_dst_set(struct ip_tunnel_dst *idst,
> > dst = NULL;
> >
> > spin_lock_bh(&idst->lock);
> > - old_dst = rcu_dereference(idst->dst);
> > + old_dst = rcu_dereference_protected(idst->dst,
> > + lockdep_is_held(&idst->lock));
> > rcu_assign_pointer(idst->dst, dst);
> > - dst_release(old_dst);
> > spin_unlock_bh(&idst->lock);
> > + dst_release(old_dst);
>
> Do you really need a rcu_dereference*() here? We don't dereference
> it inside spin_lock protection.
Please read rcu_dereference_protected() documentation in
include/linux/rcupdate.h
Also you can run sparse, with CONFIG_SPARSE_RCU_POINTER=y in
your .config
make C=2 net/ipv4/ip_tunnel.o
And then you'll know the answer to this question.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: suspicious RCU usage in net/ipv4/ip_tunnel.c:80
2014-01-12 17:44 ` Eric Dumazet
@ 2014-01-13 6:36 ` Cong Wang
2014-01-13 8:20 ` Eric Dumazet
0 siblings, 1 reply; 10+ messages in thread
From: Cong Wang @ 2014-01-13 6:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Tom Herbert, netdev
On Sun, Jan 12, 2014 at 9:44 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2014-01-11 at 11:15 -0800, Cong Wang wrote:
>> On Fri, Jan 10, 2014 at 5:21 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> >
>> > Nope, the synchronize_rcu() is not needed here.
>>
>> OK.
>>
>> >
>> > Please use following sparse ready patch, thanks :
>> >
>> > diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
>> > index d3929a69f008..6eda759b5c4b 100644
>> > --- a/net/ipv4/ip_tunnel.c
>> > +++ b/net/ipv4/ip_tunnel.c
>> > @@ -77,10 +77,11 @@ static inline void __tunnel_dst_set(struct ip_tunnel_dst *idst,
>> > dst = NULL;
>> >
>> > spin_lock_bh(&idst->lock);
>> > - old_dst = rcu_dereference(idst->dst);
>> > + old_dst = rcu_dereference_protected(idst->dst,
>> > + lockdep_is_held(&idst->lock));
>> > rcu_assign_pointer(idst->dst, dst);
>> > - dst_release(old_dst);
>> > spin_unlock_bh(&idst->lock);
>> > + dst_release(old_dst);
>>
>> Do you really need a rcu_dereference*() here? We don't dereference
>> it inside spin_lock protection.
>
> Please read rcu_dereference_protected() documentation in
> include/linux/rcupdate.h
I did before I replied.
>
> Also you can run sparse, with CONFIG_SPARSE_RCU_POINTER=y in
> your .config
>
> make C=2 net/ipv4/ip_tunnel.o
>
> And then you'll know the answer to this question.
>
Sounds like it is only to shut up a sparse warning, then its name
is misleading, we clearly don't dereference it here.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: suspicious RCU usage in net/ipv4/ip_tunnel.c:80
2014-01-13 6:36 ` Cong Wang
@ 2014-01-13 8:20 ` Eric Dumazet
2014-01-14 4:53 ` Paul E. McKenney
0 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2014-01-13 8:20 UTC (permalink / raw)
To: Cong Wang; +Cc: Tom Herbert, netdev
On Sun, 2014-01-12 at 22:36 -0800, Cong Wang wrote:
> > Please read rcu_dereference_protected() documentation in
> > include/linux/rcupdate.h
>
> I did before I replied.
>
> >
> > Also you can run sparse, with CONFIG_SPARSE_RCU_POINTER=y in
> > your .config
> >
> > make C=2 net/ipv4/ip_tunnel.o
> >
> > And then you'll know the answer to this question.
> >
>
> Sounds like it is only to shut up a sparse warning, then its name
> is misleading, we clearly don't dereference it here.
Historical reasons, you should have been there when Paul invented the
name and lazy people like us let him do so !
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b62730baea32f86fe91a7930e4b7ee8d82778b79
You are lucky, there is plenty of documentation, maybe too much..
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: suspicious RCU usage in net/ipv4/ip_tunnel.c:80
2014-01-11 1:21 ` Eric Dumazet
2014-01-11 19:15 ` Cong Wang
@ 2014-01-13 19:35 ` Cong Wang
1 sibling, 0 replies; 10+ messages in thread
From: Cong Wang @ 2014-01-13 19:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Tom Herbert, netdev
On Fri, Jan 10, 2014 at 5:21 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Please use following sparse ready patch, thanks :
>
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index d3929a69f008..6eda759b5c4b 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -77,10 +77,11 @@ static inline void __tunnel_dst_set(struct ip_tunnel_dst *idst,
> dst = NULL;
>
> spin_lock_bh(&idst->lock);
> - old_dst = rcu_dereference(idst->dst);
> + old_dst = rcu_dereference_protected(idst->dst,
> + lockdep_is_held(&idst->lock));
> rcu_assign_pointer(idst->dst, dst);
> - dst_release(old_dst);
> spin_unlock_bh(&idst->lock);
> + dst_release(old_dst);
> }
>
> static inline void tunnel_dst_set(struct ip_tunnel *t, struct dst_entry *dst)
>
>
Tested-by: Cong Wang <cwang@twopensource.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: suspicious RCU usage in net/ipv4/ip_tunnel.c:80
2014-01-13 8:20 ` Eric Dumazet
@ 2014-01-14 4:53 ` Paul E. McKenney
2014-01-17 0:26 ` Cong Wang
0 siblings, 1 reply; 10+ messages in thread
From: Paul E. McKenney @ 2014-01-14 4:53 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Cong Wang, Tom Herbert, netdev
On Mon, Jan 13, 2014 at 12:20:33AM -0800, Eric Dumazet wrote:
> On Sun, 2014-01-12 at 22:36 -0800, Cong Wang wrote:
> > > Please read rcu_dereference_protected() documentation in
> > > include/linux/rcupdate.h
> >
> > I did before I replied.
>
>
>
> >
> > >
> > > Also you can run sparse, with CONFIG_SPARSE_RCU_POINTER=y in
> > > your .config
> > >
> > > make C=2 net/ipv4/ip_tunnel.o
> > >
> > > And then you'll know the answer to this question.
> > >
> >
> > Sounds like it is only to shut up a sparse warning, then its name
> > is misleading, we clearly don't dereference it here.
OK, I'll bite... This code invokes dst_release() which looks to me
like it dereferences this pointer:
void dst_release(struct dst_entry *dst)
{
if (dst) {
int newrefcnt;
newrefcnt = atomic_dec_return(&dst->__refcnt);
WARN_ON(newrefcnt < 0);
if (unlikely(dst->flags & DST_NOCACHE) && !newrefcnt) {
dst = dst_destroy(dst);
if (dst)
__dst_free(dst);
}
}
}
If you really were not dereferencing the pointer, for example, if you
were only testing it against NULL or some such, then you can use
rcu_access_pointer(). This is a bit cheaper on DEC Alpha, FWIW.
You can think of rcu_dereference() as meaning "fetch this RCU-protected
pointer with intent to dereference()" if that helps.
Or am I missing your point?
> Historical reasons, you should have been there when Paul invented the
> name and lazy people like us let him do so !
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b62730baea32f86fe91a7930e4b7ee8d82778b79
Indeed! ;-)
And rcu_dereference() is at least an improvement over the earlier
hand-placed smp_read_barrier_depends().
Thanx, Paul
> You are lucky, there is plenty of documentation, maybe too much..
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: suspicious RCU usage in net/ipv4/ip_tunnel.c:80
2014-01-14 4:53 ` Paul E. McKenney
@ 2014-01-17 0:26 ` Cong Wang
2014-01-17 0:49 ` Eric Dumazet
0 siblings, 1 reply; 10+ messages in thread
From: Cong Wang @ 2014-01-17 0:26 UTC (permalink / raw)
To: paulmck; +Cc: Eric Dumazet, Tom Herbert, netdev
On Mon, Jan 13, 2014 at 8:53 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> OK, I'll bite... This code invokes dst_release() which looks to me
> like it dereferences this pointer:
>
Yeah, sure, my point is it doesn't dereference the pointer when it holds
the spin_lock. I read rcu_dereference_protected() as "deference the rcu
pointer when we protected by ...", but we dereference it after the lock is
released. :)
@Eric, will you submit your patch as a formal one? I already tested it.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: suspicious RCU usage in net/ipv4/ip_tunnel.c:80
2014-01-17 0:26 ` Cong Wang
@ 2014-01-17 0:49 ` Eric Dumazet
0 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2014-01-17 0:49 UTC (permalink / raw)
To: Cong Wang; +Cc: paulmck, Tom Herbert, netdev
On Thu, 2014-01-16 at 16:26 -0800, Cong Wang wrote:
> On Mon, Jan 13, 2014 at 8:53 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > OK, I'll bite... This code invokes dst_release() which looks to me
> > like it dereferences this pointer:
> >
>
> Yeah, sure, my point is it doesn't dereference the pointer when it holds
> the spin_lock. I read rcu_dereference_protected() as "deference the rcu
> pointer when we protected by ...", but we dereference it after the lock is
> released. :)
See rcu_dereference_protected() as rcu_fetch_protected()
We read/fetch the pointer, but wont dereference the memory pointer by
this pointer later.
>
>
> @Eric, will you submit your patch as a formal one? I already tested it.
I actually sent another patch :
http://patchwork.ozlabs.org/patch/311902/
Thanks !
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-01-17 0:49 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-11 0:37 suspicious RCU usage in net/ipv4/ip_tunnel.c:80 Cong Wang
2014-01-11 1:21 ` Eric Dumazet
2014-01-11 19:15 ` Cong Wang
2014-01-12 17:44 ` Eric Dumazet
2014-01-13 6:36 ` Cong Wang
2014-01-13 8:20 ` Eric Dumazet
2014-01-14 4:53 ` Paul E. McKenney
2014-01-17 0:26 ` Cong Wang
2014-01-17 0:49 ` Eric Dumazet
2014-01-13 19:35 ` Cong Wang
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.