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=unavailable 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 AEDE2C3F2D1 for ; Thu, 5 Mar 2020 21:59:51 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5A8A02072A for ; Thu, 5 Mar 2020 21:59:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="lSsXtoDI" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5A8A02072A 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 DC94D6B0007; Thu, 5 Mar 2020 16:59:50 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D54156B0008; Thu, 5 Mar 2020 16:59:50 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C1B926B000A; Thu, 5 Mar 2020 16:59:50 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0069.hostedemail.com [216.40.44.69]) by kanga.kvack.org (Postfix) with ESMTP id A61446B0007 for ; Thu, 5 Mar 2020 16:59:50 -0500 (EST) Received: from smtpin14.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 497DE2C12 for ; Thu, 5 Mar 2020 21:59:50 +0000 (UTC) X-FDA: 76562676540.14.farm54_55d8547c0f20d X-HE-Tag: farm54_55d8547c0f20d X-Filterd-Recvd-Size: 5940 Received: from mail-oi1-f196.google.com (mail-oi1-f196.google.com [209.85.167.196]) by imf34.hostedemail.com (Postfix) with ESMTP for ; Thu, 5 Mar 2020 21:59:49 +0000 (UTC) Received: by mail-oi1-f196.google.com with SMTP id l12so439260oil.9 for ; Thu, 05 Mar 2020 13:59:49 -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=GZ9X7pBOQ32PghBEC4+dP85I/S+wu1As9RhQFRnXXto=; b=lSsXtoDIO0iLHjqPicvvXtqjyT7OR/kBmY+VLmgBRevQBgKkkDTBQRGLxzsgcELH6t gqiGETJBzstXBAnv9oYvbcIBIz89GpuahxVSYeZiGIBZ4JvWLqjwVYKVD1bfwB074NTL 9MZik/rr8kw3M1VyRiiRn60Ts6yKRUKNcOSsYWGch+SBvf7epfYp8rNztCtiP/kPuX9K 10Bcd7ggd2vSizpgzvw3rq7TbbT5qAgso27hxp1LZMY9bXozbQWsZzRKL1BXWCK66cCq FzIrI80C7cvzbZRPcHeBZj4Pxdrxrivy0+6UjMRhYPpEW17iy/T0kUB0lmxycqhI6IJV wl0w== 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=GZ9X7pBOQ32PghBEC4+dP85I/S+wu1As9RhQFRnXXto=; b=lKnKHsDoahAzL4/UPqsZ1lF9sAWNXf3Uj6GLAK6Rdq76OyNad0QdsMBzX7eM/Z5ztI eZV85sst1uJvbPjK8cZ4TWZJ6osbbnd0ix/5/zd0xP20gNLX3RFm+l9+0yRBVixUIYOW ShgKj8aY6l3jKH9fgF0JZJw/TkpEuDRSzLB78iXf8AWD1mbwVIjlTOtDPuonrVI4R21P qdxjy/Iyf6gJBQ4mpZ6GojHsDfpInJVthPnhJwLq8zB3AX60AheWNq9Gv1vjeH3ukFk/ S4meH6M0L/HOzO7+KlcJjQrNWC/90oCe17DgyX2e6g/7DP/BbvwlQ0E9Yf6YFHjiGf7B X+1Q== X-Gm-Message-State: ANhLgQ0C2pV4zgMIvAZ0Z14A09jvYrwo1RBEp+SxAZXbYgHvAr3rJ0qK e7hAcRbgwfMeVIPs6Ut6TsSlKDnZo0JGz12PX9emdg== X-Google-Smtp-Source: ADFU+vtYXWLwZtcPM8JDPJKFFjMDljTV5RXVkWBf8UUOHRKcT1Dlz5TI0VyF36iQc7ZItQBmxNnUhKuVpOCALNdBsMo= X-Received: by 2002:a05:6808:64e:: with SMTP id z14mr383631oih.79.1583445588674; Thu, 05 Mar 2020 13:59:48 -0800 (PST) MIME-Version: 1.0 References: <20200305205525.245058-1-shakeelb@google.com> <9505d35b-f9fc-149b-6df5-e65ad95acabb@gmail.com> In-Reply-To: <9505d35b-f9fc-149b-6df5-e65ad95acabb@gmail.com> From: Shakeel Butt Date: Thu, 5 Mar 2020 13:59:37 -0800 Message-ID: Subject: Re: [PATCH v3] net: memcg: late association of sock to memcg To: Eric Dumazet Cc: Eric Dumazet , 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 Thu, Mar 5, 2020 at 1:17 PM Eric Dumazet wrote: > > > > On 3/5/20 12:55 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 v2: > > - Additional check for charging. > > - Release the sock after charging. > > > > 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 | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c > > index a4db79b1b643..5face55cf818 100644 > > --- a/net/ipv4/inet_connection_sock.c > > +++ b/net/ipv4/inet_connection_sock.c > > @@ -482,6 +482,26 @@ 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, set and charge the > > + * 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); > > + if (newsk->sk_memcg && amt) > > + mem_cgroup_charge_skmem(newsk->sk_memcg, amt); > > + > > + release_sock(newsk); > > + } > > out: > > release_sock(sk); > > if (req) > > > > This patch looks fine, but why keeping the mem_cgroup_sk_alloc(newsk); > in sk_clone_lock() ? > > Note that all TCP sk_clone_lock() calls happen in softirq context. So, basically re-doing something like 9f1c2674b328 ("net: memcontrol: defer call to mem_cgroup_sk_alloc()") in this patch. I am fine with that. Roman, any concerns?