On Wed, Feb 20, 2008 at 04:02:52PM +0000, 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 repeatedly creating/deleting the L2TP (PPP) sessions) I get a soft
> lockup detected in pppol2tp_xmit() after anything between 1 minute and
> an hour. :( I'm investigating that now.
>
> Thanks for your help!
Not at all!
>
>> (testing patch #1)
But I hope you tested with the fixed (take 2) version of this patch...
Since it's quite experimental (testing) this patch could be wrong
as it is, but I hope it should show the proper way to solve this
problem. Probably you did some of these, but here are a few of my
suggestions for testing this:
1) try my patch with your full bh locking changing patch;
2) add while loops to these trylocks on failure, with e.g. __delay(1);
this should work like full locks again, but there should be no (this
kind of) lockdep reports;
3) I send here another testing patch with this second way to do this:
on the write side, but it's even more "experimental" and only a
proof of concept (should be applied on vanilla ppp_generic).
Regards,
Jarek P.
(testing patch #2)
---
drivers/net/ppp_generic.c | 20 +++++++++++++-------
1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c
index 4dc5b4b..70bd255 100644
--- a/drivers/net/ppp_generic.c
+++ b/drivers/net/ppp_generic.c
@@ -2606,11 +2606,16 @@ ppp_connect_channel(struct channel *pch, int unit)
ppp = ppp_find_unit(unit);
if (!ppp)
goto out;
- write_lock_bh(&pch->upl);
+
ret = -EINVAL;
- if (pch->ppp)
- goto outl;
+ read_lock_bh(&pch->upl);
+ if (pch->ppp) {
+ read_unlock_bh(&pch->upl);
+ goto out;
+ }
+ read_unlock_bh(&pch->upl);
+ atomic_inc(&ppp->file.refcnt);
ppp_lock(ppp);
if (pch->file.hdrlen > ppp->file.hdrlen)
ppp->file.hdrlen = pch->file.hdrlen;
@@ -2619,13 +2624,14 @@ ppp_connect_channel(struct channel *pch, int unit)
ppp->dev->hard_header_len = hdrlen;
list_add_tail(&pch->clist, &ppp->channels);
++ppp->n_channels;
- pch->ppp = ppp;
- atomic_inc(&ppp->file.refcnt);
ppp_unlock(ppp);
- ret = 0;
- outl:
+ /* avoid lock dependency with ppp_locks */
+ write_lock_bh(&pch->upl);
+ BUG_ON(pch->ppp);
+ pch->ppp = ppp;
write_unlock_bh(&pch->upl);
+ ret = 0;
out:
mutex_unlock(&all_ppp_mutex);
return ret;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
