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 74EBFC46CD2 for ; Tue, 30 Jan 2024 07:03:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E4E546B0081; Tue, 30 Jan 2024 02:03:46 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DFE646B0082; Tue, 30 Jan 2024 02:03:46 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C9F296B0085; Tue, 30 Jan 2024 02:03:46 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id B5DEC6B0081 for ; Tue, 30 Jan 2024 02:03:46 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 51DC4A1ADE for ; Tue, 30 Jan 2024 07:03:46 +0000 (UTC) X-FDA: 81735087252.17.CF6173E Received: from mail-lj1-f180.google.com (mail-lj1-f180.google.com [209.85.208.180]) by imf13.hostedemail.com (Postfix) with ESMTP id 76AE620003 for ; Tue, 30 Jan 2024 07:03:44 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=i6aIGrHv; spf=pass (imf13.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.180 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706598224; a=rsa-sha256; cv=none; b=675bJkIR30dkPe+CxATRqfBpAY5JlrBZq5e8nqgeUX5Gxmj32sBwQIMbFguOvDws6Xh002 ZmTDYVsB6BBz623jjTEHRWW1doxJeOBhmDnxQ5c0kOE/e0BpIICupOc5a9R80iP9LOAoWa HAA8qEWfGdjA+mgKJrgqkxyLRq2OIwU= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=i6aIGrHv; spf=pass (imf13.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.180 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706598224; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=DIE4KQDb529O0G81Tw1SwOjqWduyvxr5+he0HDK9pg4=; b=6zR5URyP5oh7kqcP4uL8bZRL3njdREEAjFJhvjtG5fWUheE3ONFXAiVQK4uBvvAWBiFGDx Wqu6NnQ3W8akI6pqnCm5bnRomNqRefuNMW7yquTh6pyhM/NuYOoxyGV5pEd/IMZulwBvOH d5ypt9rYiZSlpZdfYs7d9tSqDYSijdo= Received: by mail-lj1-f180.google.com with SMTP id 38308e7fff4ca-2cf4fafa386so38689791fa.1 for ; Mon, 29 Jan 2024 23:03:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1706598223; x=1707203023; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=DIE4KQDb529O0G81Tw1SwOjqWduyvxr5+he0HDK9pg4=; b=i6aIGrHvMt5M5I/djZXGgFnP90ExeSnlQgKMATlNXSUUEOVt1YKjXzYmBLptezi5C9 a41rQ+kiJ5XW1y+pA3t9zAZvkvmMGy9ltnNrSZaBfeJJrhL+3F73gjJxINTsDkWJFGYK b2AX9LU/BBSqI4UQM/o/dvEppl+vvi0yPobUbTWsCQlRjOEjJI5LY2z9dHHMvI9nsXMm gExBmY4jJGePAq3QmEDJLqZFtG8GFTGAsexwrXDSJI7VxdkjyNjeJCVsFvlqNKDKvIuC Zs/Bq6gQCVK459snTA+Z2EL1GeCvfoPNjCa3FX0BWgVLs38/H/ZnpjvkJ0FJPPjVrTGC f5Cg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706598223; x=1707203023; h=content-transfer-encoding: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=DIE4KQDb529O0G81Tw1SwOjqWduyvxr5+he0HDK9pg4=; b=MrEgel/kS3OSSexvPvEr6c7Cn5YV8I0QfMVnr9lC2j/uHmRyipqxhodPe57pneQRw2 r8WnnBmrxXaMeYOtfiMksylQdjQs7GGku2Gakhst6LNRrmPT5vDPM8NcPjNH/MSBWqm3 inraDuuiv53WKk1TpQAe8SzOiitaouAQaFdL+Xp8WD1ws9hMt7WWpl3Po/36YTKUrQbI dI1dq7V8pOUYV5SUGve2mUlSXIY+f6lskXYvi4N0YIIfF9kB7jIynkkW8Pqh+0g3MBDn 1i+TPdxOQRJRMeXXX1zcz/d9XuH6TG8uRUy3hOMZbEF9Q7KTlZV5YKLlHWEydNErcSwC qD1g== X-Gm-Message-State: AOJu0Yy/mJm5PjrQlXb5zJJpXM6rtJwGmQGLOhQGvw+1x8STri6AHtj3 VW3h9ebvsSvUii5/0+K54jVXv6DGQngeybBQeHnHADB4vpkjQvNYJCPAld1MEZ5tYWI7yLC1vmT dZIAMNqEzDzH3q48PkSw9DNCB6fk= X-Google-Smtp-Source: AGHT+IGEnMW99hRYngYXqneuOspacZBUb2liuGzMKm4/XbYrYOCoYbv5g+Q0nVkvgf+GorwlY/RT/i5T8wJ+MclxTG8= X-Received: by 2002:a2e:b041:0:b0:2cf:2004:3614 with SMTP id d1-20020a2eb041000000b002cf20043614mr4785270ljl.15.1706598222755; Mon, 29 Jan 2024 23:03:42 -0800 (PST) MIME-Version: 1.0 References: <20240129175423.1987-1-ryncsn@gmail.com> <20240129175423.1987-4-ryncsn@gmail.com> <87sf2fgxi5.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: From: Kairui Song Date: Tue, 30 Jan 2024 15:03:25 +0800 Message-ID: Subject: Re: [PATCH v3 3/7] mm/swap: always account swapped in page into current memcg To: "Huang, Ying" Cc: linux-mm@kvack.org, Andrew Morton , Chris Li , Hugh Dickins , Johannes Weiner , Matthew Wilcox , Michal Hocko , Yosry Ahmed , David Hildenbrand , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 76AE620003 X-Stat-Signature: s164ctf9r86gqoxws891gswuhe8w3k3y X-Rspam-User: X-HE-Tag: 1706598224-638306 X-HE-Meta: U2FsdGVkX1/AGFFgIJBKrVm4MMXCdQ/oiD9DmxYBxl8g+5vcmkd1pCqmiUwZyH9qoUFhnhh4ylNoxBKQrDd7ggi/IgrefgdP26T0PAzKt6tSSUPyjXwXPQf/alMWZqSauWJI7MhKXA/H1i+0Y2GxMDoEMdupL1VZN63XNjZD5JXX2VB0yAdWGpPsCQTcWD7feH/PDQeImOaEY8gH/jy8V5VwHLe0y945uQQqKRR5/3/mNJTJHq9wpRV2kQdmEanBIcBN9fPd6ZjuffvyIUC5Zh9KQIpYPd/Rbv8vAOvBTOjgLJkF2pogrOuttCfWHkL9npQ0xk81vCRYrMB6b6/TzG8+epQVggElGBFMlI1dTTGj4EuRZ0qk2SIKW8svAgWMcKD5uzCGNTnGGlER4GeoEH4NbpsG1wnmrx4IEaxQFI4jFrHvKxhYjFHukGl8LOd109hvxhX/x9jW2LGetm3G3IJnPVFtLOC0o8JnrwTl6MYqb9BrKYfPefaTrJHJnJIoj6VwIZ1XnnxsxBzfHmcOE/P6HNxIo2AlNgk7pWPYwEHnXGszdrpTwfweohr3k1FTu5KUHi7Yr87Gj0VcLUrHxMd20FXv3a14Ay7NR/1A0j0ttoHSil0xeGTdjugitBMa41DkMdyW5O/AHbx3HoJx0BK7LtfeTNm2kvkzTIc69Ddz0xV9SUXtp6Fq8RGoflhU1GJIwXE78ykZfPS4aTInvwwEh2D1/Nc8FT2v9dXY6ODI3FNVGDPW/KferhHZxXTkvmuu6ABU7VKg5/Zbrw5z8N4JMB6xBT7LN2GeGmu3AlSF+KUTGca3QIrYLtSPd2qhCjutp9DhRAMUfbMKLgM0+nSvCwGrmrSGLlP+e3Wruqw1n3eWCk/jgteqVb+/pSjRuN3Nf5muf7HtQIC1zCt266BwLsHfkqRPQTTyHNLyIjmR1o7By/TFDNtgA7/hS2CjJ1hGeEvz1h7HNIUl9hK elPlpk5T zXJDyMJd3FSOlIao2cVrGnAfw4loBzRN+/eBsD8+Dz91nWobkSbUYxsDYYaPVwUES3EH6v4udIca28bTwZQog9DvIuUVnyeLYEIQXTAmI3hjP99l505n05LPU74UgVJI8av6iTU1stUE2OLml2pOLYA0Hw84YDYGCHd5Fmf0zkR3MyPOmZw8bodfdFc1TxLaGlkR61xaZycE4C5Jgc/WtOsh+1FMkerWHHG+9PGaJIKOM92StZNQqZBoYl22A+h57opqC/QFEREDAuf+JCpsJbHWS+leqaztmKBkXcSamE+d7/kXMicswjjFhPwQ1zJMiLBbTeYlvoXkQHup3Rb1z5ScFa3UTB9Evq4I2y5dcS3RFXSCRkr/N3mf91euliy02CfXDRQNQbIckR7JQt2mfKq2H25Yo/bn+mfr/d4Qd313v9gAXky/vqwDHYOtiLBmCUyCxalwDmcogOrbYHHd8y9kuto76UlCrOhi+FJxOhCT+rzs= 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: List-Subscribe: List-Unsubscribe: On Tue, Jan 30, 2024 at 3:01=E2=80=AFPM Kairui Song wrot= e: > > On Tue, Jan 30, 2024 at 2:14=E2=80=AFPM Huang, Ying wrote: > > > > Kairui Song writes: > > > > > From: Kairui Song > > > > > > Currently, mem_cgroup_swapin_charge_folio is always called with > > > mm =3D=3D NULL, except in swapin_direct. > > > > > > swapin_direct is only used when swapin should skip readahead > > > and swapcache (SWP_SYNCHRONOUS_IO). All other callers of > > > mem_cgroup_swapin_charge_folio are for swapin that should > > > not skip readahead and cache. > > > > > > This could cause swapin charging to behave differently depending > > > on swap device, which is unexpected. > > > > > > This is currently not happening because the only caller of > > > swapin_direct is the direct anon page fault path, where mm always > > > equals to current->mm, but will no longer be true if swapin_direct > > > is shared and have other callers (eg, swapoff) to share the > > > readahead skipping logic. > > > > > > So make swapin_direct also pass NULL for mm, so swpain charge > > > will behave consistently and not effected by type of swapin device > > > or readahead policy. > > > > > > After this, the second param of mem_cgroup_swapin_charge_folio is > > > never used now, so it can be safely dropped. > > > > > > Signed-off-by: Kairui Song > > > --- > > > include/linux/memcontrol.h | 4 ++-- > > > mm/memcontrol.c | 5 ++--- > > > mm/swap_state.c | 7 +++---- > > > 3 files changed, 7 insertions(+), 9 deletions(-) > > > > > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > > > index 20ff87f8e001..540590d80958 100644 > > > --- a/include/linux/memcontrol.h > > > +++ b/include/linux/memcontrol.h > > > @@ -693,7 +693,7 @@ static inline int mem_cgroup_charge(struct folio = *folio, struct mm_struct *mm, > > > int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gf= p, > > > long nr_pages); > > > > > > -int mem_cgroup_swapin_charge_folio(struct folio *folio, struct mm_st= ruct *mm, > > > +int mem_cgroup_swapin_charge_folio(struct folio *folio, > > > gfp_t gfp, swp_entry_t entry); > > > void mem_cgroup_swapin_uncharge_swap(swp_entry_t entry); > > > > > > @@ -1281,7 +1281,7 @@ static inline int mem_cgroup_hugetlb_try_charge= (struct mem_cgroup *memcg, > > > } > > > > > > static inline int mem_cgroup_swapin_charge_folio(struct folio *folio= , > > > - struct mm_struct *mm, gfp_t gfp, swp_entry_t en= try) > > > + gfp_t gfp, swp_entry_t entry) > > > { > > > return 0; > > > } > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > > index e4c8735e7c85..5852742df958 100644 > > > --- a/mm/memcontrol.c > > > +++ b/mm/memcontrol.c > > > @@ -7306,8 +7306,7 @@ int mem_cgroup_hugetlb_try_charge(struct mem_cg= roup *memcg, gfp_t gfp, > > > * > > > * Returns 0 on success. Otherwise, an error code is returned. > > > */ > > > -int mem_cgroup_swapin_charge_folio(struct folio *folio, struct mm_st= ruct *mm, > > > - gfp_t gfp, swp_entry_t entry) > > > +int mem_cgroup_swapin_charge_folio(struct folio *folio, gfp_t gfp, s= wp_entry_t entry) > > > { > > > struct mem_cgroup *memcg; > > > unsigned short id; > > > @@ -7320,7 +7319,7 @@ int mem_cgroup_swapin_charge_folio(struct folio= *folio, struct mm_struct *mm, > > > rcu_read_lock(); > > > memcg =3D mem_cgroup_from_id(id); > > > if (!memcg || !css_tryget_online(&memcg->css)) > > > - memcg =3D get_mem_cgroup_from_mm(mm); > > > + memcg =3D get_mem_cgroup_from_current(); > > > > The behavior of get_mem_cgroup_from_mm(NULL) and > > get_mem_cgroup_from_current() isn't same exactly. Are you sure that > > this is OK? > > Hi Ying, thank you very much for the careful review. > > IIUC, usually get_mem_cgroup_from_mm(NULL) is for allocations without > mm context (after set_active_memcg), so remote charging cgroup is used > first. > > But for swap cases it's a bit special, all swapin are issued from > userspace, so remote charging isn't useful. Not sure if it even may > potentially lead to charging into the wrong cgroup. > > And for this callsite, it's called only when `if (!memcg || > !css_tryget_online(&memcg->css))` is true, only case I know is swapoff > (and the memcg is dead) case, or there are some leaks. The behaviour Oh, actually shmem may also have the zombie cgroup issue, the conclusion is still the same though, the task accessed the shmem owns the charge. > of swapoff case has been discussed previously, so currently we just > charge it into the current task's memcg. > > This is indeed a potential behaviour change though, I can change it > back to get_mem_cgroup_from_mm(NULL), and post another patch later for > this and discuss in more details. > > > > > -- > > Best Regards, > > Huang, Ying