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
smime.p7s
Description: S/MIME cryptographic signature
