Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-26 Thread James Chapman
Jarek Poplawski wrote: Jarek Poplawski wrote, On 02/25/2008 02:39 PM: ... Hmm... Wait a minute! But on the other hand David has written about his cons here, and it looks reasonable: this place would be fixed, but some others can start reports like this. Maybe, it's better to analyze yet if it's

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-26 Thread Jarek Poplawski
On Tue, Feb 26, 2008 at 12:14:26PM +, James Chapman wrote: Jarek Poplawski wrote: Jarek Poplawski wrote, On 02/25/2008 02:39 PM: ... Hmm... Wait a minute! But on the other hand David has written about his cons here, and it looks reasonable: this place would be fixed, but some others can

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-26 Thread Jarek Poplawski
On Tue, Feb 26, 2008 at 01:03:34PM +, Jarek Poplawski wrote: On Tue, Feb 26, 2008 at 12:14:26PM +, James Chapman wrote: ... there are hanging. Btw, they don't hang if I disable irqs around the ppp_input() call. ...and disabling bh instead isn't enough, BTW? I guess not: they are

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-26 Thread Jarek Poplawski
James Chapman wrote, On 02/26/2008 01:14 PM: ... Luckily, I'm in the lab where my two borrowed servers are today so I have access to their consoles. Hopefully I'll be able to find out why there are hanging. Btw, they don't hang if I disable irqs around the ppp_input() call. Maybe you've

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-25 Thread James Chapman
Jarek Poplawski wrote: Jarek Poplawski wrote, On 02/21/2008 01:08 PM: ... Another, probably simpler way would be to move almost all pppol2tp_xmit ... Actually, the simplest off all seems to be now this old idea to maybe make sk_dst_lock globally softirq immune. At least I think it's worth of

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-25 Thread Jarek Poplawski
On Mon, Feb 25, 2008 at 12:19:50PM +, James Chapman wrote: Jarek Poplawski wrote: Jarek Poplawski wrote, On 02/21/2008 01:08 PM: ... Another, probably simpler way would be to move almost all pppol2tp_xmit ... Actually, the simplest off all seems to be now this old idea to maybe make

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-25 Thread Jarek Poplawski
On Mon, Feb 25, 2008 at 01:05:08PM +, Jarek Poplawski wrote: ... On Mon, Feb 25, 2008 at 12:19:50PM +, James Chapman wrote: Is this an acceptable solution? If so, I'll prepare and send official patches. IMHO this should be acceptable because I can't see any reason for changing

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-25 Thread Jarek Poplawski
On Mon, Feb 25, 2008 at 01:39:48PM +, Jarek Poplawski wrote: ... Maybe, it's better to analyze yet if it's really so hard to eliminate taking this lock on the xmit path? BTW, I'm not sure if it helps, but this matters only for the sockets which could be used (and locked) outside of

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-25 Thread Jarek Poplawski
Jarek Poplawski wrote, On 02/25/2008 02:39 PM: ... Hmm... Wait a minute! But on the other hand David has written about his cons here, and it looks reasonable: this place would be fixed, but some others can start reports like this. Maybe, it's better to analyze yet if it's really so hard to

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-21 Thread Jarek Poplawski
On Wed, Feb 20, 2008 at 10:37:57PM +, James Chapman wrote: Jarek Poplawski wrote: (testing patch #1) But I hope you tested with the fixed (take 2) version of this patch... Yes I did. :) But I just got another lockdep error (attached). Since it's quite experimental (testing) this

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-21 Thread James Chapman
Jarek Poplawski wrote: On Wed, Feb 20, 2008 at 10:37:57PM +, James Chapman wrote: Jarek Poplawski wrote: (testing patch #1) But I hope you tested with the fixed (take 2) version of this patch... Yes I did. :) But I just got another lockdep error (attached). Since it's quite

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-21 Thread Jarek Poplawski
On Thu, Feb 21, 2008 at 09:53:56AM +, James Chapman wrote: Jarek Poplawski wrote: ... The _bh locking fixes in pppol2tp combined with your ppp_generic change solved that problem. So I then added data traffic into the mix (since this will happen in a real network) and found that lockups

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-21 Thread Jarek Poplawski
Jarek Poplawski wrote, On 02/21/2008 01:08 PM: ... Another, probably simpler way would be to move almost all pppol2tp_xmit ... Actually, the simplest off all seems to be now this old idea to maybe make sk_dst_lock globally softirq immune. At least I think it's worth of testing, to check for

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-20 Thread James Chapman
Jarek Poplawski wrote: On Mon, Feb 18, 2008 at 10:09:24PM +, James Chapman wrote: ... Unfortunately the ISP's syslog stops. But I've been able to borrow two Quad Xeon boxes and have reproduced the problem. Here's a new version of the patch. The patch avoids disabling irqs and fixes the

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-20 Thread Jarek Poplawski
On Wed, Feb 20, 2008 at 04:02:52PM +, James Chapman wrote: ... I tried your ppp_generic patch with only the hlist_lock bh patch in pppol2tp and it seems to fix the ppp create/delete issue. However, when I added much more traffic into the test (flood pings over ppp interfaces while

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-20 Thread James Chapman
Jarek Poplawski wrote: (testing patch #1) But I hope you tested with the fixed (take 2) version of this patch... Yes I did. :) But I just got another lockdep error (attached). Since it's quite experimental (testing) this patch could be wrong as it is, but I hope it should show the proper

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-19 Thread James Chapman
David Miller wrote: From: James Chapman [EMAIL PROTECTED] Date: Mon, 18 Feb 2008 22:09:24 + Here's a new version of the patch. The patch avoids disabling irqs and fixes the sk_dst_get() usage that DaveM mentioned. But even with this patch, lockdep still complains if hundreds of ppp

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-19 Thread James Chapman
Jarek Poplawski wrote: On Mon, Feb 18, 2008 at 10:09:24PM +, James Chapman wrote: Jarek Poplawski wrote: Hi, It seems, this nice report is still uncomplete: could you check if there could have been something more yet? Unfortunately the ISP's syslog stops. But I've been able to borrow two

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-19 Thread Jarek Poplawski
On Tue, Feb 19, 2008 at 10:30:47AM +, Jarek Poplawski wrote: ... IMHO, just like I wrote earlier, the main problem is in ppp_generic(), ...or maybe ppp_generic.c? Whatever... Jarek P. -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-19 Thread Jarek Poplawski
On Tue, Feb 19, 2008 at 09:03:12AM +, James Chapman wrote: David Miller wrote: From: James Chapman [EMAIL PROTECTED] Date: Mon, 18 Feb 2008 22:09:24 + Here's a new version of the patch. The patch avoids disabling irqs and fixes the sk_dst_get() usage that DaveM mentioned. But even

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-19 Thread Jarek Poplawski
On Mon, Feb 18, 2008 at 10:09:24PM +, James Chapman wrote: ... Unfortunately the ISP's syslog stops. But I've been able to borrow two Quad Xeon boxes and have reproduced the problem. Here's a new version of the patch. The patch avoids disabling irqs and fixes the sk_dst_get() usage that

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-19 Thread Jarek Poplawski
On Wed, Feb 20, 2008 at 12:06:40AM +0100, Jarek Poplawski wrote: ... (testing patch #1) SORRY!!! take 2 (unlocking fixed) --- drivers/net/ppp_generic.c | 39 +-- 1 files changed, 29 insertions(+), 10 deletions(-) diff --git

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-18 Thread Jarek Poplawski
On Mon, Feb 18, 2008 at 10:09:24PM +, James Chapman wrote: Jarek Poplawski wrote: Hi, It seems, this nice report is still uncomplete: could you check if there could have been something more yet? Unfortunately the ISP's syslog stops. But I've been able to borrow two Quad Xeon boxes and

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-18 Thread David Miller
From: James Chapman [EMAIL PROTECTED] Date: Mon, 18 Feb 2008 22:09:24 + Here's a new version of the patch. The patch avoids disabling irqs and fixes the sk_dst_get() usage that DaveM mentioned. But even with this patch, lockdep still complains if hundreds of ppp sessions are inserted into

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-14 Thread Jarek Poplawski
Hi, It seems, this nice report is still uncomplete: could you check if there could have been something more yet? Thanks, Jarek P. On Tue, Feb 12, 2008 at 10:58:21AM +, James Chapman wrote: ... Here is a trace from when we had _bh locks. Feb 5 16:26:32

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-12 Thread David Miller
From: James Chapman [EMAIL PROTECTED] Date: Tue, 12 Feb 2008 10:58:21 + Here is a trace from when we had _bh locks. The problem is that the pppol2tp code calls sk_dst_get() in software interrupt context and that is not allowed. sk_dst_get() grabs sk-sk_dst_lock without any BH enabling or

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-12 Thread Jarek Poplawski
On Tue, Feb 12, 2008 at 10:58:21AM +, James Chapman wrote: ... Here is a trace from when we had _bh locks. Very nice... ...But since it's quite long, and if you don't know all these paths this could take some time, maybe one question: so if lockdep got these locks right (sometimes it can be

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-12 Thread James Chapman
David Miller wrote: From: James Chapman [EMAIL PROTECTED] Date: Mon, 11 Feb 2008 23:41:18 + Jarek Poplawski wrote: On Mon, Feb 11, 2008 at 10:19:35PM +, James Chapman wrote: ... Below is example output from lockdep. The oops is reproducible when creating/deleting lots of sessions

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-12 Thread Jarek Poplawski
On Mon, Feb 11, 2008 at 11:42:12PM +, James Chapman wrote: Jarek Poplawski wrote: On Mon, Feb 11, 2008 at 11:49:24PM +0100, Jarek Poplawski wrote: On Mon, Feb 11, 2008 at 10:19:35PM +, James Chapman wrote: ... Below is example output from lockdep. The oops is reproducible when

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-12 Thread Jarek Poplawski
On Tue, Feb 12, 2008 at 10:00:03PM -0800, David Miller wrote: From: James Chapman [EMAIL PROTECTED] Date: Tue, 12 Feb 2008 10:58:21 + Here is a trace from when we had _bh locks. The problem is that the pppol2tp code calls sk_dst_get() in software interrupt context and that is not

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-11 Thread Jarek Poplawski
On Mon, Feb 11, 2008 at 11:49:24PM +0100, Jarek Poplawski wrote: On Mon, Feb 11, 2008 at 10:19:35PM +, James Chapman wrote: ... Below is example output from lockdep. The oops is reproducible when creating/deleting lots of sessions while passing data. The lock is being acquired for

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-11 Thread James Chapman
Jarek Poplawski wrote: James Chapman wrote, On 02/11/2008 10:22 AM: Fix locking issues in the pppol2tp driver which can cause a kernel crash on SMP boxes when hundreds of L2TP sessions are created/deleted simultaneously (ISP environment). The driver was violating read_lock() and write_lock()

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-11 Thread Jarek Poplawski
On Mon, Feb 11, 2008 at 10:19:35PM +, James Chapman wrote: ... Below is example output from lockdep. The oops is reproducible when creating/deleting lots of sessions while passing data. The lock is being acquired for read and write in softirq contexts. Is there a better way to fix

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-11 Thread David Miller
From: James Chapman [EMAIL PROTECTED] Date: Mon, 11 Feb 2008 23:41:18 + Jarek Poplawski wrote: On Mon, Feb 11, 2008 at 10:19:35PM +, James Chapman wrote: ... Below is example output from lockdep. The oops is reproducible when creating/deleting lots of sessions while passing

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-11 Thread Jarek Poplawski
James Chapman wrote, On 02/11/2008 10:22 AM: Fix locking issues in the pppol2tp driver which can cause a kernel crash on SMP boxes when hundreds of L2TP sessions are created/deleted simultaneously (ISP environment). The driver was violating read_lock() and write_lock() scheduling rules so we

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-11 Thread Jarek Poplawski
On Mon, Feb 11, 2008 at 11:41:18PM +, James Chapman wrote: Jarek Poplawski wrote: On Mon, Feb 11, 2008 at 10:19:35PM +, James Chapman wrote: ... Below is example output from lockdep. The oops is reproducible when creating/deleting lots of sessions while passing data. The lock is

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-11 Thread James Chapman
Jarek Poplawski wrote: On Mon, Feb 11, 2008 at 11:49:24PM +0100, Jarek Poplawski wrote: On Mon, Feb 11, 2008 at 10:19:35PM +, James Chapman wrote: ... Below is example output from lockdep. The oops is reproducible when creating/deleting lots of sessions while passing data. The lock is

Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

2008-02-11 Thread James Chapman
Jarek Poplawski wrote: On Mon, Feb 11, 2008 at 10:19:35PM +, James Chapman wrote: ... Below is example output from lockdep. The oops is reproducible when creating/deleting lots of sessions while passing data. The lock is being acquired for read and write in softirq contexts. Is there a