| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJD7tkZRH5-y-ux2hDt70JpmL5=YwRwrg_otiTbAei09x-GAJQ@mail.gmail.com>
Date: Tue, 3 Oct 2023 16:22:10 -0700
From: Yosry Ahmed <yosryahmed@...gle.com>
To: Nhat Pham <nphamcs@...il.com>
Cc: akpm@...ux-foundation.org, riel@...riel.com, hannes@...xchg.org,
mhocko@...nel.org, roman.gushchin@...ux.dev, shakeelb@...gle.com,
muchun.song@...ux.dev, tj@...nel.org, lizefan.x@...edance.com,
shuah@...nel.org, mike.kravetz@...cle.com, fvdl@...gle.com,
linux-mm@...ck.org, kernel-team@...a.com,
linux-kernel@...r.kernel.org, cgroups@...r.kernel.org
Subject: Re: [PATCH] memcontrol: only transfer the memcg data for migration
On Tue, Oct 3, 2023 at 4:14 PM Nhat Pham <nphamcs@...il.com> wrote:
>
> For most migration use cases, only transfer the memcg data from the old
> folio to the new folio, and clear the old folio's memcg data. No
> charging and uncharging will be done. These use cases include the new
> hugetlb memcg accounting behavior (which was not previously handled).
>
> This shaves off some work on the migration path, and avoids the
> temporary double charging of a folio during its migration.
>
> The only exception is replace_page_cache_folio(), which will use the old
> mem_cgroup_migrate() (now renamed to mem_cgroup_replace_folio). In that
> context, the isolation of the old page isn't quite as thorough as with
> migration, so we cannot use our new implementation directly.
>
> This patch is the result of the following discussion on the new hugetlb
> memcg accounting behavior:
>
> https://lore.kernel.org/lkml/20231003171329.GB314430@monkey/
>
> Reported-by: Mike Kravetz <mike.kravetz@...cle.com>
> Closes: https://lore.kernel.org/lkml/20231003171329.GB314430@monkey/
> Suggested-by: Johannes Weiner <hannes@...xchg.org>
> Signed-off-by: Nhat Pham <nphamcs@...il.com>
Does this patch fit before or after your series? In both cases I think
there might be a problem for bisectability.
> ---
> include/linux/memcontrol.h | 7 ++++++
> mm/filemap.c | 2 +-
> mm/memcontrol.c | 45 +++++++++++++++++++++++++++++++++++---
> mm/migrate.c | 3 +--
> 4 files changed, 51 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index a827e2129790..e3eaa123256b 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -711,6 +711,8 @@ static inline void mem_cgroup_uncharge_list(struct list_head *page_list)
>
> void mem_cgroup_cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages);
>
> +void mem_cgroup_replace_folio(struct folio *old, struct folio *new);
> +
> void mem_cgroup_migrate(struct folio *old, struct folio *new);
>
> /**
> @@ -1294,6 +1296,11 @@ static inline void mem_cgroup_cancel_charge(struct mem_cgroup *memcg,
> {
> }
>
> +static inline void mem_cgroup_replace_folio(struct folio *old,
> + struct folio *new)
> +{
> +}
> +
> static inline void mem_cgroup_migrate(struct folio *old, struct folio *new)
> {
> }
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 9481ffaf24e6..673745219c82 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -819,7 +819,7 @@ void replace_page_cache_folio(struct folio *old, struct folio *new)
> new->mapping = mapping;
> new->index = offset;
>
> - mem_cgroup_migrate(old, new);
> + mem_cgroup_replace_folio(old, new);
>
> xas_lock_irq(&xas);
> xas_store(&xas, new);
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6660684f6f97..cbaa26605b3d 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -7316,16 +7316,17 @@ void __mem_cgroup_uncharge_list(struct list_head *page_list)
> }
>
> /**
> - * mem_cgroup_migrate - Charge a folio's replacement.
> + * mem_cgroup_replace_folio - Charge a folio's replacement.
> * @old: Currently circulating folio.
> * @new: Replacement folio.
> *
> * Charge @new as a replacement folio for @old. @old will
> - * be uncharged upon free.
> + * be uncharged upon free. This is only used by the page cache
> + * (in replace_page_cache_folio()).
> *
> * Both folios must be locked, @new->mapping must be set up.
> */
> -void mem_cgroup_migrate(struct folio *old, struct folio *new)
> +void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
> {
> struct mem_cgroup *memcg;
> long nr_pages = folio_nr_pages(new);
> @@ -7364,6 +7365,44 @@ void mem_cgroup_migrate(struct folio *old, struct folio *new)
> local_irq_restore(flags);
> }
>
> +/**
> + * mem_cgroup_migrate - Transfer the memcg data from the old to the new folio.
> + * @old: Currently circulating folio.
> + * @new: Replacement folio.
> + *
> + * Transfer the memcg data from the old folio to the new folio for migration.
> + * The old folio's data info will be cleared. Note that the memory counters
> + * will remain unchanged throughout the process.
> + *
> + * Both folios must be locked, @new->mapping must be set up.
> + */
> +void mem_cgroup_migrate(struct folio *old, struct folio *new)
> +{
> + struct mem_cgroup *memcg;
> +
> + VM_BUG_ON_FOLIO(!folio_test_locked(old), old);
> + VM_BUG_ON_FOLIO(!folio_test_locked(new), new);
> + VM_BUG_ON_FOLIO(folio_test_anon(old) != folio_test_anon(new), new);
> + VM_BUG_ON_FOLIO(folio_nr_pages(old) != folio_nr_pages(new), new);
> +
> + if (mem_cgroup_disabled())
> + return;
> +
> + memcg = folio_memcg(old);
> + /*
> + * Note that it is normal to see !memcg for a hugetlb folio.
> + * It could have been allocated when memory_hugetlb_accounting was not
> + * selected, for e.g.
> + */
> + VM_WARN_ON_ONCE_FOLIO(!memcg, old);
> + if (!memcg)
> + return;
> +
> + /* Transfer the charge and the css ref */
> + commit_charge(new, memcg);
> + old->memcg_data = 0;
> +}
> +
> DEFINE_STATIC_KEY_FALSE(memcg_sockets_enabled_key);
> EXPORT_SYMBOL(memcg_sockets_enabled_key);
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 7d1804c4a5d9..6034c7ed1d65 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -633,8 +633,7 @@ void folio_migrate_flags(struct folio *newfolio, struct folio *folio)
>
> folio_copy_owner(newfolio, folio);
>
> - if (!folio_test_hugetlb(folio))
> - mem_cgroup_migrate(folio, newfolio);
> + mem_cgroup_migrate(folio, newfolio);
> }
> EXPORT_SYMBOL(folio_migrate_flags);
>
> --
> 2.34.1
>
Powered by blists - more mailing lists