memcg: workingset: remove folio_memcg_rcu usage
Subsystems (5)
Shakeel Butt
maintainerPATCH 1/1 memcg: workingset: remove folio_memcg_rcu usage
The function workingset_activation() is called from folio_mark_accessed() with the guarantee that the given folio can not be freed under us in workingset_activation(). In addition, the association of the folio and its memcg can not be broken here because charge migration is no more. There is no need to use folio_memcg_rcu. Simply use folio_memcg_charged() because that is what this function cares about.
---
Andrew, please put this patch after the charge migration deprecation series.
include/linux/memcontrol.h | 35 ----------------------------------- mm/workingset.c | 10 ++-------- 2 files changed, 2 insertions(+), 43 deletions(-)
include/linux/memcontrol.h +0 -35
@@ -443,35 +443,6 @@ static inline bool folio_memcg_charged(struct folio *folio)return __folio_memcg (folio) !=NULL ; } -/** -* folio_memcg_rcu - Locklessly get the memory cgroup associated with a folio. -* @folio: Pointer to the folio. -* -* This function assumes that the folio is known to have a -* proper memory cgroup pointer. It's not safe to call this function -* against some type of folios, e.g. slab folios or ex-slab folios. -* -* Return: A pointer to the memory cgroup associated with the folio, -* or NULL. -*/ -static inline struct mem_cgroup *folio_memcg_rcu (struct folio *folio) -{ -unsigned long memcg_data =READ_ONCE (folio->memcg_data); - -VM_BUG_ON_FOLIO (folio_test_slab (folio), folio); - -if (memcg_data &MEMCG_DATA_KMEM ) { -struct obj_cgroup *objcg; - - objcg = (void *)(memcg_data & ~OBJEXTS_FLAGS_MASK ); -return obj_cgroup_memcg (objcg); - } - -WARN_ON_ONCE (!rcu_read_lock_held ()); - -return (struct mem_cgroup *)(memcg_data & ~OBJEXTS_FLAGS_MASK ); -} -/* * folio_memcg_check - Get the memory cgroup associated with a folio. * @folio: Pointer to the folio.
@@ -1084,12 +1055,6 @@ static inline struct mem_cgroup *folio_memcg(struct folio *folio)return NULL ; } -static inline struct mem_cgroup *folio_memcg_rcu (struct folio *folio) -{ -WARN_ON_ONCE (!rcu_read_lock_held ()); -return NULL ; -} -static inline struct mem_cgroup *folio_memcg_check (struct folio *folio) {return NULL ;
mm/workingset.c +2 -8
@@ -591,9 +591,6 @@ void workingset_refault(struct folio *folio, void *shadow)*/ void workingset_activation (struct folio *folio) { -struct mem_cgroup *memcg; - -rcu_read_lock ();/* * Filter non-memcg pages here, e.g. unmap can call * mark_page_accessed() on VDSO pages.
@@ -601,12 +598,9 @@ void workingset_activation(struct folio *folio)* XXX: See workingset_refault() - this should return * root_mem_cgroup even for !CONFIG_MEMCG. */ - memcg =folio_memcg_rcu (folio); -if (!mem_cgroup_disabled () && !memcg) - goto out; +if (mem_cgroup_disabled () || !folio_memcg_charged (folio)) +return ;workingset_age_nonresident (folio_lruvec (folio),folio_nr_pages (folio)); -out: -rcu_read_unlock (); }/*
Signature · 1 line
2.43.5
Quoted 88 lines · Shakeel Butt
The function
workingset_activation()is called fromfolio_mark_accessed()with the guarantee that the given folio can not be freed under us inworkingset_activation(). In addition, the association of the folio and its memcg can not be broken here because charge migration is no more. There is no need to usefolio_memcg_rcu. Simply usefolio_memcg_charged()because that is what this function cares about.Signed-off-by Shakeel Butt <[email protected]>Suggested-by Yu Zhao <[email protected]>---
Andrew, please put this patch after the charge migration deprecation series.
include/linux/memcontrol.h | 35 ----------------------------------- mm/workingset.c | 10 ++-------- 2 files changed, 2 insertions(+), 43 deletions(-)include/linux/memcontrol.h +0
-35@@ -443,35 +443,6 @@ static inline bool folio_memcg_charged(struct folio *folio)return __folio_memcg (folio) !=NULL ; } -/** -* folio_memcg_rcu - Locklessly get the memory cgroup associated with a folio. -* @folio: Pointer to the folio. -* -* This function assumes that the folio is known to have a -* proper memory cgroup pointer. It's not safe to call this function -* against some type of folios, e.g. slab folios or ex-slab folios. -* -* Return: A pointer to the memory cgroup associated with the folio, -* or NULL. -*/ -static inline struct mem_cgroup *folio_memcg_rcu (struct folio *folio) -{ -unsigned long memcg_data =READ_ONCE (folio->memcg_data); - -VM_BUG_ON_FOLIO (folio_test_slab (folio), folio); - -if (memcg_data &MEMCG_DATA_KMEM ) { -struct obj_cgroup *objcg; - - objcg = (void *)(memcg_data & ~OBJEXTS_FLAGS_MASK ); -return obj_cgroup_memcg (objcg); - } - -WARN_ON_ONCE (!rcu_read_lock_held ()); - -return (struct mem_cgroup *)(memcg_data & ~OBJEXTS_FLAGS_MASK ); -} -/* * folio_memcg_check - Get the memory cgroup associated with a folio. * @folio: Pointer to the folio. @@ -1084,12 +1055,6 @@ static inline struct mem_cgroup *folio_memcg(struct folio *folio)return NULL ; } -static inline struct mem_cgroup *folio_memcg_rcu (struct folio *folio) -{ -WARN_ON_ONCE (!rcu_read_lock_held ()); -return NULL ; -} -static inline struct mem_cgroup *folio_memcg_check (struct folio *folio) {return NULL ;mm/workingset.c +0
-3@@ -591,9 +591,6 @@ void workingset_refault(struct folio *folio, void *shadow)*/ void workingset_activation (struct folio *folio) { -struct mem_cgroup *memcg; - -rcu_read_lock ();/* * Filter non-memcg pages here, e.g. unmap can call * mark_page_accessed() on VDSO pages.
mm/workingset.c void workingset_activation(struct folio *folio) · line 599
Quoted 2 lines
mm/workingset.c +0
-0@@ -601,12 +598,9 @@ void workingset_activation(struct folio *folio)* XXX: See workingset_refault() - this should return * root_mem_cgroup even for !CONFIG_MEMCG.
The "XXX" part of the comments doesn't apply anymore.
mm/workingset.c void workingset_activation(struct folio *folio) · line 603
Quoted 7 lines
mm/workingset.c +2
-3*/ - memcg =folio_memcg_rcu (folio); -if (!mem_cgroup_disabled () && !memcg) - goto out; +if (mem_cgroup_disabled () || !folio_memcg_charged (folio)) +return ;workingset_age_nonresident (folio_lruvec (folio),folio_nr_pages (folio));
if (mem_cgroup_disabled() || folio_memcg_charged(folio))
workingset_age_nonresident()
We still want to call workingset_age_nonresident() when memcg is disabled. (We just want to "Filter non-memcg pages here" when memcg is enabled, as the comment above says, which I assume still applies?)
Quoted 8 lines
mm/workingset.c +0
-3-out: -rcu_read_unlock (); }/* -- 2.43.5
Shakeel Butt
maintainerQuoted 102 lines · Yu Zhao
Quoted 88 lines · Shakeel Butt
The function
workingset_activation()is called fromfolio_mark_accessed()with the guarantee that the given folio can not be freed under us inworkingset_activation(). In addition, the association of the folio and its memcg can not be broken here because charge migration is no more. There is no need to usefolio_memcg_rcu. Simply usefolio_memcg_charged()because that is what this function cares about.Signed-off-by Shakeel Butt <[email protected]>Suggested-by Yu Zhao <[email protected]>---
Andrew, please put this patch after the charge migration deprecation series.
include/linux/memcontrol.h | 35 ----------------------------------- mm/workingset.c | 10 ++-------- 2 files changed, 2 insertions(+), 43 deletions(-)include/linux/memcontrol.h +0
-35@@ -443,35 +443,6 @@ static inline bool folio_memcg_charged(struct folio *folio)return __folio_memcg (folio) !=NULL ; } -/** -* folio_memcg_rcu - Locklessly get the memory cgroup associated with a folio. -* @folio: Pointer to the folio. -* -* This function assumes that the folio is known to have a -* proper memory cgroup pointer. It's not safe to call this function -* against some type of folios, e.g. slab folios or ex-slab folios. -* -* Return: A pointer to the memory cgroup associated with the folio, -* or NULL. -*/ -static inline struct mem_cgroup *folio_memcg_rcu (struct folio *folio) -{ -unsigned long memcg_data =READ_ONCE (folio->memcg_data); - -VM_BUG_ON_FOLIO (folio_test_slab (folio), folio); - -if (memcg_data &MEMCG_DATA_KMEM ) { -struct obj_cgroup *objcg; - - objcg = (void *)(memcg_data & ~OBJEXTS_FLAGS_MASK ); -return obj_cgroup_memcg (objcg); - } - -WARN_ON_ONCE (!rcu_read_lock_held ()); - -return (struct mem_cgroup *)(memcg_data & ~OBJEXTS_FLAGS_MASK ); -} -/* * folio_memcg_check - Get the memory cgroup associated with a folio. * @folio: Pointer to the folio. @@ -1084,12 +1055,6 @@ static inline struct mem_cgroup *folio_memcg(struct folio *folio)return NULL ; } -static inline struct mem_cgroup *folio_memcg_rcu (struct folio *folio) -{ -WARN_ON_ONCE (!rcu_read_lock_held ()); -return NULL ; -} -static inline struct mem_cgroup *folio_memcg_check (struct folio *folio) {return NULL ;mm/workingset.c +0
-3@@ -591,9 +591,6 @@ void workingset_refault(struct folio *folio, void *shadow)*/ void workingset_activation (struct folio *folio) { -struct mem_cgroup *memcg; - -rcu_read_lock ();/* * Filter non-memcg pages here, e.g. unmap can call * mark_page_accessed() on VDSO pages. mm/workingset.c void workingset_activation(struct folio *folio) · line 599
Quoted 2 lines
mm/workingset.c +0
-0@@ -601,12 +598,9 @@ void workingset_activation(struct folio *folio)* XXX: See workingset_refault() - this should return * root_mem_cgroup even for !CONFIG_MEMCG. The "XXX" part of the comments doesn't apply anymore.
mm/workingset.c void workingset_activation(struct folio *folio) · line 603
Quoted 7 lines
mm/workingset.c +2
-3*/ - memcg =folio_memcg_rcu (folio); -if (!mem_cgroup_disabled () && !memcg) - goto out; +if (mem_cgroup_disabled () || !folio_memcg_charged (folio)) +return ;workingset_age_nonresident (folio_lruvec (folio),folio_nr_pages (folio));
Quoted 7 lines
if (
mem_cgroup_disabled()||folio_memcg_charged(folio))
workingset_age_nonresident()We still want to call
workingset_age_nonresident()when memcg is disabled. (We just want to "Filter non-memcg pages here" when memcg is enabled, as the comment above says, which I assume still applies?)
Yes, you are right. Let me send the v2 with the fix.