Prev: uml: Remove unused variable from line driver
Next: [PATCH] Mass Storage Gadget: Handle eject request
From: Paul E. McKenney on 26 Apr 2010 12:10
On Sun, Apr 25, 2010 at 04:20:13PM -0400, Miles Lane wrote:
> > I am down to seeing three suspicious rcu_dereference_check traces when
> > I apply this patch and all the previous patches to 2.6.34-rc5-git6.
> >
> > 1. The "__sched_setscheduler+0x19d/0x300" trace.
> > 2. The two "is_swiotlb_buffer+0x2e/0x3b" traces (waiting to see
> > Johannes' patch show up in a Linux snapshot)
> >
> > Did I miss a patch for the setscheduler issue?
>
> Hmm. I am still seeing these two messages as well.
>
> [ 83.363146] [ INFO: suspicious rcu_dereference_check() usage. ]
> [ 83.363148] ---------------------------------------------------
> [ 83.363151] include/net/inet_timewait_sock.h:227 invoked
> rcu_dereference_check() without protection!
> [ 83.363154]
> [ 83.363155] other info that might help us debug this:
> [ 83.363156]
> [ 83.363158]
> [ 83.363159] rcu_scheduler_active = 1, debug_locks = 1
> [ 83.363162] 2 locks held by gwibber-service/5076:
> [ 83.363164] #0: (&p->lock){+.+.+.}, at: [<ffffffff8110534a>]
> seq_read+0x37/0x381
> [ 83.363176] #1: (&(&hashinfo->ehash_locks[i])->rlock){+.-...},
> at: [<ffffffff813ddcd5>] established_get_next+0xc4/0x132
> [ 83.363186]
> [ 83.363187] stack backtrace:
> [ 83.363191] Pid: 5076, comm: gwibber-service Not tainted 2.6.34-rc5-git6 #27
> [ 83.363194] Call Trace:
> [ 83.363202] [<ffffffff81068086>] lockdep_rcu_dereference+0x9d/0xa5
> [ 83.363207] [<ffffffff813dc998>] twsk_net+0x4f/0x57
> [ 83.363212] [<ffffffff813ddc65>] established_get_next+0x54/0x132
> [ 83.363216] [<ffffffff813dde47>] tcp_seq_next+0x5d/0x6a
> [ 83.363221] [<ffffffff81105599>] seq_read+0x286/0x381
> [ 83.363226] [<ffffffff81105313>] ? seq_read+0x0/0x381
> [ 83.363231] [<ffffffff8113503c>] proc_reg_read+0x8d/0xac
> [ 83.363236] [<ffffffff810ebf14>] vfs_read+0xa6/0x103
> [ 83.363241] [<ffffffff810ec027>] sys_read+0x45/0x69
> [ 83.363246] [<ffffffff81002b6b>] system_call_fastpath+0x16/0x1b
>
> [ 84.660302] [ INFO: suspicious rcu_dereference_check() usage. ]
> [ 84.660304] ---------------------------------------------------
> [ 84.660308] include/net/inet_timewait_sock.h:227 invoked
> rcu_dereference_check() without protection!
> [ 84.660311]
> [ 84.660312] other info that might help us debug this:
> [ 84.660313]
> [ 84.660315]
> [ 84.660316] rcu_scheduler_active = 1, debug_locks = 1
> [ 84.660319] no locks held by gwibber-service/5081.
> [ 84.660321]
> [ 84.660322] stack backtrace:
> [ 84.660325] Pid: 5081, comm: gwibber-service Not tainted 2.6.34-rc5-git6 #27
> [ 84.660328] Call Trace:
> [ 84.660339] [<ffffffff81068086>] lockdep_rcu_dereference+0x9d/0xa5
> [ 84.660345] [<ffffffff813cad6f>] twsk_net+0x4f/0x57
> [ 84.660350] [<ffffffff813cb18f>] __inet_twsk_hashdance+0x50/0x158
> [ 84.660355] [<ffffffff813e0bb9>] tcp_time_wait+0x1c1/0x24b
> [ 84.660360] [<ffffffff813d3d97>] tcp_fin+0x83/0x162
> [ 84.660364] [<ffffffff813d4727>] tcp_data_queue+0x1ff/0xa1e
> [ 84.660370] [<ffffffff810496aa>] ? mod_timer+0x1e/0x20
> [ 84.660375] [<ffffffff813d8363>] tcp_rcv_state_process+0x89d/0x8f2
> [ 84.660381] [<ffffffff813943bb>] ? release_sock+0x30/0x10b
> [ 84.660386] [<ffffffff813de772>] tcp_v4_do_rcv+0x2de/0x33f
> [ 84.660391] [<ffffffff8139440d>] release_sock+0x82/0x10b
> [ 84.660395] [<ffffffff813ce875>] tcp_close+0x1b5/0x37e
> [ 84.660401] [<ffffffff813ecdb7>] inet_release+0x50/0x57
> [ 84.660405] [<ffffffff81391ae4>] sock_release+0x1a/0x66
> [ 84.660410] [<ffffffff81391b52>] sock_close+0x22/0x26
> [ 84.660415] [<ffffffff810ece07>] __fput+0x120/0x1cd
> [ 84.660420] [<ffffffff810ecec9>] fput+0x15/0x17
> [ 84.660424] [<ffffffff810e9d41>] filp_close+0x63/0x6d
> [ 84.660428] [<ffffffff810e9e22>] sys_close+0xd7/0x111
> [ 84.660434] [<ffffffff81002b6b>] system_call_fastpath+0x16/0x1b

Eric Dumazet traced these down to a commit from Eric Biederman.

If I don't hear from Eric Biederman in a few days, I will attempt a
patch, but it would be more likely to be correct coming from someone
with a better understanding of the code. ;-)

