On Tue, Oct 30, 2012 at 08:07:25PM +0100, Krzysztof Mazur wrote:
> On Tue, Oct 30, 2012 at 09:37:48AM +0000, David Woodhouse wrote:
> >
> > Should we be locking it earlier, so that the atm_may_send() call is also
> > covered by the lock?
>
> Yes, but only to protect against concurent vcc_sendmsg().
>
> >
> > Either way, it's an obvious improvement on what we had before ??? and even
> > if the answer to my question above is 'yes', exceeding the configured
> > size by one packet is both harmless and almost never going to happen
> > since we now limit ourselves to two packets anyway. So:
> >
> > Acked-By: David Woodhouse <[email protected]>
> >
>
David, I think we should also fix the issue with sk_sndbuf < MTU,
which is described in comment in pppoatm_may_send() added by
your "pppoatm: Fix excessive queue bloat" patch.
The vcc_sendmsg() already does that.
Krzysiek
-- >8 --
Subject: [PATCH] pppoatm: fix sending packets when sk_sndbuf < MTU
Now pppoatm_send() works, when sk_sndbuf is smaller than MTU. This
issue was already pointed in comment:
/*
* It's not clear that we need to bother with using atm_may_send()
* to check we don't exceed sk->sk_sndbuf. If userspace sets a
* value of sk_sndbuf which is lower than the MTU, we're going to
* block for ever. But the code always did that before we introduced
* the packet count limit, so...
*/
The test is copied from alloc_tx() which is used by vcc_sendmsg().
Signed-off-by: Krzysztof Mazur <[email protected]>
---
net/atm/pppoatm.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index 4cc81b5..f25536b 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -306,12 +306,9 @@ static int pppoatm_send(struct ppp_channel *chan, struct
sk_buff *skb)
/*
* It's not clear that we need to bother with using atm_may_send()
- * to check we don't exceed sk->sk_sndbuf. If userspace sets a
- * value of sk_sndbuf which is lower than the MTU, we're going to
- * block for ever. But the code always did that before we introduced
- * the packet count limit, so...
+ * to check we don't exceed sk->sk_sndbuf.
*/
- if (!atm_may_send(vcc, skb->truesize))
+ if (sk_wmem_alloc_get(sk_atm(vcc)) && !atm_may_send(vcc, skb->truesize))
goto nospace_unlock_sock;
atomic_add(skb->truesize, &sk_atm(ATM_SKB(skb)->vcc)->sk_wmem_alloc);
--
1.8.0.172.g62af90c
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
