From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EE189C3F2D1 for ; Thu, 5 Mar 2020 04:54:47 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A1323208C3 for ; Thu, 5 Mar 2020 04:54:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="tJ/6smOa" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A1323208C3 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 4BD576B0003; Wed, 4 Mar 2020 23:54:47 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 46EB56B0005; Wed, 4 Mar 2020 23:54:47 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 35D176B0007; Wed, 4 Mar 2020 23:54:47 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0238.hostedemail.com [216.40.44.238]) by kanga.kvack.org (Postfix) with ESMTP id 1DC3A6B0003 for ; Wed, 4 Mar 2020 23:54:47 -0500 (EST) Received: from smtpin22.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 9AD232DFD for ; Thu, 5 Mar 2020 04:54:46 +0000 (UTC) X-FDA: 76560093372.22.farm70_c913bd9fc035 X-HE-Tag: farm70_c913bd9fc035 X-Filterd-Recvd-Size: 7022 Received: from mail-ot1-f65.google.com (mail-ot1-f65.google.com [209.85.210.65]) by imf25.hostedemail.com (Postfix) with ESMTP for ; Thu, 5 Mar 2020 04:54:46 +0000 (UTC) Received: by mail-ot1-f65.google.com with SMTP id v19so4456853ote.8 for ; Wed, 04 Mar 2020 20:54:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=wxMB2pp+9PjI13jJjWt8ByuXtWLK+X9MK3SPC71Gnu4=; b=tJ/6smOavDVAgu3Gdv6UPcaz/ASw6W4rh6d3f/b7iBjY6Foxfv6i1G0OlFGnI0h645 wA0vSFhrAVZuAr2PJDr9E4WHV/usr5bRjaixTFpn8JjPp3QzkHVa+tncWWsSfQ9gGU/O 6OCZybxu/Ly8SyivT03y5Svx2ipVvnyrSkVDGOiGek+CA3OhEXmERwV4mxW5DXyzwWmt ODfNLKdcYqr5PNNI8tKVwTv4MkqptJjbpXUwR/rR/dN19neY/vhWLvyoY2YrNTsIPkvP nKghJJEO1ErZxQXjl2DWtHTDR4Lk8Iahf874jvpX4hFCMpZlNuHHd8+wXH5Jn3IHb92E JRsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=wxMB2pp+9PjI13jJjWt8ByuXtWLK+X9MK3SPC71Gnu4=; b=I2eQtwaUk3SzMboKiP+t6YYPSj7pfNNyRpxav51cBIJ0KwG+TjQXOPA701wYGCxuJq sz8hNQZAFkjYw62djiz2PoR310WRi2FAm+uBtEzEztAmdATOF99XIxbwuluz5AVOgbEk 2v0TomB9bLqQz0J2qisuXY1hzD4IvqQ6vcyVf0MWeifsE276E0M8g6CUCTWMDxDH0lmn k2LYr5IAZO+04oMRR9CmaEqRuloQ+Jg7rIkUzBBKJXysQFSUZmo+4GOKiWZtjEdENW71 7g9dtW/rWguqFhx5MOZ6pmDpFlKU18OGVY/Nhsu0b7eldexjvueMyt4Yu8jEBc5DRfeU BovQ== X-Gm-Message-State: ANhLgQ3Mg7jFUHI5Toox9w13A29EgxOZ1VnRRnurR7nRVKzsL43uXa2x RvZSMgf4UiMnxTjZxCu9aw58SQmuQ7hiIkNJwgeM6w== X-Google-Smtp-Source: ADFU+vsA/fXuJ3IRHxsfbCJ4aVofj1cKFBMR9e3ak4P1jFEdSsgTVK8BlhVGbuvdGoUrb3G4k+2cTTJNUxh+RFfBW2k= X-Received: by 2002:a9d:6:: with SMTP id 6mr5250710ota.191.1583384085099; Wed, 04 Mar 2020 20:54:45 -0800 (PST) MIME-Version: 1.0 References: <20200304233856.257891-1-shakeelb@google.com> In-Reply-To: From: Shakeel Butt Date: Wed, 4 Mar 2020 20:54:34 -0800 Message-ID: Subject: Re: [PATCH v2] net: memcg: late association of sock to memcg To: Eric Dumazet Cc: Roman Gushchin , Johannes Weiner , Michal Hocko , Andrew Morton , "David S . Miller" , Alexey Kuznetsov , Hideaki YOSHIFUJI , netdev , linux-mm , Cgroups , LKML Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Mar 4, 2020 at 8:38 PM Eric Dumazet wrote: > > On Wed, Mar 4, 2020 at 6:19 PM Shakeel Butt wrote: > > > > On Wed, Mar 4, 2020 at 5:36 PM Eric Dumazet wrote: > > > > > > On Wed, Mar 4, 2020 at 3:39 PM Shakeel Butt wrote: > > > > > > > > If a TCP socket is allocated in IRQ context or cloned from unassociated > > > > (i.e. not associated to a memcg) in IRQ context then it will remain > > > > unassociated for its whole life. Almost half of the TCPs created on the > > > > system are created in IRQ context, so, memory used by such sockets will > > > > not be accounted by the memcg. > > > > > > > > This issue is more widespread in cgroup v1 where network memory > > > > accounting is opt-in but it can happen in cgroup v2 if the source socket > > > > for the cloning was created in root memcg. > > > > > > > > To fix the issue, just do the late association of the unassociated > > > > sockets at accept() time in the process context and then force charge > > > > the memory buffer already reserved by the socket. > > > > > > > > Signed-off-by: Shakeel Butt > > > > --- > > > > Changes since v1: > > > > - added sk->sk_rmem_alloc to initial charging. > > > > - added synchronization to get memory usage and set sk_memcg race-free. > > > > > > > > net/ipv4/inet_connection_sock.c | 19 +++++++++++++++++++ > > > > 1 file changed, 19 insertions(+) > > > > > > > > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c > > > > index a4db79b1b643..7bcd657cd45e 100644 > > > > --- a/net/ipv4/inet_connection_sock.c > > > > +++ b/net/ipv4/inet_connection_sock.c > > > > @@ -482,6 +482,25 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err, bool kern) > > > > } > > > > spin_unlock_bh(&queue->fastopenq.lock); > > > > } > > > > + > > > > + if (mem_cgroup_sockets_enabled && !newsk->sk_memcg) { > > > > + int amt; > > > > + > > > > + /* atomically get the memory usage and set sk->sk_memcg. */ > > > > + lock_sock(newsk); > > > > + > > > > + /* The sk has not been accepted yet, no need to look at > > > > + * sk->sk_wmem_queued. > > > > + */ > > > > + amt = sk_mem_pages(newsk->sk_forward_alloc + > > > > + atomic_read(&sk->sk_rmem_alloc)); > > > > + mem_cgroup_sk_alloc(newsk); > > > > + > > > > + release_sock(newsk); > > > > + > > > > + if (newsk->sk_memcg) > > > > > > Most sockets in accept queue should have amt == 0, so maybe avoid > > > calling this thing only when amt == 0 ? > > > > > > > Thanks, will do in the next version. BTW I have tested with adding > > mdelay() here and running iperf3 and I did see non-zero amt. > > > > > Also I would release_sock(newsk) after this, otherwise incoming > > > packets could mess with newsk->sk_forward_alloc > > > > > > > I think that is fine. Once sk->sk_memcg is set then > > mem_cgroup_charge_skmem() will be called for new incoming packets. > > Here we just need to call mem_cgroup_charge_skmem() with amt before > > sk->sk_memcg was set. > > > Unfortunately, as soon as release_sock(newsk) is done, incoming > packets can be fed to the socket, > and completely change memory usage of the socket. > > For example, the whole queue might have been zapped, or collapsed, if > we receive a RST packet, > or if memory pressure asks us to prune the out of order queue. > > So you might charge something, then never uncharge it, since at > close() time the socket will have zero bytes to uncharge. > Ok, thanks for the explanation. I will fix this in the next version.