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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A5539C352A1 for ; Tue, 6 Dec 2022 19:14:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 26D7B8E0003; Tue, 6 Dec 2022 14:14:04 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 21CB98E0001; Tue, 6 Dec 2022 14:14:04 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 096BB8E0003; Tue, 6 Dec 2022 14:14:04 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id EAF348E0001 for ; Tue, 6 Dec 2022 14:14:03 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id B01DE1C6669 for ; Tue, 6 Dec 2022 19:14:03 +0000 (UTC) X-FDA: 80212831566.05.85F41F6 Received: from mail-yb1-f176.google.com (mail-yb1-f176.google.com [209.85.219.176]) by imf12.hostedemail.com (Postfix) with ESMTP id 59F8E40014 for ; Tue, 6 Dec 2022 19:14:03 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=XYKeMYNu; spf=pass (imf12.hostedemail.com: domain of edumazet@google.com designates 209.85.219.176 as permitted sender) smtp.mailfrom=edumazet@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1670354043; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=nsGstoMegH9NCBA+9S++kT1eq+4LgLp/g5jcJesallk=; b=LSEkkzwZW9nbExok1iMMWMZZLy48RvNnCXUb0CK6cy9om7VDl2sK8wBrTHxMsSw9PTg9eb fmRsC46+7vkzUoxMDNX1c2h+HRILRFpJiVjFdKouGR0BsPnzGbPpTMt0YtVC7FsRvjWLa0 2cTvojQSfjag+MQpkf/WT8j4eQfo20w= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=XYKeMYNu; spf=pass (imf12.hostedemail.com: domain of edumazet@google.com designates 209.85.219.176 as permitted sender) smtp.mailfrom=edumazet@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1670354043; a=rsa-sha256; cv=none; b=5/w1kfH1yHY70n3Jt9X8cjCYi09igvEnonJPsmQGRwV49BgdfUC87RRWC9az+ulWf+6bzd Jf5g1wNQV3uHYMpOw2PJmDR7yGp7fFnQd7ZWRERDVftChqh1eEXjCCEgwyY/ii7Vrv7l7j eH2apJjR83f/C6U/3hCyR/0Nso5QUiI= Received: by mail-yb1-f176.google.com with SMTP id c140so19810155ybf.11 for ; Tue, 06 Dec 2022 11:14:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=nsGstoMegH9NCBA+9S++kT1eq+4LgLp/g5jcJesallk=; b=XYKeMYNurkuR6sR3Bimkf7Sn8dHGUPB/zyUAi5+51h+e/3eKyuQGNJmkMVJA2I8mIO CRj1XB6I+6aa93zGfB/XreNSlmTLe7W5eHZVi/gq5zyZ7EEkjk4q49HSkidWnAE7J2UZ yhegxEOwEF91TJJvN5JjZ1tgaxWE7Z0j3VlZWiVxWL3QhIxeqHNLGUahLLYMgnMWo6m7 LWQBrrP98g3lSPdohibAd0RZqtMzH2ASNda0/fz+SLByFsys48P+q9qh3dZu1Okmq1jf RtZC+vv238QR0AXfwTzcUPRCnYQP38SscsXB3ILVgMfK8CzeVP8pxS7S6iPEzQGHWcIP fdBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=nsGstoMegH9NCBA+9S++kT1eq+4LgLp/g5jcJesallk=; b=xXV7ugdS3TvtD2/+g47XC9Jc5usX53BEbW2vk4U8ErScQlNfxjczeYscpi5G7GJ9GP UuXcUV4WAV6AHj4rlEHLa2ySMieC+tGwqX8t+FU8h4bqWUNV8fHMZMYgI/gtEVHfOcAm nh3VQR3THJ33pCI/ip3eetfVLwQ36VScOTP9t2cke2aJnmpGllt+GNWuON3jgUGoPDHn ieEs9wTVsjDvm60y6JsU6H3U31TMqAiuLWrsGAvH81hXX76T0K1hRCaWj2innCbzES5+ 8RGfAOgIudW/q0hbNUB+23ywAtNM3czLplScrQiTPQHLRkSesr1CxnltgyU0jo5Woy6L nLsw== X-Gm-Message-State: ANoB5pknkAbcr+qojg3evbDJpghA2TUD5q5e7eqiow6rDD5aAXzhOZUE 9a8HkXS9WB2iD8AT54HZQkhlaQKm6HmNO2+Gb+b7QQ== X-Google-Smtp-Source: AA0mqf7m6QMPk8QvSd5EosLtYWGKojHXXveXDH0jx2Rge6XwaoiwkjiHmCyuNB2B6z2oCDVoagQzgUk1NRNWwoi5oyU= X-Received: by 2002:a25:24d:0:b0:6fd:2917:cf60 with SMTP id 74-20020a25024d000000b006fd2917cf60mr18389766ybc.427.1670354042180; Tue, 06 Dec 2022 11:14:02 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Eric Dumazet Date: Tue, 6 Dec 2022 20:13:50 +0100 Message-ID: Subject: Re: Low TCP throughput due to vmpressure with swap enabled To: Johannes Weiner Cc: Ivan Babrou , Linux MM , Linux Kernel Network Developers , linux-kernel , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton , "David S. Miller" , Hideaki YOSHIFUJI , David Ahern , Jakub Kicinski , Paolo Abeni , cgroups@vger.kernel.org, kernel-team Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 59F8E40014 X-Stat-Signature: 7oa1gf6sdbq3675fkjopbcid7ariq4z3 X-Rspam-User: X-Spamd-Result: default: False [-2.90 / 9.00]; BAYES_HAM(-6.00)[100.00%]; SORBS_IRL_BL(3.00)[209.85.219.176:from]; BAD_REP_POLICIES(0.10)[]; RCVD_NO_TLS_LAST(0.10)[]; MIME_GOOD(-0.10)[text/plain]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; FROM_EQ_ENVFROM(0.00)[]; DMARC_POLICY_ALLOW(0.00)[google.com,reject]; RCPT_COUNT_TWELVE(0.00)[17]; DKIM_TRACE(0.00)[google.com:+]; TO_MATCH_ENVRCPT_SOME(0.00)[]; PREVIOUSLY_DELIVERED(0.00)[linux-mm@kvack.org]; R_DKIM_ALLOW(0.00)[google.com:s=20210112]; ARC_SIGNED(0.00)[hostedemail.com:s=arc-20220608:i=1]; FROM_HAS_DN(0.00)[]; R_SPF_ALLOW(0.00)[+ip4:209.85.128.0/17]; TO_DN_SOME(0.00)[]; ARC_NA(0.00)[] X-Rspamd-Server: rspam08 X-HE-Tag: 1670354043-701603 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 Tue, Dec 6, 2022 at 8:00 PM Johannes Weiner wrote: > > On Mon, Dec 05, 2022 at 04:50:46PM -0800, Ivan Babrou wrote: > > And now I can see plenty of this: > > > > [ 108.156707][ T5175] socket pressure[2]: 4294673429 > > [ 108.157050][ T5175] socket pressure[2]: 4294673429 > > [ 108.157301][ T5175] socket pressure[2]: 4294673429 > > [ 108.157581][ T5175] socket pressure[2]: 4294673429 > > [ 108.157874][ T5175] socket pressure[2]: 4294673429 > > [ 108.158254][ T5175] socket pressure[2]: 4294673429 > > > > I think the first result below is to blame: > > > > $ rg '.->socket_pressure' mm > > mm/memcontrol.c > > 5280: memcg->socket_pressure = jiffies; > > 7198: memcg->socket_pressure = 0; > > 7201: memcg->socket_pressure = 1; > > 7211: memcg->socket_pressure = 0; > > 7215: memcg->socket_pressure = 1; > > Hoo boy, that's a silly mistake indeed. Thanks for tracking it down. > > > While we set socket_pressure to either zero or one in > > mem_cgroup_charge_skmem, it is still initialized to jiffies on memcg > > creation. Zero seems like a more appropriate starting point. With that > > change I see it working as expected with no TCP speed bumps. My > > ebpf_exporter program also looks happy and reports zero clamps in my > > brief testing. > > Excellent, now this behavior makes sense. > > > I also think we should downgrade socket_pressure from "unsigned long" > > to "bool", as it only holds zero and one now. > > Sounds good to me! > > Attaching the updated patch below. If nobody has any objections, I'll > add a proper changelog, reported-bys, sign-off etc and send it out. > > --- > include/linux/memcontrol.h | 8 +++--- > include/linux/vmpressure.h | 7 ++--- > mm/memcontrol.c | 20 +++++++++---- > mm/vmpressure.c | 58 ++++++-------------------------------- > mm/vmscan.c | 15 +--------- > 5 files changed, 30 insertions(+), 78 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index e1644a24009c..ef1c388be5b3 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -283,11 +283,11 @@ struct mem_cgroup { > atomic_long_t memory_events[MEMCG_NR_MEMORY_EVENTS]; > atomic_long_t memory_events_local[MEMCG_NR_MEMORY_EVENTS]; > > - unsigned long socket_pressure; > + /* Socket memory allocations have failed */ > + bool socket_pressure; > > /* Legacy tcp memory accounting */ > bool tcpmem_active; > - int tcpmem_pressure; > > #ifdef CONFIG_MEMCG_KMEM > int kmemcg_id; > @@ -1701,10 +1701,10 @@ void mem_cgroup_sk_alloc(struct sock *sk); > void mem_cgroup_sk_free(struct sock *sk); > static inline bool mem_cgroup_under_socket_pressure(struct mem_cgroup *memcg) > { > - if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && memcg->tcpmem_pressure) > + if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && memcg->socket_pressure) && READ_ONCE(memcg->socket_pressure)) > return true; > do { > - if (time_before(jiffies, READ_ONCE(memcg->socket_pressure))) > + if (memcg->socket_pressure) if (READ_ONCE(...)) > return true; > } while ((memcg = parent_mem_cgroup(memcg))); > return false; > diff --git a/include/linux/vmpressure.h b/include/linux/vmpressure.h > index 6a2f51ebbfd3..20d93de37a17 100644 > --- a/include/linux/vmpressure.h > +++ b/include/linux/vmpressure.h > @@ -11,9 +11,6 @@ > #include > > struct vmpressure { > - unsigned long scanned; > - unsigned long reclaimed; > - > unsigned long tree_scanned; > unsigned long tree_reclaimed; > /* The lock is used to keep the scanned/reclaimed above in sync. */ > @@ -30,7 +27,7 @@ struct vmpressure { > struct mem_cgroup; > > #ifdef CONFIG_MEMCG > -extern void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree, > +extern void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, > unsigned long scanned, unsigned long reclaimed); > extern void vmpressure_prio(gfp_t gfp, struct mem_cgroup *memcg, int prio); > > @@ -44,7 +41,7 @@ extern int vmpressure_register_event(struct mem_cgroup *memcg, > extern void vmpressure_unregister_event(struct mem_cgroup *memcg, > struct eventfd_ctx *eventfd); > #else > -static inline void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree, > +static inline void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, > unsigned long scanned, unsigned long reclaimed) {} > static inline void vmpressure_prio(gfp_t gfp, struct mem_cgroup *memcg, > int prio) {} > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 2d8549ae1b30..0d4b9dbe775a 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -5277,7 +5277,6 @@ static struct mem_cgroup *mem_cgroup_alloc(void) > vmpressure_init(&memcg->vmpressure); > INIT_LIST_HEAD(&memcg->event_list); > spin_lock_init(&memcg->event_list_lock); > - memcg->socket_pressure = jiffies; > #ifdef CONFIG_MEMCG_KMEM > memcg->kmemcg_id = -1; > INIT_LIST_HEAD(&memcg->objcg_list); > @@ -7195,10 +7194,10 @@ bool mem_cgroup_charge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages, > struct page_counter *fail; > > if (page_counter_try_charge(&memcg->tcpmem, nr_pages, &fail)) { > - memcg->tcpmem_pressure = 0; Orthogonal to your patch, but: Maybe avoid touching this cache line too often and use READ/WRITE_ONCE() ? if (READ_ONCE(memcg->socket_pressure)) WRITE_ONCE(memcg->socket_pressure, false); > + memcg->socket_pressure = false; > return true; > } > - memcg->tcpmem_pressure = 1; > + memcg->socket_pressure = true; Same remark. > if (gfp_mask & __GFP_NOFAIL) { > page_counter_charge(&memcg->tcpmem, nr_pages); > return true; > @@ -7206,12 +7205,21 @@ bool mem_cgroup_charge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages, > return false; > } > > - if (try_charge(memcg, gfp_mask, nr_pages) == 0) { > - mod_memcg_state(memcg, MEMCG_SOCK, nr_pages); > - return true; > + if (try_charge(memcg, gfp_mask & ~__GFP_NOFAIL, nr_pages) == 0) { > + memcg->socket_pressure = false; same remark. > + goto success; > + } > + memcg->socket_pressure = true; same remark. > + if (gfp_mask & __GFP_NOFAIL) { > + try_charge(memcg, gfp_mask, nr_pages); > + goto success; > } > > return false; > + > +success: > + mod_memcg_state(memcg, MEMCG_SOCK, nr_pages); > + return true; > } > > /**