Netdev List
 help / color / mirror / Atom feed
* softirq lockdep trace when ethernet (tg3) brought up.
@ 2013-11-14 19:16 Dave Jones
  2013-11-14 19:41 ` Eric Dumazet
  0 siblings, 1 reply; 6+ messages in thread
From: Dave Jones @ 2013-11-14 19:16 UTC (permalink / raw)
  To: netdev

See this during boot on one of my test boxes:
Not sure why it only shows up on that machine. TG3 specific ?

=================================
[ INFO: inconsistent lock state ]
3.12.0+ #1 Not tainted
---------------------------------
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
swapper/1/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
 (&af_inet_stats->syncp.seq){+.?...}, at: [<c14d18f8>] __netif_receive_skb_core+0x85d/0xb94
{SOFTIRQ-ON-W} state was registered at:
  [<c1090dfa>] __lock_acquire+0x558/0x1808
  [<c10926cf>] lock_acquire+0x7d/0x18b
  [<c1534cce>] ip4_datagram_connect+0x38e/0x3b2
  [<c1544479>] inet_dgram_connect+0x2a/0x68
  [<c14ba636>] SYSC_connect+0xaf/0xc9
  [<c14bb422>] SYSC_socketcall+0x20d/0x937
  [<c14bbc0b>] SyS_socketcall+0x13/0x15
  [<c15f33bb>] sysenter_do_call+0x12/0x32
irq event stamp: 147986
hardirqs last  enabled at (147986): [<c15ebcb7>] _raw_spin_unlock_irqrestore+0x4e/0x59
hardirqs last disabled at (147985): [<c15eba8f>] _raw_spin_lock_irqsave+0x18/0x7e
softirqs last  enabled at (147960): [<c1045a79>] _local_bh_enable+0x1f/0x42
softirqs last disabled at (147961): [<c100414b>] do_softirq_own_stack+0x2e/0x34

other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(&af_inet_stats->syncp.seq);
   <Interrupt>
     lock(&af_inet_stats->syncp.seq);
 
 *** DEADLOCK ***

1 lock held by swapper/1/0:
 #0:  (rcu_read_lock){.+.+..}, at: [<c14d11ac>] __netif_receive_skb_core+0x111/0xb94

stack backtrace:
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.12.0+ #1 
Hardware name: Dell Inc.                 Precision WorkStation 490    /0DT031, BIOS A08 04/25/2008
 c1d53dc0 00000000 efe95ce0 c15e3b8e efe44260 efe95d1c c15e06f1 c178edb4
 c178f110 00000000 00000000 00000000 00000001 00000001 00000001 00000000
 c178f110 00000004 efe4475c 00000010 efe95d50 c109031d 00000004 c1077fc8
Call Trace:
 [<c15e3b8e>] dump_stack+0x4b/0x75
 [<c15e06f1>] print_usage_bug+0x1cf/0x1d9
 [<c109031d>] mark_lock+0x1ea/0x26b
 [<c1077fc8>] ? sched_clock_local+0x42/0x12e
 [<c108fa61>] ? check_usage_backwards+0x109/0x109
 [<c1090db5>] __lock_acquire+0x513/0x1808
 [<c1078223>] ? sched_clock_cpu+0xcd/0x130
 [<c109041f>] ? mark_held_locks+0x81/0xe7
 [<c108d6e5>] ? trace_hardirqs_off+0xb/0xd
 [<c108dd1e>] ? put_lock_stats.isra.30+0xd/0x20
 [<c108e119>] ? lock_release_holdtime.part.31+0x8b/0xd4
 [<c10926cf>] lock_acquire+0x7d/0x18b
 [<c14d18f8>] ? __netif_receive_skb_core+0x85d/0xb94
 [<c150a1aa>] ip_rcv+0x6a/0x5d0
 [<c14d18f8>] ? __netif_receive_skb_core+0x85d/0xb94
 [<c14d18f8>] __netif_receive_skb_core+0x85d/0xb94
 [<c14d11ac>] ? __netif_receive_skb_core+0x111/0xb94
 [<c14d1c45>] __netif_receive_skb+0x16/0x51
 [<c14d1c9f>] netif_receive_skb+0x1f/0x1a7
 [<c14d26b3>] napi_gro_receive+0x5f/0x7e
 [<f8533cfe>] tg3_poll_work+0xc61/0xeaa [tg3]
 [<c108d6e5>] ? trace_hardirqs_off+0xb/0xd
 [<f853ed4c>] tg3_poll+0x6a/0x314 [tg3]
 [<c1090523>] ? trace_hardirqs_on_caller+0x9e/0x1aa
 [<c14d2888>] net_rx_action+0x11a/0x29a
 [<c1046c78>] __do_softirq+0xd2/0x38d
 [<c1046ba6>] ? __tasklet_schedule+0x131/0x131
 <IRQ>  [<c104725a>] ? irq_exit+0xa8/0xb0
 [<c15f39d5>] ? do_IRQ+0x45/0xb0
 [<c10ad1e5>] ? rcu_irq_exit+0x5a/0x93
 [<c15f38b5>] ? common_interrupt+0x35/0x3c
 [<c109007b>] ? print_irqtrace_events+0x2d/0xe5
 [<c100976e>] ? default_idle+0x1e/0x210
 [<c100a0b6>] ? arch_cpu_idle+0x24/0x26
 [<c10a283c>] ? cpu_startup_entry+0x219/0x39d
 [<c1028032>] ? setup_APIC_timer+0xa5/0x102
 [<c10266d8>] ? start_secondary+0x22e/0x340

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: softirq lockdep trace when ethernet (tg3) brought up.
  2013-11-14 19:16 softirq lockdep trace when ethernet (tg3) brought up Dave Jones
@ 2013-11-14 19:41 ` Eric Dumazet
  2013-11-14 21:37   ` [PATCH] ipv4: fix possible seqlock deadlock Eric Dumazet
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2013-11-14 19:41 UTC (permalink / raw)
  To: Dave Jones; +Cc: netdev

On Thu, 2013-11-14 at 14:16 -0500, Dave Jones wrote:
> See this during boot on one of my test boxes:
> Not sure why it only shows up on that machine. TG3 specific ?

Not tg3 specific, 32bit arches specific ;)

Thanks for the report, I'll take a look.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ipv4: fix possible seqlock deadlock
  2013-11-14 19:41 ` Eric Dumazet
