From mboxrd@z Thu Jan 1 00:00:00 1970 From: "David Woodhouse" Subject: Re: [PATCH] atm: Preserve value of skb->truesize when accounting to vcc Date: Sat, 16 Jun 2018 15:19:48 -0000 Message-ID: <10912b7594637a95567a2ffe3ef5dbc1.squirrel@twosheds.infradead.org> References: <20180616105544.246714-1-dwmw2@infradead.org> <62fbc51a-4738-6c21-8c53-d84fee7a9bbd@gmail.com> Mime-Version: 1.0 Content-Type: text/plain;charset=utf-8 Content-Transfer-Encoding: 8bit Cc: "David Woodhouse" , netdev@vger.kernel.org, ldir@darbyshire-bryant.me.uk To: "Eric Dumazet" Return-path: Received: from twosheds.infradead.org ([90.155.92.209]:44088 "EHLO twosheds.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754135AbeFPPTv (ORCPT ); Sat, 16 Jun 2018 11:19:51 -0400 In-Reply-To: <62fbc51a-4738-6c21-8c53-d84fee7a9bbd@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: >> Commit 14afee4b609 ("net: convert sock.sk_wmem_alloc from atomic_t to >> refcount_t") did exactly what it was intended to do, and turned this >> mostly-theoretical problem into a real one, causing PPPoATM to fail >> immediately as sk_wmem_alloc underflows and atm_may_send() *immediately* >> starts refusing to allow new packets. ... >> Fixes: 14afee4b ("net: convert sock.sk_wmem_alloc from atomic_t to >> refcount_t") > > This Fixes tag shoots the messenger really. A little bit, yes. The text hopefully made that clear. > I suggest to instead use : > > Fixes: 158f323b9868 ("net: adjust skb->truesize in pskb_expand_head()") > > Because even without the conversion to refcount_t, we could have a LOCKDEP > splat in : > > filter = rcu_dereference_check(sk->sk_filter, > atomic_read(&sk->sk_wmem_alloc) == 0); > > Note that some places make a further check even when LOCKDEP is not used. > > net/ipv4/af_inet.c:154: WARN_ON(refcount_read(&sk->sk_wmem_alloc)); > net/iucv/af_iucv.c:405: WARN_ON(refcount_read(&sk->sk_wmem_alloc)); > net/key/af_key.c:112: WARN_ON(refcount_read(&sk->sk_wmem_alloc)); > net/netlink/af_netlink.c:410: WARN_ON(refcount_read(&sk->sk_wmem_alloc)); > net/packet/af_packet.c:1286: WARN_ON(refcount_read(&sk->sk_wmem_alloc)); > net/rxrpc/af_rxrpc.c:852: WARN_ON(refcount_read(&sk->sk_wmem_alloc)); > net/unix/af_unix.c:490: WARN_ON(refcount_read(&sk->sk_wmem_alloc)); > > > We might factorize these checks into __sk_destruct() > How many of those were likely to trigger in practice on an ATM VCC though? If we are taking the Fixes: tag as a hint about which stable kernels we might want to backport to, rather than a moral assignment of blame, then 14afee4b is probably not the worst place to point it. But I don't mind much... -- dwmw2