+ ppp_generic-fix-lockdep-warning.patch added to -mm tree

+ ppp_generic-fix-lockdep-warning.patch added to -mm tree

[akpm]

The patch titled
     ppp_generic: fix lockdep warning
has been added to the -mm tree.  Its filename is
     ppp_generic-fix-lockdep-warning.patch

*** Remember to use Documentation/SubmitChecklist when testing your code ***

See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
out what to do about this

------------------------------------------------------
Subject: ppp_generic: fix lockdep warning
From: Jarek Poplawski <jarkao2@o2.pl>

> =======================================================
> [ INFO: possible circular locking dependency detected ]
> 2.6.21-rc4 #1
> -------------------------------------------------------
> pppd/8926 is trying to acquire lock:
> (&vlan_netdev_xmit_lock_key){-...}, at: [<c0265486>]
> dev_queue_xmit+0x247/0x2f1
>
> but task is already holding lock:
> (&pch->downl){-+..}, at: [<c0230c72>] ppp_channel_push+0x19/0x9a
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #3 (&pch->downl){-+..}:
>       [<c013642b>] __lock_acquire+0xe62/0x1010
>       [<c0136642>] lock_acquire+0x69/0x83
>       [<c02afc13>] _spin_lock_bh+0x30/0x3d
>       [<c022f715>] ppp_push+0x5a/0x9a
>       [<c022fb40>] ppp_xmit_process+0x2e/0x511
>       [<c0231a05>] ppp_write+0xb8/0xf2
>       [<c015ec26>] vfs_write+0x7f/0xba
>       [<c015f158>] sys_write+0x3d/0x64
>       [<c01027de>] sysenter_past_esp+0x5f/0x99
>       [<ffffffff>] 0xffffffff
>
> -> #2 (&ppp->wlock){-+..}:
>       [<c013642b>] __lock_acquire+0xe62/0x1010
>       [<c0136642>] lock_acquire+0x69/0x83
>       [<c02afc13>] _spin_lock_bh+0x30/0x3d
>       [<c022fb2b>] ppp_xmit_process+0x19/0x511
>       [<c02318d3>] ppp_start_xmit+0x18a/0x204
>       [<c0263a6f>] dev_hard_start_xmit+0x1f6/0x2c4
>       [<c026ded3>] __qdisc_run+0x81/0x1bc
>       [<c026549e>] dev_queue_xmit+0x25f/0x2f1
>       [<c027c75f>] ip_output+0x1be/0x25f
>       [<c02788ce>] ip_forward+0x159/0x22b
>       [<c027745c>] ip_rcv+0x297/0x4dd
>       [<c0263698>] netif_receive_skb+0x164/0x1f2
>       [<c022199d>] e1000_clean_rx_irq+0x12a/0x4b7
>       [<c02209bc>] e1000_clean+0x3ff/0x5dd
>       [<c0265084>] net_rx_action+0x7d/0x12b
>       [<c011e442>] __do_softirq+0x82/0xf2
>       [<c011e509>] do_softirq+0x57/0x59
>       [<c011e877>] irq_exit+0x7f/0x81
>       [<c0105011>] do_IRQ+0x45/0x84
>       [<c0103252>] common_interrupt+0x2e/0x34
>       [<c0100b66>] mwait_idle+0x12/0x14
>       [<c0100c60>] cpu_idle+0x6c/0x86
>       [<c01001cd>] rest_init+0x23/0x36
>       [<c0377d89>] start_kernel+0x3ca/0x461
>       [<00000000>] 0x0
>       [<ffffffff>] 0xffffffff
>
> -> #1 (&dev->_xmit_lock){-+..}:
>       [<c013642b>] __lock_acquire+0xe62/0x1010
>       [<c0136642>] lock_acquire+0x69/0x83
>       [<c02afc13>] _spin_lock_bh+0x30/0x3d
>       [<c0266861>] dev_mc_add+0x34/0x16a
>       [<c02ab5c7>] vlan_dev_set_multicast_list+0x88/0x25c
>       [<c0266592>] __dev_mc_upload+0x22/0x24
>       [<c0266914>] dev_mc_add+0xe7/0x16a
>       [<c029f323>] igmp_group_added+0xe6/0xeb
>       [<c029f50b>] ip_mc_inc_group+0x13f/0x210
>       [<c029f5fa>] ip_mc_up+0x1e/0x61
>       [<c029ab81>] inetdev_event+0x154/0x2c7
>       [<c0125a46>] notifier_call_chain+0x2c/0x39
>       [<c0125a7c>] raw_notifier_call_chain+0x8/0xa
>       [<c026477a>] dev_open+0x6d/0x71
>       [<c0263028>] dev_change_flags+0x51/0x101
>       [<c029b7ca>] devinet_ioctl+0x4df/0x644
>       [<c029bc03>] inet_ioctl+0x5c/0x6f
>       [<c02596e0>] sock_ioctl+0x4f/0x1e8
>       [<c0168c32>] do_ioctl+0x22/0x71
>       [<c0168cd6>] vfs_ioctl+0x55/0x27e
>       [<c0168f32>] sys_ioctl+0x33/0x51
>       [<c01027de>] sysenter_past_esp+0x5f/0x99
>       [<ffffffff>] 0xffffffff
>
> -> #0 (&vlan_netdev_xmit_lock_key){-...}:
>       [<c0136289>] __lock_acquire+0xcc0/0x1010
>       [<c0136642>] lock_acquire+0x69/0x83
>       [<c02afbd6>] _spin_lock+0x2b/0x38
>       [<c0265486>] dev_queue_xmit+0x247/0x2f1
>       [<c02334f6>] __pppoe_xmit+0x1a9/0x215
>       [<c023356c>] pppoe_xmit+0xa/0xc
>       [<c0230c9a>] ppp_channel_push+0x41/0x9a
>       [<c0231a13>] ppp_write+0xc6/0xf2
>       [<c015ec26>] vfs_write+0x7f/0xba
>       [<c015f158>] sys_write+0x3d/0x64
>       [<c01027de>] sysenter_past_esp+0x5f/0x99
>       [<ffffffff>] 0xffffffff
>
> other info that might help us debug this:
>
> 1 lock held by pppd/8926:
> #0:  (&pch->downl){-+..}, at: [<c0230c72>] ppp_channel_push+0x19/0x9a
>
> stack backtrace:
> [<c0103834>] show_trace_log_lvl+0x1a/0x30
> [<c0103f16>] show_trace+0x12/0x14
> [<c0103f9d>] dump_stack+0x16/0x18
> [<c01343cd>] print_circular_bug_tail+0x68/0x71
> [<c0136289>] __lock_acquire+0xcc0/0x1010
> [<c0136642>] lock_acquire+0x69/0x83
> [<c02afbd6>] _spin_lock+0x2b/0x38
> [<c0265486>] dev_queue_xmit+0x247/0x2f1
> [<c02334f6>] __pppoe_xmit+0x1a9/0x215
> [<c023356c>] pppoe_xmit+0xa/0xc
> [<c0230c9a>] ppp_channel_push+0x41/0x9a
> [<c0231a13>] ppp_write+0xc6/0xf2
> [<c015ec26>] vfs_write+0x7f/0xba
> [<c015f158>] sys_write+0x3d/0x64
> [<c01027de>] sysenter_past_esp+0x5f/0x99
> =======================
> Clocksource tsc unstable (delta = 4686844667 ns)
> Time: acpi_pm clocksource has been installed.
...