Thanx, Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Paul E. McKenney on 27 Apr 2010 00:30
On Mon, Apr 26, 2010 at 11:35:10AM -0700, Eric W. Biederman wrote:
> "Paul E. McKenney" <paulmck(a)linux.vnet.ibm.com> writes:
>
> > Eric Dumazet traced these down to a commit from Eric Biederman.
> >
> > If I don't hear from Eric Biederman in a few days, I will attempt a
> > patch, but it would be more likely to be correct coming from someone
> > with a better understanding of the code. ;-)
>
> I already replied.
>
> http://lkml.org/lkml/2010/4/21/420

You did indeed!!! This experience is giving me an even better appreciation
of the maintainers' ability to keep all their patches straight!

I will put together something based on your suggestion.

Thanx, Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Paul E. McKenney on 27 Apr 2010 12:30
On Mon, Apr 26, 2010 at 09:27:44PM -0700, Paul E. McKenney wrote:
> On Mon, Apr 26, 2010 at 11:35:10AM -0700, Eric W. Biederman wrote:
> > "Paul E. McKenney" <paulmck(a)linux.vnet.ibm.com> writes:
> >
> > > Eric Dumazet traced these down to a commit from Eric Biederman.
> > >
> > > If I don't hear from Eric Biederman in a few days, I will attempt a
> > > patch, but it would be more likely to be correct coming from someone
> > > with a better understanding of the code. ;-)
> >
> > I already replied.
> >
> > http://lkml.org/lkml/2010/4/21/420
>
> You did indeed!!! This experience is giving me an even better appreciation
> of the maintainers' ability to keep all their patches straight!
>
> I will put together something based on your suggestion.

How about the following?

Thanx, Paul

------------------------------------------------------------------------

commit 85fa42bd568ab99c375f018761ae6345249942cd
Author: Paul E. McKenney <paulmck(a)linux.vnet.ibm.com>
Date: Mon Apr 26 21:40:05 2010 -0700

net: suppress RCU lockdep false positive in twsk_net()

Calls to twsk_net() are in some cases protected by reference counting
as an alternative to RCU protection. Cases covered by reference counts
include __inet_twsk_kill(), inet_twsk_free(), inet_twdr_do_twkill_work(),
inet_twdr_twcal_tick(), and tcp_timewait_state_process(). RCU is used
by inet_twsk_purge(). Locking is used by established_get_first()
and established_get_next(). Finally, __inet_twsk_hashdance() is an
initialization case.

It appears to be non-trivial to locate the appropriate locks and
reference counts from within twsk_net(), so used rcu_dereference_raw().

Signed-off-by: Paul E. McKenney <paulmck(a)linux.vnet.ibm.com>

diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
index 79f67ea..a066fdd 100644
--- a/include/net/inet_timewait_sock.h
+++ b/include/net/inet_timewait_sock.h
@@ -224,7 +224,9 @@ static inline
struct net *twsk_net(const struct inet_timewait_sock *twsk)
{
#ifdef CONFIG_NET_NS
- return rcu_dereference(twsk->tw_net);
+ return rcu_dereference_raw(twsk->tw_net); /* protected by locking, */
+ /* reference counting, */
+ /* initialization, or RCU. */
#else
return &init_net;
#endif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Eric Dumazet on 27 Apr 2010 12:50
Le mardi 27 avril 2010 à 09:22 -0700, Paul E. McKenney a écrit :
> On Mon, Apr 26, 2010 at 09:27:44PM -0700, Paul E. McKenney wrote:
> > On Mon, Apr 26, 2010 at 11:35:10AM -0700, Eric W. Biederman wrote:
> > > "Paul E. McKenney" <paulmck(a)linux.vnet.ibm.com> writes:
> > >
> > > > Eric Dumazet traced these down to a commit from Eric Biederman.
> > > >
> > > > If I don't hear from Eric Biederman in a few days, I will attempt a
> > > > patch, but it would be more likely to be correct coming from someone
> > > > with a better understanding of the code. ;-)
> > >
> > > I already replied.
> > >
> > > http://lkml.org/lkml/2010/4/21/420
> >
> > You did indeed!!! This experience is giving me an even better appreciation
> > of the maintainers' ability to keep all their patches straight!
> >
> > I will put together something based on your suggestion.
>
> How about the following?
>
> Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit 85fa42bd568ab99c375f018761ae6345249942cd
> Author: Paul E. McKenney <paulmck(a)linux.vnet.ibm.com>
> Date: Mon Apr 26 21:40:05 2010 -0700
>
> net: suppress RCU lockdep false positive in twsk_net()
>
> Calls to twsk_net() are in some cases protected by reference counting
> as an alternative to RCU protection. Cases covered by reference counts
> include __inet_twsk_kill(), inet_twsk_free(), inet_twdr_do_twkill_work(),
> inet_twdr_twcal_tick(), and tcp_timewait_state_process(). RCU is used
> by inet_twsk_purge(). Locking is used by established_get_first()
> and established_get_next(). Finally, __inet_twsk_hashdance() is an
> initialization case.
>
> It appears to be non-trivial to locate the appropriate locks and
> reference counts from within twsk_net(), so used rcu_dereference_raw().
>
> Signed-off-by: Paul E. McKenney <paulmck(a)linux.vnet.ibm.com>
>
> diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
> index 79f67ea..a066fdd 100644
> --- a/include/net/inet_timewait_sock.h
> +++ b/include/net/inet_timewait_sock.h
> @@ -224,7 +224,9 @@ static inline
> struct net *twsk_net(const struct inet_timewait_sock *twsk)
> {
> #ifdef CONFIG_NET_NS
> - return rcu_dereference(twsk->tw_net);
> + return rcu_dereference_raw(twsk->tw_net); /* protected by locking, */
> + /* reference counting, */
> + /* initialization, or RCU. */
> #else
> return &init_net;
> #endif
> --

Acked-by: Eric Dumazet <eric.dumazet(a)gmail.com>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Paul E. McKenney on 27 Apr 2010 19:40
On Tue, Apr 27, 2010 at 01:58:30PM -0400, Miles Lane wrote:
> On Tue, Apr 27, 2010 at 12:22 PM, Paul E. McKenney
> <paulmck(a)linux.vnet.ibm.com> wrote:
> > On Mon, Apr 26, 2010 at 09:27:44PM -0700, Paul E. McKenney wrote:
> >> On Mon, Apr 26, 2010 at 11:35:10AM -0700, Eric W. Biederman wrote:
> >> > "Paul E. McKenney" <paulmck(a)linux.vnet.ibm.com> writes:
> >> >
> >> > > Eric Dumazet traced these down to a commit from Eric Biederman.
> >> > >
> >> > > If I don't hear from Eric Biederman in a few days, I will attempt a
> >> > > patch, but it would be more likely to be correct coming from someone
> >> > > with a better understanding of the code. �;-)
> >> >
> >> > I already replied.
> >> >
> >> > http://lkml.org/lkml/2010/4/21/420
> >>
> >> You did indeed!!! �This experience is giving me an even better appreciation
> >> of the maintainers' ability to keep all their patches straight!
> >>
> >> I will put together something based on your suggestion.
> >
> > How about the following?
> >
> > � � � � � � � � � � � � � � � � � � � � � � � � � � � �Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> > commit 85fa42bd568ab99c375f018761ae6345249942cd
> > Author: Paul E. McKenney <paulmck(a)linux.vnet.ibm.com>
> > Date: � Mon Apr 26 21:40:05 2010 -0700
> >
> > � �net: suppress RCU lockdep false positive in twsk_net()
> >
> > � �Calls to twsk_net() are in some cases protected by reference counting
> > � �as an alternative to RCU protection. �Cases covered by reference counts
> > � �include __inet_twsk_kill(), inet_twsk_free(), inet_twdr_do_twkill_work(),
> > � �inet_twdr_twcal_tick(), and tcp_timewait_state_process(). �RCU is used
> > � �by inet_twsk_purge(). �Locking is used by established_get_first()
> > � �and established_get_next(). �Finally, __inet_twsk_hashdance() is an
> > � �initialization case.
> >
> > � �It appears to be non-trivial to locate the appropriate locks and
> > � �reference counts from within twsk_net(), so used rcu_dereference_raw().
> >
> > � �Signed-off-by: Paul E. McKenney <paulmck(a)linux.vnet.ibm.com>
> >
> > diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
> > index 79f67ea..a066fdd 100644
> > --- a/include/net/inet_timewait_sock.h
> > +++ b/include/net/inet_timewait_sock.h
> > @@ -224,7 +224,9 @@ static inline
> > �struct net *twsk_net(const struct inet_timewait_sock *twsk)
> > �{
> > �#ifdef CONFIG_NET_NS
> > - � � � return rcu_dereference(twsk->tw_net);
> > + � � � return rcu_dereference_raw(twsk->tw_net); /* protected by locking, */
> > + � � � � � � � � � � � � � � � � � � � � � � � � /* reference counting, */
> > + � � � � � � � � � � � � � � � � � � � � � � � � /* initialization, or RCU. */
> > �#else
> > � � � �return &init_net;
> > �#endif
> >
>
> Worked for me. Thanks!

Thank you both! I have added Eric's Acked-by and Miles's Tested-by.

Thanx, Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4 5
Prev: uml: Remove unused variable from line driver
Next: [PATCH] Mass Storage Gadget: Handle eject request