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

[Jarek Poplawski <jarkao2@gmail.com>]

On Wed, Feb 20, 2008 at 10:37:57PM +0000, 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 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;
>
> Hmm, isn't this just bypassing the lockdep checks?

Yes! But it's only for debugging: to find if this change in locking
is to be blamed for these new lockups. It should effectively work just
like without this patch, but without this lockdep warning. So, if
after such change lockups still happen, then it would seem you didn't
test this enough before. Otherwise the new patch is to blame and needs
reworking.

>
>> 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).
>
> I'll look over it. I think I need to take a step back and look at what's  
> happening in more detail though.

This is something completely new and changes all the picture: the xmit
path wasn't expected (at least by me) to be called in softirq context
at all, and there were no traces of this on previous reports. But,
since lockdep always stops after the first warning, there could be
even more surprises like this in the future. I'll check this report.

Regards,
Jarek P.