lkml.org 
[lkml]   [2012]   [Oct]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 2/3] pppoatm: fix race condition with destroying of vcc
From
Date
On Wed, 2012-10-31 at 12:30 +0100, Krzysztof Mazur wrote:
> Yes, original patch had also the same problem with sock_owned_by_user(),
> so I just incorrectly assumed that we can do "goto nospace" after
> pppoatm_may_send(), but ppooatm_may_send() must be the last test.
>
> So I just moved all other tests earlier and and now pppoatm_may_send()
> is also protected by ATM socket lock as you suggested earlier.

I don't think that's sufficient. When we return zero from
pppoatm_send(), the generic PPP code considers the channel to be
blocked, and it won't send any more data to it, ever, until we call
ppp_output_wakeup(). Which we do from a tasklet, triggered in
pppoatm_pop() *iff* the BLOCKED flag is set.

So we play silly buggers in pppoatm_may_send() to ensure that *if* we're
going to return zero, we make damn sure the BLOCKED flag is set and that
pppoatm_pop() is going to see that it's set. There are extensive
comments in pppoatm_pop() and pppoatm_may_send() which try to explain
this. It works because there's *always* going to be packet in flight if
we say that the sk_wmem is full, so of course there's *always* going to
be a later call to pppoatm_pop() to wake things up.

However, if you're going to return zero from pppoatm_send() when
sock_owned_by_user() is true, what guarantees that ppp_output_wakeup()
will ever be called?

--
dwmw2

[unhandled content-type:application/x-pkcs7-signature]
\
 
 \ /
  Last update: 2012-10-31 13:41    [W:0.107 / U:0.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site