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 7DE2CC3F2CE for ; Thu, 5 Mar 2020 02:19:02 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 37CFB20866 for ; Thu, 5 Mar 2020 02:19:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="Oj+tVF6M" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 37CFB20866 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 C5FEB6B0003; Wed, 4 Mar 2020 21:19:01 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C3E716B0005; Wed, 4 Mar 2020 21:19:01 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AFDE56B0007; Wed, 4 Mar 2020 21:19:01 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0087.hostedemail.com [216.40.44.87]) by kanga.kvack.org (Postfix) with ESMTP id 946AF6B0003 for ; Wed, 4 Mar 2020 21:19:01 -0500 (EST) Received: from smtpin04.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 2F484181AC9C6 for ; Thu, 5 Mar 2020 02:19:01 +0000 (UTC) X-FDA: 76559700882.04.crib66_6bd0e5e5d5145 X-HE-Tag: crib66_6bd0e5e5d5145 X-Filterd-Recvd-Size: 6668 Received: from mail-ot1-f65.google.com (mail-ot1-f65.google.com [209.85.210.65]) by imf39.hostedemail.com (Postfix) with ESMTP for ; Thu, 5 Mar 2020 02:19:00 +0000 (UTC) Received: by mail-ot1-f65.google.com with SMTP id x97so4193078ota.6 for ; Wed, 04 Mar 2020 18:19:00 -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=8Gmox/kCX2AOvaG1blFHqNiRizW+Wg7yNRzLpxgtKLU=; b=Oj+tVF6Mg8622gqzDs/4jpJI1NLx3VtEGWbfX7g0xxhbdfx+25pa99Lsk7g38hu0xr bW6MHWgTJ/Q+hCiWynOOt8aZ2fAfmlQtHHEkSS8/Woxe1P+Z2eOgeKdbM0LZxKpP0NPa LbatkAZDksdGn3Gadji/3AY/vi/Hd7HTMvZ/r8OKOtvZ9n4mCVwgjQ4O3yfxY84GfAZd d3EIO690GmWj4Pml5zsgwrTmFrey0GMiLKB0DrcSKznw8GLyRghPPl8B+d2mqC38Is09 UQEPTrGsxGNV4g+AwmUv8YPXlOErgDtVtchuGrujjcxTIuWDEzOPO96OVkfArkdzm+wC 5SzQ== 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=8Gmox/kCX2AOvaG1blFHqNiRizW+Wg7yNRzLpxgtKLU=; b=aEA9KDio4ce1g0lCy1eERFC3wr45pgZO3qS7JLXFrfXubYYxkHk9wSMkKGoLS0Iaea FUd12KCVe4+lIQYoysuHu1Z2vKB5NPjyvck6CdP2DZt2OsFtrTHLDC/vcRC/VwQCzRFi iTciwk1WPinfmCG5J6o3NHAOI331vKVzs4m9+BsASYaeANmZg29GRzBGBkynBws1nc7a Unn7UVuBzbkL4n1LFhfg80uc4thASrzPUW1i2XuXGdnYI5UhHuDGQOkIllqmjZ3H58Iu c+E6kQimglrk54Pqg9Hd5j/PGiIl6IidUfXg8TKsc/7lI85TUibS5R/s8k4z6AcwPZVH omwA== X-Gm-Message-State: ANhLgQ1p8nA9T2yH3NhMc9W1p35J+80riD1y8GJLUURYhFuU2uvc/LSz RPJs5Q8ZfgvbClfpSczk6RFe/AaIEFPXN40RFGXg+w== X-Google-Smtp-Source: ADFU+vucht722GyYw42Nwlm95SH5G6wJ3ZVu/v034CHYYgRaQtLUnV72QjnUmxeeCj4dZxFLaNJk5PCxnSwz75EsPJI= X-Received: by 2002:a05:6830:46:: with SMTP id d6mr4856010otp.81.1583374739727; Wed, 04 Mar 2020 18:18:59 -0800 (PST) MIME-Version: 1.0 References: <20200304233856.257891-1-shakeelb@google.com> In-Reply-To: From: Shakeel Butt Date: Wed, 4 Mar 2020 18:18:48 -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 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. > if (amt && newsk->sk_memcg) > mem_cgroup_charge_skmem(newsk->sk_memcg, amt); > release_sock(newsk); > > Also, I wonder if mem_cgroup_charge_skmem() has been used at all > these last four years > on arches with PAGE_SIZE != 4096 > > ( SK_MEM_QUANTUM is not anymore PAGE_SIZE, but 4096) > Oh so sk_mem_pages() does not really give the number of pages. Yeah this needs a fix for non-4906 page size architectures. Though I can understand why this has not been caught yet. Network memory accounting is opt-in in cgroup v1 and most of the users still use v1. In cgroup v2, it is enabled and there is no way to opt-out. Facebook is a well-known v2 user and it seems like they don't have non-4096 page size arch systems.