@ 2013-11-14 21:37   ` Eric Dumazet
  2013-11-14 21:44     ` Dave Jones
  2013-11-14 22:32     ` David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2013-11-14 21:37 UTC (permalink / raw)
  To: Dave Jones, David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

ip4_datagram_connect() being called from process context,
it should use IP_INC_STATS() instead of IP_INC_STATS_BH()
otherwise we can deadlock on 32bit arches, or get corruptions of
SNMP counters.

Fixes: 584bdf8cbdf6 ("[IPV4]: Fix "ipOutNoRoutes" counter error for TCP and UDP")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Dave Jones <davej@redhat.com>
---
 net/ipv4/datagram.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c
index b28e863fe0a7..19e36376d2a0 100644
--- a/net/ipv4/datagram.c
+++ b/net/ipv4/datagram.c
@@ -57,7 +57,7 @@ int ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	if (IS_ERR(rt)) {
 		err = PTR_ERR(rt);
 		if (err == -ENETUNREACH)
-			IP_INC_STATS_BH(sock_net(sk), IPSTATS_MIB_OUTNOROUTES);
+			IP_INC_STATS(sock_net(sk), IPSTATS_MIB_OUTNOROUTES);
 		goto out;
 	}
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ipv4: fix possible seqlock deadlock
  2013-11-14 21:37   ` [PATCH] ipv4: fix possible seqlock deadlock Eric Dumazet
@ 2013-11-14 21:44     ` Dave Jones
  2013-11-14 21:52       ` Eric Dumazet
  2013-11-14 22:32     ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Dave Jones @ 2013-11-14 21:44 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev

On Thu, Nov 14, 2013 at 01:37:54PM -0800, Eric Dumazet wrote:
 > From: Eric Dumazet <edumazet@google.com>
 > 
 > ip4_datagram_connect() being called from process context,
 > it should use IP_INC_STATS() instead of IP_INC_STATS_BH()
 > otherwise we can deadlock on 32bit arches, or get corruptions of
 > SNMP counters.
 > 
 > Fixes: 584bdf8cbdf6 ("[IPV4]: Fix "ipOutNoRoutes" counter error for TCP and UDP")

whoa, 2007. Any idea why I only just started hitting this ?

	Dave

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ipv4: fix possible seqlock deadlock
  2013-11-14 21:44     ` Dave Jones
@ 2013-11-14 21:52       ` Eric Dumazet
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2013-11-14 21:52 UTC (permalink / raw)
  To: Dave Jones; +Cc: David Miller, netdev

On Thu, 2013-11-14 at 16:44 -0500, Dave Jones wrote:
> On Thu, Nov 14, 2013 at 01:37:54PM -0800, Eric Dumazet wrote:
>  > From: Eric Dumazet <edumazet@google.com>
>  > 
>  > ip4_datagram_connect() being called from process context,
>  > it should use IP_INC_STATS() instead of IP_INC_STATS_BH()
>  > otherwise we can deadlock on 32bit arches, or get corruptions of
>  > SNMP counters.
>  > 
>  > Fixes: 584bdf8cbdf6 ("[IPV4]: Fix "ipOutNoRoutes" counter error for TCP and UDP")
> 
> whoa, 2007. Any idea why I only just started hitting this ?
> 

lockdep support was added very recently.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ipv4: fix possible seqlock deadlock
  2013-11-14 21:37   ` [PATCH] ipv4: fix possible seqlock deadlock Eric Dumazet
  2013-11-14 21:44     ` Dave Jones
@ 2013-11-14 22:32     ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2013-11-14 22:32 UTC (permalink / raw)
  To: eric.dumazet; +Cc: davej, netdev

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 14 Nov 2013 13:37:54 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> ip4_datagram_connect() being called from process context,
> it should use IP_INC_STATS() instead of IP_INC_STATS_BH()
> otherwise we can deadlock on 32bit arches, or get corruptions of
> SNMP counters.
> 
> Fixes: 584bdf8cbdf6 ("[IPV4]: Fix "ipOutNoRoutes" counter error for TCP and UDP")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Dave Jones <davej@redhat.com>

Applied and queued up for -stable, thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-11-14 22:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-14 19:16 softirq lockdep trace when ethernet (tg3) brought up Dave Jones
2013-11-14 19:41 ` Eric Dumazet
2013-11-14 21:37   ` [PATCH] ipv4: fix possible seqlock deadlock Eric Dumazet
2013-11-14 21:44     ` Dave Jones
2013-11-14 21:52       ` Eric Dumazet
2013-11-14 22:32     ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox