On Fri, 2017-06-30 at 13:08 +0300, Elena Reshetova wrote:
> diff --git a/include/linux/atmdev.h b/include/linux/atmdev.h
> index c1da539..4d97a89 100644
> --- a/include/linux/atmdev.h
> +++ b/include/linux/atmdev.h
> @@ -254,7 +254,7 @@ static inline void atm_return(struct atm_vcc *vcc,int
> truesize)
>
> static inline int atm_may_send(struct atm_vcc *vcc,unsigned int size)
> {
> - return (size + atomic_read(&sk_atm(vcc)->sk_wmem_alloc)) <
> + return (size + refcount_read(&sk_atm(vcc)->sk_wmem_alloc)) <
> sk_atm(vcc)->sk_sndbuf;
> }Hm, this (commit 14afee4b6092fd) may have broken PPPoATM. I did spend a while staring hard at my own commit 9d02daf754238 which introduced pppoatm_may_send(), but it's actually atm_may_send() which is never allowing packets to be pushed. Debugging (which is ongoing) has so far shown that we are accounting for a packet in pppoatm_send() which has skb->truesize==0x1c0, and later end up in pppoatm_pop()→atm_raw_pop() with skb->truesize==0x2c0. This was always harmless before, but now it's a refcount_t it appears to underflow and go into its "screw you" mode and never let any more packets get sent. I'm staring hard at the special case in pskb_expand_head() to *not* change skb->truesize under certain circumstances, and wondering if that (is, should be) covering the case of ATM skbs: /* It is not generally safe to change skb->truesize. * For the moment, we really care of rx path, or * when skb is orphaned (not attached to a socket). */ if (!skb->sk || skb->destructor == sock_edemux) skb->truesize += size - osize; Failing that, maybe we should copy the accounted value of skb->truesize into the struct skb_atm_data in skb->cb at the time we add it to sk_wmem_alloc — and then subtract *that* value from sk_wmem_alloc in atm_raw_pop() instead of the then current value of skb->truesize. Suggestions?
smime.p7s
Description: S/MIME cryptographic signature
