From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Horman Subject: Re: [bpf-next PATCH 1/4] bpf: sockmap, free memory on sock close with cork data Date: Thu, 12 Apr 2018 15:13:23 +0200 Message-ID: <20180412131322.xv54avo456ubb5do@netronome.com> References: <20180401150054.24727.78359.stgit@john-Precision-Tower-5810> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org, davem@davemloft.net To: John Fastabend Return-path: Received: from mail-wm0-f68.google.com ([74.125.82.68]:36853 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752062AbeDLNNZ (ORCPT ); Thu, 12 Apr 2018 09:13:25 -0400 Received: by mail-wm0-f68.google.com with SMTP id x82so9508508wmg.1 for ; Thu, 12 Apr 2018 06:13:25 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20180401150054.24727.78359.stgit@john-Precision-Tower-5810> Sender: netdev-owner@vger.kernel.org List-ID: On Sun, Apr 01, 2018 at 08:00:54AM -0700, John Fastabend wrote: > If a socket with pending cork data is closed we do not return the > memory to the socket until the garbage collector free's the psock > structure. The garbage collector though can run after the sock has > completed its close operation. If this ordering happens the sock code > will through a WARN_ON because there is still outstanding memory s/through/throw/ ? > accounted to the sock. > > To resolve this ensure we return memory to the sock when a socket > is closed. > > Signed-off-by: John Fastabend > Fixes: 91843d540a13 ("bpf: sockmap, add msg_cork_bytes() helper") > --- > kernel/bpf/sockmap.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c > index d2bda5a..8ddf326 100644 > --- a/kernel/bpf/sockmap.c > +++ b/kernel/bpf/sockmap.c > @@ -211,6 +211,12 @@ static void bpf_tcp_close(struct sock *sk, long timeout) > close_fun = psock->save_close; > > write_lock_bh(&sk->sk_callback_lock); > + if (psock->cork) { > + free_start_sg(psock->sock, psock->cork); > + kfree(psock->cork); > + psock->cork = NULL; > + } > + > list_for_each_entry_safe(md, mtmp, &psock->ingress, list) { > list_del(&md->list); > free_start_sg(psock->sock, md); >