lockdep has seen locks "-> #0" - "-> #3" taken in circular order, but IMHO,
lock "-> #3" (&pch->downl) taken after "-> #2" (&ppp->wlock) differs from
&pch->downl lock taken in "-> #0" (before &vlan_netdev_xmit_lock_key) and
lockdep should be notified about this.

Reported & tested by: "Yuriy N. Shkandybin" <jura@netams.com>
Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/net/ppp_generic.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff -puN drivers/net/ppp_generic.c~ppp_generic-fix-lockdep-warning drivers/net/ppp_generic.c
--- a/drivers/net/ppp_generic.c~ppp_generic-fix-lockdep-warning
+++ a/drivers/net/ppp_generic.c
@@ -1435,7 +1435,8 @@ ppp_channel_push(struct channel *pch)
 	struct sk_buff *skb;
 	struct ppp *ppp;
 
-	spin_lock_bh(&pch->downl);
+	local_bh_disable();
+	spin_lock_nested(&pch->downl, SINGLE_DEPTH_NESTING);
 	if (pch->chan != 0) {
 		while (!skb_queue_empty(&pch->file.xq)) {
 			skb = skb_dequeue(&pch->file.xq);
@@ -1449,7 +1450,8 @@ ppp_channel_push(struct channel *pch)
 		/* channel got deregistered */
 		skb_queue_purge(&pch->file.xq);
 	}
-	spin_unlock_bh(&pch->downl);
+	spin_unlock(&pch->downl);
+	local_bh_enable();
 	/* see if there is anything from the attached unit to be sent */
 	if (skb_queue_empty(&pch->file.xq)) {
 		read_lock_bh(&pch->upl);
_

Patches currently in -mm which might be from jarkao2@o2.pl are

ppp_generic-fix-lockdep-warning.patch

Re: + ppp_generic-fix-lockdep-warning.patch added to -mm tree

[Andrew Morton]

(added netdev)

On Wed, 11 Apr 2007 09:57:33 +0400 "Yuriy N. Shkandybin" <jura@netams.com> wrote:

> I've tested  2.6.21-rc6-mm1
> Linux vpn1 2.6.21-rc6-mm1 #4 SMP Wed Apr 11 03:34:26 MSD 2007 x86_64 
> Intel(R) Pentium(R) D CPU 2.80GHz GenuineIntel GNU/Linux
> 
> warn appeares upon first pppoe connection to rp-pppoe server in kernel mode
> 
> result:
> =======================================================
> [ INFO: possible circular locking dependency detected ]
> 2.6.21-rc6-mm1 #4
> -------------------------------------------------------
> pppd/14305 is trying to acquire lock:
>  (&vlan_netdev_xmit_lock_key){-...}, at: [<ffffffff8022f90b>] 
> dev_queue_xmit+0x26b/0x300
> 
> but task is already holding lock:
>  (&pch->downl#2){-+..}, at: [<ffffffff80388d3c>] ppp_push+0x5f/0xa7
> 
> which lock already depends on the new lock.
> 
> 
> the existing dependency chain (in reverse order) is:
> 
> -> #3 (&pch->downl#2){-+..}:
>        [<ffffffff80290c5f>] __lock_acquire+0xedf/0x1048
>        [<ffffffff80290e17>] lock_acquire+0x4f/0x78
>        [<ffffffff80388d3c>] ppp_push+0x5f/0xa7
>        [<ffffffff80263434>] _spin_lock_bh+0x2a/0x39
>        [<ffffffff80388d3c>] ppp_push+0x5f/0xa7
>        [<ffffffff8038967d>] ppp_xmit_process+0x3d/0x590
>        [<ffffffff8038b915>] ppp_write+0x105/0x140
>        [<ffffffff802163f3>] vfs_write+0xa3/0xf0
>        [<ffffffff80216e04>] sys_write+0x47/0x75
>        [<ffffffff8025d11e>] system_call+0x7e/0x83
>        [<ffffffffffffffff>] 0xffffffffffffffff
> 
> -> #2 (&ppp->wlock){-+..}:
>        [<ffffffff80290c5f>] __lock_acquire+0xedf/0x1048
>        [<ffffffff80290e17>] lock_acquire+0x4f/0x78
>        [<ffffffff80389667>] ppp_xmit_process+0x27/0x590
>        [<ffffffff80263434>] _spin_lock_bh+0x2a/0x39
>        [<ffffffff80389667>] ppp_xmit_process+0x27/0x590
>        [<ffffffff8038b78c>] ppp_start_xmit+0x1cc/0x250
>        [<ffffffff803c1bff>] dev_hard_start_xmit+0x22f/0x290
>        [<ffffffff803ccbf1>] __qdisc_run+0xd1/0x1f8
>        [<ffffffff8022f928>] dev_queue_xmit+0x288/0x300
>        [<ffffffff80239b72>] ip_mc_output+0x292/0x3f0
>        [<ffffffff803eb991>] raw_sendmsg+0x511/0x7c3
>        [<ffffffff80245a25>] inet_sendmsg+0x35/0x55
>        [<ffffffff80254da7>] sock_sendmsg+0xdf/0x102
>        [<ffffffff8028f826>] trace_hardirqs_on+0xc6/0x160
>        [<ffffffff802885da>] autoremove_wake_function+0x0/0x46
>        [<ffffffff80263324>] _spin_unlock_bh+0x2f/0x36
>        [<ffffffff80230ec8>] release_sock+0xcd/0xd6
>        [<ffffffff803dbcc2>] ip_setsockopt+0x142/0xbb3
>        [<ffffffff803bf33c>] verify_iovec+0x3c/0xc2
>        [<ffffffff803b8e9d>] sys_sendmsg+0x133/0x248
>        [<ffffffff8028f826>] trace_hardirqs_on+0xc6/0x160
>        [<ffffffff80282028>] getrusage+0x1b8/0x1d9
>        [<ffffffff8028f826>] trace_hardirqs_on+0xc6/0x160
>        [<ffffffff80262e27>] trace_hardirqs_on_thunk+0x35/0x37
>        [<ffffffff8025d11e>] system_call+0x7e/0x83
>        [<ffffffffffffffff>] 0xffffffffffffffff
> 
> -> #1 (&dev->_xmit_lock){-+..}:
>        [<ffffffff80290c5f>] __lock_acquire+0xedf/0x1048
>        [<ffffffff80290e17>] lock_acquire+0x4f/0x78
>        [<ffffffff803c4f59>] dev_mc_add+0x40/0x169
>        [<ffffffff80263434>] _spin_lock_bh+0x2a/0x39
>        [<ffffffff803c4f59>] dev_mc_add+0x40/0x169
>        [<ffffffff80403457>] vlan_dev_set_multicast_list+0xa7/0x2b8
>        [<ffffffff803c4c64>] __dev_mc_upload+0x24/0x26
>        [<ffffffff803c4ff7>] dev_mc_add+0xde/0x169
>        [<ffffffff803f47d7>] igmp_group_added+0x56/0x5f
>        [<ffffffff8026322b>] _write_unlock_bh+0x2f/0x36
>        [<ffffffff803f4965>] ip_mc_inc_group+0x105/0x17a
>        [<ffffffff803f49fc>] ip_mc_up+0x22/0x69
>        [<ffffffff803f1b48>] inetdev_event+0x1b8/0x2f0
>        [<ffffffff80281d69>] notifier_call_chain+0x49/0x6b
>        [<ffffffff80281dcc>] __raw_notifier_call_chain+0x9/0xb
>        [<ffffffff80281ddf>] raw_notifier_call_chain+0x11/0x13
>        [<ffffffff803c335d>] dev_open+0x7d/0x80
>        [<ffffffff803c1527>] dev_change_flags+0x107/0x138
>        [<ffffffff803f290c>] devinet_ioctl+0x5cc/0x720
>        [<ffffffff803c2fec>] dev_ioctl+0x1fc/0x31b
>        [<ffffffff8022104f>] __up_read+0x3f/0x9d
>        [<ffffffff803f2d5d>] inet_ioctl+0x5d/0x77
>        [<ffffffff803b854f>] sock_ioctl+0x4f/0x215
>        [<ffffffff80241aca>] do_ioctl+0x2a/0x83
>        [<ffffffff8022fe92>] vfs_ioctl+0x62/0x2b0
>        [<ffffffff8028f826>] trace_hardirqs_on+0xc6/0x160
>        [<ffffffff8024cef0>] sys_ioctl+0x41/0x65
>        [<ffffffff8025d11e>] system_call+0x7e/0x83
>        [<ffffffffffffffff>] 0xffffffffffffffff
> 
> -> #0 (&vlan_netdev_xmit_lock_key){-...}:
>        [<ffffffff8028ded0>] print_circular_bug_entry+0x49/0x59
>        [<ffffffff80290ad3>] __lock_acquire+0xd53/0x1048
>        [<ffffffff8020a5d5>] kmem_cache_alloc+0x1a5/0x5e0
>        [<ffffffff8028f7ea>] trace_hardirqs_on+0x8a/0x160
>        [<ffffffff80290e17>] lock_acquire+0x4f/0x78
>        [<ffffffff8022f90b>] dev_queue_xmit+0x26b/0x300
>        [<ffffffff802633fb>] _spin_lock+0x25/0x34
>        [<ffffffff8022f90b>] dev_queue_xmit+0x26b/0x300
>        [<ffffffff8038d753>] __pppoe_xmit+0x1e8/0x265
>        [<ffffffff8038d7dc>] pppoe_xmit+0xc/0xe
>        [<ffffffff80388d51>] ppp_push+0x74/0xa7
>        [<ffffffff8038967d>] ppp_xmit_process+0x3d/0x590
>        [<ffffffff8038b915>] ppp_write+0x105/0x140
>        [<ffffffff802163f3>] vfs_write+0xa3/0xf0
>        [<ffffffff80216e04>] sys_write+0x47/0x75
>        [<ffffffff8025d11e>] system_call+0x7e/0x83
>        [<ffffffffffffffff>] 0xffffffffffffffff
> 
> other info that might help us debug this:
> 
> 2 locks held by pppd/14305:
>  #0:  (&ppp->wlock){-+..}, at: [<ffffffff80389667>] 
> ppp_xmit_process+0x27/0x590
>  #1:  (&pch->downl#2){-+..}, at: [<ffffffff80388d3c>] ppp_push+0x5f/0xa7
> 
> stack backtrace:
> 
> Call Trace:
>  [<ffffffff8028e95b>] print_circular_bug_tail+0x7c/0x91
>  [<ffffffff8028ded0>] print_circular_bug_entry+0x49/0x59
>  [<ffffffff80290ad3>] __lock_acquire+0xd53/0x1048
>  [<ffffffff8020a5d5>] kmem_cache_alloc+0x1a5/0x5e0
>  [<ffffffff8028f7ea>] trace_hardirqs_on+0x8a/0x160
>  [<ffffffff80290e17>] lock_acquire+0x4f/0x78
>  [<ffffffff8022f90b>] dev_queue_xmit+0x26b/0x300
>  [<ffffffff802633fb>] _spin_lock+0x25/0x34
>  [<ffffffff8022f90b>] dev_queue_xmit+0x26b/0x300
>  [<ffffffff8038d753>] __pppoe_xmit+0x1e8/0x265
>  [<ffffffff8038d7dc>] pppoe_xmit+0xc/0xe
>  [<ffffffff80388d51>] ppp_push+0x74/0xa7
>  [<ffffffff8038967d>] ppp_xmit_process+0x3d/0x590
>  [<ffffffff8038b915>] ppp_write+0x105/0x140
>  [<ffffffff802163f3>] vfs_write+0xa3/0xf0
>  [<ffffffff80216e04>] sys_write+0x47/0x75
>  [<ffffffff8025d11e>] system_call+0x7e/0x83
> 
> INFO: lockdep is turned off.

Thanks.  So you're saying that
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc6/2.6.21-rc6-mm1/broken-out/ppp_generic-fix-lockdep-warning.patch
did not fix anything?

Re: + ppp_generic-fix-lockdep-warning.patch added to -mm tree

[Yuriy N. Shkandybin]

> (added netdev)
>
> On Wed, 11 Apr 2007 09:57:33 +0400 "Yuriy N. Shkandybin" <jura@netams.com> 
> wrote:
>
>> I've tested  2.6.21-rc6-mm1
>> Linux vpn1 2.6.21-rc6-mm1 #4 SMP Wed Apr 11 03:34:26 MSD 2007 x86_64
>> Intel(R) Pentium(R) D CPU 2.80GHz GenuineIntel GNU/Linux
>>
>> warn appeares upon first pppoe connection to rp-pppoe server in kernel 
>> mode
>>
>
> Thanks.  So you're saying that
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc6/2.6.21-rc6-mm1/broken-out/ppp_generic-fix-lockdep-warning.patch
> did not fix anything?
As i understand this patch already in -mm tree, so I've booted into last mm 
kernel and received this locked warning.
Or i've mistaked and should apply this patch manually?
>
> 

Re: + ppp_generic-fix-lockdep-warning.patch added to -mm tree

[Andrew Morton]

On Wed, 11 Apr 2007 12:52:28 +0400 "Yuriy N. Shkandybin" <jura@netams.com> wrote:

> 
> 
> > (added netdev)
> >
> > On Wed, 11 Apr 2007 09:57:33 +0400 "Yuriy N. Shkandybin" <jura@netams.com> 
> > wrote:
> >
> >> I've tested  2.6.21-rc6-mm1
> >> Linux vpn1 2.6.21-rc6-mm1 #4 SMP Wed Apr 11 03:34:26 MSD 2007 x86_64
> >> Intel(R) Pentium(R) D CPU 2.80GHz GenuineIntel GNU/Linux
> >>
> >> warn appeares upon first pppoe connection to rp-pppoe server in kernel 
> >> mode
> >>
> >
> > Thanks.  So you're saying that
> > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc6/2.6.21-rc6-mm1/broken-out/ppp_generic-fix-lockdep-warning.patch
> > did not fix anything?
> As i understand this patch already in -mm tree, so I've booted into last mm 
> kernel and received this locked warning.
> Or i've mistaked and should apply this patch manually?

No, that's OK - that patch is indeed in 2.6.21-rc6-mm1.  It appears that it
did not fix this bug.

Re: + ppp_generic-fix-lockdep-warning.patch added to -mm tree

[Jarek Poplawski]

On Wed, Apr 11, 2007 at 12:52:28PM +0400, Yuriy N. Shkandybin wrote:
...
> >On Wed, 11 Apr 2007 09:57:33 +0400 "Yuriy N. Shkandybin" <jura@netams.com> 
> >wrote:
> >
> >>I've tested  2.6.21-rc6-mm1
> >>Linux vpn1 2.6.21-rc6-mm1 #4 SMP Wed Apr 11 03:34:26 MSD 2007 x86_64
> >>Intel(R) Pentium(R) D CPU 2.80GHz GenuineIntel GNU/Linux
> >>
> >>warn appeares upon first pppoe connection to rp-pppoe server in kernel 
> >>mode
> >>
> >
> >Thanks.  So you're saying that
> >ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc6/2.6.21-rc6-mm1/broken-out/ppp_generic-fix-lockdep-warning.patch
> >did not fix anything?
> As i understand this patch already in -mm tree, so I've booted into last mm 
> kernel and received this locked warning.
> Or i've mistaked and should apply this patch manually?

Hi!

Yuriy - thanks for testing my patch ...(pause) Not!

It seems this patch is not visible in this version - probably
is overpatched by something else. But your new log shows there
is another connection between these locks (ppp_xmit_process
and ppp_push instead of ppp_channel_push in "-> #0"), so the
patch is not sufficient (and could be dumped).

I don't know your vlans configuration, but it seems the real
lockup isn't very probable here - it's rather lockdep question.
I think vlan's too broad lockdep class is the main "guilty"
here, but probably pppoe also could be enhanced: it's making
the things unnecessarily complicated by calling dev_queue_xmit
under ppp_generic's xmit locks. I wonder if there is any reason
against using a tasklet here.

I'll try to find more time to untie this yet - or maybe some
maintainer will find this interesting, too...

Regards,
Jarek P.

PS: sorry for late responding (vacations).

Re: + ppp_generic-fix-lockdep-warning.patch added to -mm tree

[Michal Ostrowski]

The "xmit" function of a PPP channel is a synchronous operation.  If the 
transmission fails, we must notify the caller and let them re-submit the 
skb later.  The return status of dev_queue_xmit is needed to determine 
the return code passed back to the caller and thus the call is made 
synchronously and not in a tasklet.

Looking at the stack traces earlier in this thread, it seems to me that 
even if the PPPoE call was made in a tasklet, this same warning could be 
generated.

--
Michal Ostrowski
mostrows@earthlink.net



Jarek Poplawski wrote:
> On Wed, Apr 11, 2007 at 12:52:28PM +0400, Yuriy N. Shkandybin wrote:
> ...
>>> On Wed, 11 Apr 2007 09:57:33 +0400 "Yuriy N. Shkandybin" <jura@netams.com> 
>>> wrote:
>>>
>>>> I've tested  2.6.21-rc6-mm1
>>>> Linux vpn1 2.6.21-rc6-mm1 #4 SMP Wed Apr 11 03:34:26 MSD 2007 x86_64
>>>> Intel(R) Pentium(R) D CPU 2.80GHz GenuineIntel GNU/Linux
>>>>
>>>> warn appeares upon first pppoe connection to rp-pppoe server in kernel 
>>>> mode
>>>>
>>> Thanks.  So you're saying that
>>> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc6/2.6.21-rc6-mm1/broken-out/ppp_generic-fix-lockdep-warning.patch
>>> did not fix anything?
>> As i understand this patch already in -mm tree, so I've booted into last mm 
>> kernel and received this locked warning.
>> Or i've mistaked and should apply this patch manually?
> 
> Hi!
> 
> Yuriy - thanks for testing my patch ...(pause) Not!
> 
> It seems this patch is not visible in this version - probably
> is overpatched by something else. But your new log shows there
> is another connection between these locks (ppp_xmit_process
> and ppp_push instead of ppp_channel_push in "-> #0"), so the
> patch is not sufficient (and could be dumped).
> 
> I don't know your vlans configuration, but it seems the real
> lockup isn't very probable here - it's rather lockdep question.
> I think vlan's too broad lockdep class is the main "guilty"
> here, but probably pppoe also could be enhanced: it's making
> the things unnecessarily complicated by calling dev_queue_xmit
> under ppp_generic's xmit locks. I wonder if there is any reason
> against using a tasklet here.
> 
> I'll try to find more time to untie this yet - or maybe some
> maintainer will find this interesting, too...
> 
> Regards,
> Jarek P.
> 
> PS: sorry for late responding (vacations).
> 
> 

Re: + ppp_generic-fix-lockdep-warning.patch added to -mm tree

[Jarek Poplawski]

On Tue, Apr 17, 2007 at 08:26:32AM -0500, Michal Ostrowski wrote:
> The "xmit" function of a PPP channel is a synchronous operation.  If the 
> transmission fails, we must notify the caller and let them re-submit the 
> skb later.  The return status of dev_queue_xmit is needed to determine 
> the return code passed back to the caller and thus the call is made 
> synchronously and not in a tasklet.

Sure! But on the other hand:

- the return code from dev_queue_xmit doesn't guarantee
the transmission won't fail,

- similar code in ppp_async: ppp_async_send isn't so
truthful and doesn't even check the return from
ppp_async_push; BTW - probably other layers should
care for transmission errors and re-submiting,

- maybe I'm wrong here, but I think every "layer" should
look (work) similarly here: dev_queue_xmit (or qdisc_run)
thinks it's talking to some independent network device,
which after dev_hard_start_xmit (and dev->hard_start_xmit)
does some transmission; if, instead of this, next
dev_queue_xmits are called with xmit locks held from
previous "devs", then it looks like logical recursion and
locking is really hard to follow (even if it's OK).

> Looking at the stack traces earlier in this thread, it seems to me that 
> even if the PPPoE call was made in a tasklet, this same warning could be 
> generated.

Of course a tasklet by itself isn't a cure, but if
dev_queue_xmit is done from tasklet - only locks got
within this tasklet should be counted.

Thanks for response & best regards,
Jarek P.

Re: + ppp_generic-fix-lockdep-warning.patch added to -mm tree

[Jarek Poplawski]

On Tue, Apr 17, 2007 at 09:37:44AM +0200, Jarek Poplawski wrote:
...
> Yuriy - thanks for testing my patch ...(pause) Not!
> 
> It seems this patch is not visible in this version - probably
...

Sorry! It was only something with my eyes.
(Probably too much of Pamela!).

Jarek P.