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 D61E9C46CD2 for ; Tue, 30 Jan 2024 07:01:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 412906B0078; Tue, 30 Jan 2024 02:01:42 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3C2E46B007B; Tue, 30 Jan 2024 02:01:42 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 263BB6B0081; Tue, 30 Jan 2024 02:01:42 -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 13B106B0078 for ; Tue, 30 Jan 2024 02:01:42 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id D17E0807CE for ; Tue, 30 Jan 2024 07:01:41 +0000 (UTC) X-FDA: 81735082002.17.D10ED2C Received: from mail-lj1-f173.google.com (mail-lj1-f173.google.com [209.85.208.173]) by imf13.hostedemail.com (Postfix) with ESMTP id 002FB2000A for ; Tue, 30 Jan 2024 07:01:39 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=NMkwpvMa; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.173 as permitted sender) smtp.mailfrom=ryncsn@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706598100; 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=FOcr0/zytsqYYa9EmZhgnI5xTH3X0lz+GqOcsR1oGH0=; b=7MaIy2PJ21tNBTXag9NA4kb45QoHRpHCJJn7aWz/UrC83v5F5rw8q4kyr6Q7L8yKJuYzeg Lb8kV+VRdU4CtQsx3ZrUCPmzLCodlehuliuJjcM8QpsvzTFMdArx4SwLqpmRX5f7pM1x9V kiX35sb6p74DFzd+EkI/pqPRs16G6bQ= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=NMkwpvMa; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.173 as permitted sender) smtp.mailfrom=ryncsn@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706598100; a=rsa-sha256; cv=none; b=o7Ghg++fbNoRmlNOuvbYU4/XOiMTTk+UnBuCj45xg+sP4XdYIs8Yh+OWmqgQN+9jZ1CtOz 7+pXD5Lzf5HQTG41/ObAl/P3Qmds4K42AXh9W9XozoCvT1O8BlRZspgjNvuRjBOcTJRO5q 36ALuknLNno0/5g+CgBtMm9DrIIYjTk= Received: by mail-lj1-f173.google.com with SMTP id 38308e7fff4ca-2cf2fdd518bso31905971fa.1 for ; Mon, 29 Jan 2024 23:01:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1706598098; x=1707202898; 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=FOcr0/zytsqYYa9EmZhgnI5xTH3X0lz+GqOcsR1oGH0=; b=NMkwpvMaBz0sZFaHjJAElWTpXztXzjLnqX8TrAYctUhXJ7fj3RGWcmEDoUGJG/4DK/ 0Ua3F3ASRGLFShNbzrewIxCE2o7rdjvClxCrkZeR1wbvVBa1C4Fb8myjyuPFyALnh99A +RPPHJqQ5Sa++qAwKCjlPk/+l8HVX6YRRtIo9e1V9/cYry0kJfFHgPIPO97AjJV7N1Ay gqkpmOmkAbJ2yagciVxE8Vj16dEPSbqe6z3dUdmGFTEgxaZhLwwhPcbVGdThGJsYN1zp Zz7FvjtNS4R0nQUnwEJpursQ8aSk3RTLnPb8ucEZamk6izeyqp3c8fw85zS+KUyCHydi BFBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706598098; x=1707202898; 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=FOcr0/zytsqYYa9EmZhgnI5xTH3X0lz+GqOcsR1oGH0=; b=GeV5Kn4LVQ1Vfc/SJKS8jThTXagy3Vn6GIsgFzgcii6eJRlhZD0tNlrVhQuzKvw7yw QpHJTEUo83Mv8E5Aes8lE6eoHirriyhNP4TRZpsQtTyxYcL9M18Z3rHURF+LFmKgw8e6 XIHGXYlUjKP0TV0HLppF6YJsobSRJNMKlGjHfhfU5chces/UZt8K+mU/oIlYH5dD3t8x fGOAQDsOb31e+lhWaLoBorqxnNABe6DN0UlUJ7wdAW252A2EeuP7WOSzdjfRVwMUO49t P/WQaB25RRtH0IWEGcsys+9bSV8x6L2yO/co6HBVIHrGizn2Jxla7xXqBQD2wy3UxNzu T3/A== X-Gm-Message-State: AOJu0YxulsWUMeqxP5rqJa8RI5Aj4MzagDaB+v+71LrE0pCxPThIod8N xVrWEjOUxL146pt2ht9V3wzTvb9LZTFtBnMrpYeTLbtSaC+waVMHwXwvpHcOHc9FSjNlhlxpuMF 3FAkEhTpZ3zSy1HLbbSHZhWfEag4= X-Google-Smtp-Source: AGHT+IFCMFLmoHrPCGUWiRvRoNh1CqYnql8wz3vSCot8F+nrDokESSEpzx5qasJqkWZKkJUy/zxKred38mjbt1wpfiQ= X-Received: by 2002:a2e:7c08:0:b0:2cf:1be6:70f4 with SMTP id x8-20020a2e7c08000000b002cf1be670f4mr4723552ljc.44.1706598097823; Mon, 29 Jan 2024 23:01:37 -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: <87sf2fgxi5.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Kairui Song Date: Tue, 30 Jan 2024 15:01:20 +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-Rspam-User: X-Stat-Signature: xnuz1wkdaf8cbfqzyafterciranbkq8k X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 002FB2000A X-HE-Tag: 1706598099-441775 X-HE-Meta: U2FsdGVkX1+j15RX64S/HgkdwVLpArQdQTILBTC+aXzwvbEdmAXSMAuWxUfazlaPPmSrlsPOWR9dRzD9Z0lQ6RhTcsDmmHd6J9kLj/L7JF9wx/Eaam/gWba1hU3t6zhySoxIMqlogqDRsCsUEun6jPyIUv5A4m+2mar8ytFkP2FIOlli5QoTLT26w8mer+NB+wxSVg8WPAdiZF7kIsr8GkqlZR4ycEKQJNuPTV5pgERHrn1oKVAH5Sq7LZo0dqDRUMt0f0tY3nW4gRRYz3d1JqToOR9mf2pvw/CQABZhdM8HpRywQHwSVb6UlriXT9uRV78RJw1IHfp3fvHK3/yXqeBAZjxJRWjYmPnl4zbrfPJPWU9G7Ix622wPbegQpPBwBst6i4AxzM+NcJ+h8ZLAFhfo4Ppi28R8R8sB/jAzmssHqeWzW+QYfu3AlMsidiTK+PpXryLbq5thnYTIgC9XuwQK2mnBygf4LBubXJiEbD/8y56h1vIJ4npNjhbEkjO8EZUxf80jRCT70TJupQhcTj3dUPSVBT3e7RHhnQ4IL5Fpj6n/a/b26YGMo7EbgCin+mcmEovKmit3BgYcLwNLsY6DstAlhH6K8TxqEGA3jJ0cP17ODtFQWFlz3mZ50YTOIdFVZpWjss6wCKjtUDvQin0Yuz5sTHG3IJcrxHKQVX5gOo9Jsy3wUYvss5vYatL69n0e+Gu8HjxHH4BjOP5a0oLeBuPW2+Hp/CrhzGE/lOXRBMpRrl+lNyGiBzeAJV+b4LEwQ1uWuQUgifNCOjp7UvH4nV5jsb3GbUcie+85GJ5i+utR24NVGdLqafTsiipCIwSnYoCV4ZwxvvUN+maMKJVcOmRPrzbA00ds4EnuljPivbAI1v85SREElnhKAT5HljckJ1cQ4Lew9g7S6HYNnaNhWhfcClOHxCUt6WrAgtyWnKDOlI3Lvj5H5YRRW/wFmkutDIdLdiZ4u4jxAL6 L+1MKjWw aXv6sF3QcQ0stGCp06ZfCMmx/goea941K6/26s90/v9b+NO5eH80C5Tz7RhOyWXjk7xlsG0ITBOatm2yTpzgLyxyUzclmSN6qOm9zshuh1KU2mZ9Y8oYTrOzUaC6Ax9Jr+RTTfCj2F2LtZHwdo1Vd6BnumwKgjpfci7Y+Zu6x5MTe0mdcE/HmR3sUo/kekJlMwesQPAJ+hBTSEAF2PkpRdH4DNqxkoYiZSerNq2bB+Mi9R5h+T9MZ3ZkciX5pUzo3Bn+Vgiw5lWzadTtYKhCoZcASJeYEpFt1toXgZbjWOuVgnlH9FDcBeBE/IkLAi33nL4LjbaV0S2/Ej/W4cMA7YX6DwaSE64sS0EhCdkiNya9EbIv+tdX5vU6VmpwWYwCgtmZhkzzpZPAeTzCVLgQLa8aLG8WUGn+gJQ5S9GSKTIiiPPjrvFNxFJSd+XNTXl7vmyyJ7T/tt9MvyU+kIB3Pk9NwMAg/zbQtZWOoXUeSrHL070A= 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 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 *f= olio, struct mm_struct *mm, > > int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp, > > long nr_pages); > > > > -int mem_cgroup_swapin_charge_folio(struct folio *folio, struct mm_stru= ct *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(s= truct mem_cgroup *memcg, > > } > > > > static inline int mem_cgroup_swapin_charge_folio(struct folio *folio, > > - struct mm_struct *mm, gfp_t gfp, swp_entry_t entr= y) > > + 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_cgro= up *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_stru= ct *mm, > > - gfp_t gfp, swp_entry_t entry) > > +int mem_cgroup_swapin_charge_folio(struct folio *folio, gfp_t gfp, swp= _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 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