The memory cgroup maintains a private ID infrastructure decoupled from the cgroup IDs for swapout records and shadow entries. The main motivation of this private ID infra is best described in the commit 73f576c04b941 ("mm: memcontrol: fix cgroup creation failure after many small jobs"). Unfortunetely some users have started exposing these private IDs to the userspace where they should have used the cgroup IDs which are already exposed to the userspace. Let's rename the memcg ID APIs to explicitly mark them private. No functional change is intended. Signed-off-by: Shakeel Butt --- include/linux/memcontrol.h | 24 ++++++++++++++--- mm/list_lru.c | 2 +- mm/memcontrol-v1.c | 6 ++--- mm/memcontrol-v1.h | 4 +-- mm/memcontrol.c | 55 +++++++++++++++++++++----------------- mm/workingset.c | 8 +++--- 6 files changed, 61 insertions(+), 38 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index fd400082313a..1c4224bcfb23 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -65,7 +65,7 @@ struct mem_cgroup_reclaim_cookie { #define MEM_CGROUP_ID_SHIFT 16 -struct mem_cgroup_id { +struct mem_cgroup_private_id { int id; refcount_t ref; }; @@ -191,7 +191,7 @@ struct mem_cgroup { struct cgroup_subsys_state css; /* Private memcg ID. Used to ID objects that outlive the cgroup */ - struct mem_cgroup_id id; + struct mem_cgroup_private_id id; /* Accounted resources */ struct page_counter memory; /* Both v1 & v2 */ @@ -821,13 +821,19 @@ void mem_cgroup_iter_break(struct mem_cgroup *, struct mem_cgroup *); void mem_cgroup_scan_tasks(struct mem_cgroup *memcg, int (*)(struct task_struct *, void *), void *arg); -static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg) +static inline unsigned short mem_cgroup_private_id(struct mem_cgroup *memcg) { if (mem_cgroup_disabled()) return 0; return memcg->id.id; } +struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id); + +static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg) +{ + return mem_cgroup_private_id(memcg); +} struct mem_cgroup *mem_cgroup_from_id(unsigned short id); #ifdef CONFIG_SHRINKER_DEBUG @@ -1290,6 +1296,18 @@ static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id) return NULL; } +static inline unsigned short mem_cgroup_private_id(struct mem_cgroup *memcg) +{ + return 0; +} + +static inline struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id) +{ + WARN_ON_ONCE(id); + /* XXX: This should always return root_mem_cgroup */ + return NULL; +} + #ifdef CONFIG_SHRINKER_DEBUG static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg) { diff --git a/mm/list_lru.c b/mm/list_lru.c index 37b642f6cbda..13b9f66d950e 100644 --- a/mm/list_lru.c +++ b/mm/list_lru.c @@ -369,7 +369,7 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid, xa_for_each(&lru->xa, index, mlru) { rcu_read_lock(); - memcg = mem_cgroup_from_id(index); + memcg = mem_cgroup_from_private_id(index); if (!mem_cgroup_tryget(memcg)) { rcu_read_unlock(); continue; diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c index 0b50cb122ff3..0e3d972fad33 100644 --- a/mm/memcontrol-v1.c +++ b/mm/memcontrol-v1.c @@ -635,14 +635,14 @@ void memcg1_swapout(struct folio *folio, swp_entry_t entry) * have an ID allocated to it anymore, charge the closest online * ancestor for the swap instead and transfer the memory+swap charge. */ - swap_memcg = mem_cgroup_id_get_online(memcg); + swap_memcg = mem_cgroup_private_id_get_online(memcg); nr_entries = folio_nr_pages(folio); /* Get references for the tail pages, too */ if (nr_entries > 1) - mem_cgroup_id_get_many(swap_memcg, nr_entries - 1); + mem_cgroup_private_id_get_many(swap_memcg, nr_entries - 1); mod_memcg_state(swap_memcg, MEMCG_SWAP, nr_entries); - swap_cgroup_record(folio, mem_cgroup_id(swap_memcg), entry); + swap_cgroup_record(folio, mem_cgroup_private_id(swap_memcg), entry); folio_unqueue_deferred_split(folio); folio->memcg_data = 0; diff --git a/mm/memcontrol-v1.h b/mm/memcontrol-v1.h index e92b21af92b1..49933925b4ba 100644 --- a/mm/memcontrol-v1.h +++ b/mm/memcontrol-v1.h @@ -28,8 +28,8 @@ unsigned long memcg_events(struct mem_cgroup *memcg, int event); unsigned long memcg_page_state_output(struct mem_cgroup *memcg, int item); int memory_stat_show(struct seq_file *m, void *v); -void mem_cgroup_id_get_many(struct mem_cgroup *memcg, unsigned int n); -struct mem_cgroup *mem_cgroup_id_get_online(struct mem_cgroup *memcg); +void mem_cgroup_private_id_get_many(struct mem_cgroup *memcg, unsigned int n); +struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg); /* Cgroup v1-specific declarations */ #ifdef CONFIG_MEMCG_V1 diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 75fc22a33b28..25ad8433df2e 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3554,38 +3554,38 @@ static void memcg_wb_domain_size_changed(struct mem_cgroup *memcg) */ #define MEM_CGROUP_ID_MAX ((1UL << MEM_CGROUP_ID_SHIFT) - 1) -static DEFINE_XARRAY_ALLOC1(mem_cgroup_ids); +static DEFINE_XARRAY_ALLOC1(mem_cgroup_private_ids); -static void mem_cgroup_id_remove(struct mem_cgroup *memcg) +static void mem_cgroup_private_id_remove(struct mem_cgroup *memcg) { if (memcg->id.id > 0) { - xa_erase(&mem_cgroup_ids, memcg->id.id); + xa_erase(&mem_cgroup_private_ids, memcg->id.id); memcg->id.id = 0; } } -void __maybe_unused mem_cgroup_id_get_many(struct mem_cgroup *memcg, +void __maybe_unused mem_cgroup_private_id_get_many(struct mem_cgroup *memcg, unsigned int n) { refcount_add(n, &memcg->id.ref); } -static void mem_cgroup_id_put_many(struct mem_cgroup *memcg, unsigned int n) +static void mem_cgroup_private_id_put_many(struct mem_cgroup *memcg, unsigned int n) { if (refcount_sub_and_test(n, &memcg->id.ref)) { - mem_cgroup_id_remove(memcg); + mem_cgroup_private_id_remove(memcg); /* Memcg ID pins CSS */ css_put(&memcg->css); } } -static inline void mem_cgroup_id_put(struct mem_cgroup *memcg) +static inline void mem_cgroup_private_id_put(struct mem_cgroup *memcg) { - mem_cgroup_id_put_many(memcg, 1); + mem_cgroup_private_id_put_many(memcg, 1); } -struct mem_cgroup *mem_cgroup_id_get_online(struct mem_cgroup *memcg) +struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg) { while (!refcount_inc_not_zero(&memcg->id.ref)) { /* @@ -3604,15 +3604,20 @@ struct mem_cgroup *mem_cgroup_id_get_online(struct mem_cgroup *memcg) } /** - * mem_cgroup_from_id - look up a memcg from a memcg id + * mem_cgroup_from_private_id - look up a memcg from a memcg id * @id: the memcg id to look up * * Caller must hold rcu_read_lock(). */ -struct mem_cgroup *mem_cgroup_from_id(unsigned short id) +struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id) { WARN_ON_ONCE(!rcu_read_lock_held()); - return xa_load(&mem_cgroup_ids, id); + return xa_load(&mem_cgroup_private_ids, id); +} + +struct mem_cgroup *mem_cgroup_from_id(unsigned short id) +{ + return mem_cgroup_from_private_id(id); } #ifdef CONFIG_SHRINKER_DEBUG @@ -3711,7 +3716,7 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent) if (!memcg) return ERR_PTR(-ENOMEM); - error = xa_alloc(&mem_cgroup_ids, &memcg->id.id, NULL, + error = xa_alloc(&mem_cgroup_private_ids, &memcg->id.id, NULL, XA_LIMIT(1, MEM_CGROUP_ID_MAX), GFP_KERNEL); if (error) goto fail; @@ -3771,7 +3776,7 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent) lru_gen_init_memcg(memcg); return memcg; fail: - mem_cgroup_id_remove(memcg); + mem_cgroup_private_id_remove(memcg); __mem_cgroup_free(memcg); return ERR_PTR(error); } @@ -3854,7 +3859,7 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css) css_get(css); /* - * Ensure mem_cgroup_from_id() works once we're fully online. + * Ensure mem_cgroup_from_private_id() works once we're fully online. * * We could do this earlier and require callers to filter with * css_tryget_online(). But right now there are no users that @@ -3863,13 +3868,13 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css) * publish it here at the end of onlining. This matches the * regular ID destruction during offlining. */ - xa_store(&mem_cgroup_ids, memcg->id.id, memcg, GFP_KERNEL); + xa_store(&mem_cgroup_private_ids, memcg->id.id, memcg, GFP_KERNEL); return 0; offline_kmem: memcg_offline_kmem(memcg); remove_id: - mem_cgroup_id_remove(memcg); + mem_cgroup_private_id_remove(memcg); return -ENOMEM; } @@ -3892,7 +3897,7 @@ static void mem_cgroup_css_offline(struct cgroup_subsys_state *css) drain_all_stock(memcg); - mem_cgroup_id_put(memcg); + mem_cgroup_private_id_put(memcg); } static void mem_cgroup_css_released(struct cgroup_subsys_state *css) @@ -4779,7 +4784,7 @@ int mem_cgroup_swapin_charge_folio(struct folio *folio, struct mm_struct *mm, id = lookup_swap_cgroup_id(entry); rcu_read_lock(); - memcg = mem_cgroup_from_id(id); + memcg = mem_cgroup_from_private_id(id); if (!memcg || !css_tryget_online(&memcg->css)) memcg = get_mem_cgroup_from_mm(mm); rcu_read_unlock(); @@ -5174,22 +5179,22 @@ int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry) return 0; } - memcg = mem_cgroup_id_get_online(memcg); + memcg = mem_cgroup_private_id_get_online(memcg); if (!mem_cgroup_is_root(memcg) && !page_counter_try_charge(&memcg->swap, nr_pages, &counter)) { memcg_memory_event(memcg, MEMCG_SWAP_MAX); memcg_memory_event(memcg, MEMCG_SWAP_FAIL); - mem_cgroup_id_put(memcg); + mem_cgroup_private_id_put(memcg); return -ENOMEM; } /* Get references for the tail pages, too */ if (nr_pages > 1) - mem_cgroup_id_get_many(memcg, nr_pages - 1); + mem_cgroup_private_id_get_many(memcg, nr_pages - 1); mod_memcg_state(memcg, MEMCG_SWAP, nr_pages); - swap_cgroup_record(folio, mem_cgroup_id(memcg), entry); + swap_cgroup_record(folio, mem_cgroup_private_id(memcg), entry); return 0; } @@ -5206,7 +5211,7 @@ void __mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages) id = swap_cgroup_clear(entry, nr_pages); rcu_read_lock(); - memcg = mem_cgroup_from_id(id); + memcg = mem_cgroup_from_private_id(id); if (memcg) { if (!mem_cgroup_is_root(memcg)) { if (do_memsw_account()) @@ -5215,7 +5220,7 @@ void __mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages) page_counter_uncharge(&memcg->swap, nr_pages); } mod_memcg_state(memcg, MEMCG_SWAP, -nr_pages); - mem_cgroup_id_put_many(memcg, nr_pages); + mem_cgroup_private_id_put_many(memcg, nr_pages); } rcu_read_unlock(); } diff --git a/mm/workingset.c b/mm/workingset.c index e9f05634747a..13422d304715 100644 --- a/mm/workingset.c +++ b/mm/workingset.c @@ -254,7 +254,7 @@ static void *lru_gen_eviction(struct folio *folio) hist = lru_hist_from_seq(min_seq); atomic_long_add(delta, &lrugen->evicted[hist][type][tier]); - return pack_shadow(mem_cgroup_id(memcg), pgdat, token, workingset); + return pack_shadow(mem_cgroup_private_id(memcg), pgdat, token, workingset); } /* @@ -271,7 +271,7 @@ static bool lru_gen_test_recent(void *shadow, struct lruvec **lruvec, unpack_shadow(shadow, &memcg_id, &pgdat, token, workingset); - memcg = mem_cgroup_from_id(memcg_id); + memcg = mem_cgroup_from_private_id(memcg_id); *lruvec = mem_cgroup_lruvec(memcg, pgdat); max_seq = READ_ONCE((*lruvec)->lrugen.max_seq); @@ -395,7 +395,7 @@ void *workingset_eviction(struct folio *folio, struct mem_cgroup *target_memcg) lruvec = mem_cgroup_lruvec(target_memcg, pgdat); /* XXX: target_memcg can be NULL, go through lruvec */ - memcgid = mem_cgroup_id(lruvec_memcg(lruvec)); + memcgid = mem_cgroup_private_id(lruvec_memcg(lruvec)); eviction = atomic_long_read(&lruvec->nonresident_age); eviction >>= bucket_order; workingset_age_nonresident(lruvec, folio_nr_pages(folio)); @@ -456,7 +456,7 @@ bool workingset_test_recent(void *shadow, bool file, bool *workingset, * would be better if the root_mem_cgroup existed in all * configurations instead. */ - eviction_memcg = mem_cgroup_from_id(memcgid); + eviction_memcg = mem_cgroup_from_private_id(memcgid); if (!mem_cgroup_tryget(eviction_memcg)) eviction_memcg = NULL; rcu_read_unlock(); -- 2.47.3 Remove the CONFIG_SHRINKER_DEBUG guards around mem_cgroup_ino() and mem_cgroup_get_from_ino(). These APIs provide a way to get a memcg's cgroup inode number and to look up a memcg from an inode number respectively. Making these functions unconditionally available allows other in-kernel users to leverage them without requiring CONFIG_SHRINKER_DEBUG to be enabled. No functional change for existing users. Signed-off-by: Shakeel Butt --- include/linux/memcontrol.h | 4 ---- mm/memcontrol.c | 2 -- 2 files changed, 6 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 1c4224bcfb23..77f32be26ea8 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -836,14 +836,12 @@ static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg) } struct mem_cgroup *mem_cgroup_from_id(unsigned short id); -#ifdef CONFIG_SHRINKER_DEBUG static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg) { return memcg ? cgroup_ino(memcg->css.cgroup) : 0; } struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino); -#endif static inline struct mem_cgroup *mem_cgroup_from_seq(struct seq_file *m) { @@ -1308,7 +1306,6 @@ static inline struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id) return NULL; } -#ifdef CONFIG_SHRINKER_DEBUG static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg) { return 0; @@ -1318,7 +1315,6 @@ static inline struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) { return NULL; } -#endif static inline struct mem_cgroup *mem_cgroup_from_seq(struct seq_file *m) { diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 25ad8433df2e..e85816960e38 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3620,7 +3620,6 @@ struct mem_cgroup *mem_cgroup_from_id(unsigned short id) return mem_cgroup_from_private_id(id); } -#ifdef CONFIG_SHRINKER_DEBUG struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) { struct cgroup *cgrp; @@ -3641,7 +3640,6 @@ struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) return memcg; } -#endif static void free_mem_cgroup_per_node_info(struct mem_cgroup_per_node *pn) { -- 2.47.3 Change mem_cgroup_get_from_ino() to return NULL on error instead of ERR_PTR values. This simplifies the API: NULL indicates failure, and a valid pointer indicates success with a CSS reference held that the caller must release via mem_cgroup_put(). Signed-off-by: Shakeel Butt --- mm/memcontrol.c | 6 ++---- mm/shrinker_debug.c | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index e85816960e38..92beb74482fa 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3624,17 +3624,15 @@ struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) { struct cgroup *cgrp; struct cgroup_subsys_state *css; - struct mem_cgroup *memcg; + struct mem_cgroup *memcg = NULL; cgrp = cgroup_get_from_id(ino); if (IS_ERR(cgrp)) - return ERR_CAST(cgrp); + return NULL; css = cgroup_get_e_css(cgrp, &memory_cgrp_subsys); if (css) memcg = container_of(css, struct mem_cgroup, css); - else - memcg = ERR_PTR(-ENOENT); cgroup_put(cgrp); diff --git a/mm/shrinker_debug.c b/mm/shrinker_debug.c index 20eaee3e97f7..8aaeb8f5c3af 100644 --- a/mm/shrinker_debug.c +++ b/mm/shrinker_debug.c @@ -130,7 +130,7 @@ static ssize_t shrinker_debugfs_scan_write(struct file *file, if (shrinker->flags & SHRINKER_MEMCG_AWARE) { memcg = mem_cgroup_get_from_ino(ino); - if (!memcg || IS_ERR(memcg)) + if (!memcg) return -ENOENT; if (!mem_cgroup_online(memcg)) { -- 2.47.3 Switch mem_cgroup_ino() from using cgroup_ino() to cgroup_id(). The cgroup_ino() returns the kernfs inode number while cgroup_id() returns the kernfs node ID. For 64-bit systems, they are the same. Also cgroup_get_from_id() expects 64-bit node ID which is called by mem_cgroup_get_from_ino(). Change the type from unsigned long to u64 to match cgroup_id()'s return type, and update the format specifiers accordingly. Note that the names mem_cgroup_ino() and mem_cgroup_get_from_ino() are now misnomers since they deal with cgroup IDs rather than inode numbers. A follow-up patch will rename them. Signed-off-by: Shakeel Butt --- include/linux/memcontrol.h | 10 +++++----- mm/memcontrol.c | 2 +- mm/shrinker_debug.c | 7 ++++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 77f32be26ea8..c823150ec288 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -836,12 +836,12 @@ static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg) } struct mem_cgroup *mem_cgroup_from_id(unsigned short id); -static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg) +static inline u64 mem_cgroup_ino(struct mem_cgroup *memcg) { - return memcg ? cgroup_ino(memcg->css.cgroup) : 0; + return memcg ? cgroup_id(memcg->css.cgroup) : 0; } -struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino); +struct mem_cgroup *mem_cgroup_get_from_ino(u64 ino); static inline struct mem_cgroup *mem_cgroup_from_seq(struct seq_file *m) { @@ -1306,12 +1306,12 @@ static inline struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id) return NULL; } -static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg) +static inline u64 mem_cgroup_ino(struct mem_cgroup *memcg) { return 0; } -static inline struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) +static inline struct mem_cgroup *mem_cgroup_get_from_ino(u64 ino) { return NULL; } diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 92beb74482fa..1ff2f9bd820c 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3620,7 +3620,7 @@ struct mem_cgroup *mem_cgroup_from_id(unsigned short id) return mem_cgroup_from_private_id(id); } -struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) +struct mem_cgroup *mem_cgroup_get_from_ino(u64 ino) { struct cgroup *cgrp; struct cgroup_subsys_state *css; diff --git a/mm/shrinker_debug.c b/mm/shrinker_debug.c index 8aaeb8f5c3af..7ef16a0b2959 100644 --- a/mm/shrinker_debug.c +++ b/mm/shrinker_debug.c @@ -70,7 +70,7 @@ static int shrinker_debugfs_count_show(struct seq_file *m, void *v) memcg_aware ? memcg : NULL, count_per_node); if (total) { - seq_printf(m, "%lu", mem_cgroup_ino(memcg)); + seq_printf(m, "%llu", mem_cgroup_ino(memcg)); for_each_node(nid) seq_printf(m, " %lu", count_per_node[nid]); seq_putc(m, '\n'); @@ -106,7 +106,8 @@ static ssize_t shrinker_debugfs_scan_write(struct file *file, size_t size, loff_t *pos) { struct shrinker *shrinker = file->private_data; - unsigned long nr_to_scan = 0, ino, read_len; + unsigned long nr_to_scan = 0, read_len; + u64 ino; struct shrink_control sc = { .gfp_mask = GFP_KERNEL, }; @@ -119,7 +120,7 @@ static ssize_t shrinker_debugfs_scan_write(struct file *file, return -EFAULT; kbuf[read_len] = '\0'; - if (sscanf(kbuf, "%lu %d %lu", &ino, &nid, &nr_to_scan) != 3) + if (sscanf(kbuf, "%llu %d %lu", &ino, &nid, &nr_to_scan) != 3) return -EINVAL; if (nid < 0 || nid >= nr_node_ids) -- 2.47.3 DAMON was using the internal private memcg ID which is meant for tracking kernel objects that outlive their cgroup. Switch to using the public cgroup ID instead. Signed-off-by: Shakeel Butt --- include/linux/damon.h | 4 ++-- mm/damon/core.c | 7 ++----- mm/damon/ops-common.c | 2 +- mm/damon/sysfs-schemes.c | 8 ++++---- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/include/linux/damon.h b/include/linux/damon.h index a67292a2f09d..650e7ecfa32b 100644 --- a/include/linux/damon.h +++ b/include/linux/damon.h @@ -203,7 +203,7 @@ struct damos_quota_goal { u64 last_psi_total; struct { int nid; - unsigned short memcg_id; + u64 memcg_id; }; }; struct list_head list; @@ -419,7 +419,7 @@ struct damos_filter { bool matching; bool allow; union { - unsigned short memcg_id; + u64 memcg_id; struct damon_addr_range addr_range; int target_idx; struct damon_size_range sz_range; diff --git a/mm/damon/core.c b/mm/damon/core.c index 2d3e8006db50..23c44811ff7f 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -2065,16 +2065,13 @@ static unsigned long damos_get_node_memcg_used_bp( unsigned long used_pages, numerator; struct sysinfo i; - rcu_read_lock(); - memcg = mem_cgroup_from_id(goal->memcg_id); - if (!memcg || !mem_cgroup_tryget(memcg)) { - rcu_read_unlock(); + memcg = mem_cgroup_get_from_ino(goal->memcg_id); + if (!memcg) { if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP) return 0; else /* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */ return 10000; } - rcu_read_unlock(); mem_cgroup_flush_stats(memcg); lruvec = mem_cgroup_lruvec(memcg, NODE_DATA(goal->nid)); diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c index a218d9922234..dd81db95f901 100644 --- a/mm/damon/ops-common.c +++ b/mm/damon/ops-common.c @@ -274,7 +274,7 @@ bool damos_folio_filter_match(struct damos_filter *filter, struct folio *folio) if (!memcg) matched = false; else - matched = filter->memcg_id == mem_cgroup_id(memcg); + matched = filter->memcg_id == mem_cgroup_ino(memcg); rcu_read_unlock(); break; case DAMOS_FILTER_TYPE_YOUNG: diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c index e198234f0763..79aa917ab3c0 100644 --- a/mm/damon/sysfs-schemes.c +++ b/mm/damon/sysfs-schemes.c @@ -2492,7 +2492,7 @@ static bool damon_sysfs_memcg_path_eq(struct mem_cgroup *memcg, return false; } -static int damon_sysfs_memcg_path_to_id(char *memcg_path, unsigned short *id) +static int damon_sysfs_memcg_path_to_id(char *memcg_path, u64 *id) { struct mem_cgroup *memcg; char *path; @@ -2507,11 +2507,11 @@ static int damon_sysfs_memcg_path_to_id(char *memcg_path, unsigned short *id) for (memcg = mem_cgroup_iter(NULL, NULL, NULL); memcg; memcg = mem_cgroup_iter(NULL, memcg, NULL)) { - /* skip removed memcg */ - if (!mem_cgroup_id(memcg)) + /* skip offlined memcg */ + if (!mem_cgroup_online(memcg)) continue; if (damon_sysfs_memcg_path_eq(memcg, path, memcg_path)) { - *id = mem_cgroup_id(memcg); + *id = mem_cgroup_ino(memcg); found = true; break; } -- 2.47.3 The LRU gen debugfs interface was using the internal private memcg ID which is meant for tracking kernel objects that outlive their cgroup. Switch to using the public cgroup ID instead. Signed-off-by: Shakeel Butt --- mm/vmscan.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/mm/vmscan.c b/mm/vmscan.c index a4b308a2f9ad..d78043c7e4af 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -5426,7 +5426,7 @@ static int lru_gen_seq_show(struct seq_file *m, void *v) if (memcg) cgroup_path(memcg->css.cgroup, m->private, PATH_MAX); #endif - seq_printf(m, "memcg %5hu %s\n", mem_cgroup_id(memcg), path); + seq_printf(m, "memcg %llu %s\n", mem_cgroup_ino(memcg), path); } seq_printf(m, " node %5d\n", nid); @@ -5511,7 +5511,7 @@ static int run_eviction(struct lruvec *lruvec, unsigned long seq, struct scan_co return -EINTR; } -static int run_cmd(char cmd, int memcg_id, int nid, unsigned long seq, +static int run_cmd(char cmd, u64 memcg_id, int nid, unsigned long seq, struct scan_control *sc, int swappiness, unsigned long opt) { struct lruvec *lruvec; @@ -5522,19 +5522,12 @@ static int run_cmd(char cmd, int memcg_id, int nid, unsigned long seq, return -EINVAL; if (!mem_cgroup_disabled()) { - rcu_read_lock(); - - memcg = mem_cgroup_from_id(memcg_id); - if (!mem_cgroup_tryget(memcg)) - memcg = NULL; - - rcu_read_unlock(); - + memcg = mem_cgroup_get_from_ino(memcg_id); if (!memcg) return -EINVAL; } - if (memcg_id != mem_cgroup_id(memcg)) + if (memcg_id != mem_cgroup_ino(memcg)) goto done; sc->target_mem_cgroup = memcg; @@ -5601,7 +5594,7 @@ static ssize_t lru_gen_seq_write(struct file *file, const char __user *src, int n; int end; char cmd, swap_string[5]; - unsigned int memcg_id; + u64 memcg_id; unsigned int nid; unsigned long seq; unsigned int swappiness; @@ -5611,7 +5604,7 @@ static ssize_t lru_gen_seq_write(struct file *file, const char __user *src, if (!*cur) continue; - n = sscanf(cur, "%c %u %u %lu %n %4s %n %lu %n", &cmd, &memcg_id, &nid, + n = sscanf(cur, "%c %llu %u %lu %n %4s %n %lu %n", &cmd, &memcg_id, &nid, &seq, &end, swap_string, &end, &opt, &end); if (n < 4 || cur[end]) { err = -EINVAL; -- 2.47.3 Now that all callers have been converted to use either: - The private ID APIs (mem_cgroup_private_id/mem_cgroup_from_private_id) for internal kernel objects that outlive their cgroup - The public cgroup ID APIs (mem_cgroup_ino/mem_cgroup_get_from_ino) for external interfaces Remove the unused wrapper functions mem_cgroup_id() and mem_cgroup_from_id() along with their !CONFIG_MEMCG stubs. Signed-off-by: Shakeel Butt --- include/linux/memcontrol.h | 18 ------------------ mm/memcontrol.c | 5 ----- 2 files changed, 23 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index c823150ec288..3e7d69020b39 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -830,12 +830,6 @@ static inline unsigned short mem_cgroup_private_id(struct mem_cgroup *memcg) } struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id); -static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg) -{ - return mem_cgroup_private_id(memcg); -} -struct mem_cgroup *mem_cgroup_from_id(unsigned short id); - static inline u64 mem_cgroup_ino(struct mem_cgroup *memcg) { return memcg ? cgroup_id(memcg->css.cgroup) : 0; @@ -1282,18 +1276,6 @@ static inline void mem_cgroup_scan_tasks(struct mem_cgroup *memcg, { } -static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg) -{ - return 0; -} - -static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id) -{ - WARN_ON_ONCE(id); - /* XXX: This should always return root_mem_cgroup */ - return NULL; -} - static inline unsigned short mem_cgroup_private_id(struct mem_cgroup *memcg) { return 0; diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 1ff2f9bd820c..ede39dde05df 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3615,11 +3615,6 @@ struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id) return xa_load(&mem_cgroup_private_ids, id); } -struct mem_cgroup *mem_cgroup_from_id(unsigned short id) -{ - return mem_cgroup_from_private_id(id); -} - struct mem_cgroup *mem_cgroup_get_from_ino(u64 ino) { struct cgroup *cgrp; -- 2.47.3 Rename mem_cgroup_ino() to mem_cgroup_id() and mem_cgroup_get_from_ino() to mem_cgroup_get_from_id(). These functions now use cgroup IDs (from cgroup_id()) rather than inode numbers, so the names should reflect that. Signed-off-by: Shakeel Butt --- include/linux/memcontrol.h | 8 ++++---- mm/damon/core.c | 2 +- mm/damon/ops-common.c | 2 +- mm/damon/sysfs-schemes.c | 2 +- mm/memcontrol.c | 2 +- mm/shrinker_debug.c | 10 +++++----- mm/vmscan.c | 6 +++--- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 3e7d69020b39..5a1161cadb8d 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -830,12 +830,12 @@ static inline unsigned short mem_cgroup_private_id(struct mem_cgroup *memcg) } struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id); -static inline u64 mem_cgroup_ino(struct mem_cgroup *memcg) +static inline u64 mem_cgroup_id(struct mem_cgroup *memcg) { return memcg ? cgroup_id(memcg->css.cgroup) : 0; } -struct mem_cgroup *mem_cgroup_get_from_ino(u64 ino); +struct mem_cgroup *mem_cgroup_get_from_id(u64 ino); static inline struct mem_cgroup *mem_cgroup_from_seq(struct seq_file *m) { @@ -1288,12 +1288,12 @@ static inline struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id) return NULL; } -static inline u64 mem_cgroup_ino(struct mem_cgroup *memcg) +static inline u64 mem_cgroup_id(struct mem_cgroup *memcg) { return 0; } -static inline struct mem_cgroup *mem_cgroup_get_from_ino(u64 ino) +static inline struct mem_cgroup *mem_cgroup_get_from_id(u64 ino) { return NULL; } diff --git a/mm/damon/core.c b/mm/damon/core.c index 23c44811ff7f..a2513db59aee 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -2065,7 +2065,7 @@ static unsigned long damos_get_node_memcg_used_bp( unsigned long used_pages, numerator; struct sysinfo i; - memcg = mem_cgroup_get_from_ino(goal->memcg_id); + memcg = mem_cgroup_get_from_id(goal->memcg_id); if (!memcg) { if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP) return 0; diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c index dd81db95f901..a218d9922234 100644 --- a/mm/damon/ops-common.c +++ b/mm/damon/ops-common.c @@ -274,7 +274,7 @@ bool damos_folio_filter_match(struct damos_filter *filter, struct folio *folio) if (!memcg) matched = false; else - matched = filter->memcg_id == mem_cgroup_ino(memcg); + matched = filter->memcg_id == mem_cgroup_id(memcg); rcu_read_unlock(); break; case DAMOS_FILTER_TYPE_YOUNG: diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c index 79aa917ab3c0..3beb4456aa51 100644 --- a/mm/damon/sysfs-schemes.c +++ b/mm/damon/sysfs-schemes.c @@ -2511,7 +2511,7 @@ static int damon_sysfs_memcg_path_to_id(char *memcg_path, u64 *id) if (!mem_cgroup_online(memcg)) continue; if (damon_sysfs_memcg_path_eq(memcg, path, memcg_path)) { - *id = mem_cgroup_ino(memcg); + *id = mem_cgroup_id(memcg); found = true; break; } diff --git a/mm/memcontrol.c b/mm/memcontrol.c index ede39dde05df..5b2925a68832 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3615,7 +3615,7 @@ struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id) return xa_load(&mem_cgroup_private_ids, id); } -struct mem_cgroup *mem_cgroup_get_from_ino(u64 ino) +struct mem_cgroup *mem_cgroup_get_from_id(u64 ino) { struct cgroup *cgrp; struct cgroup_subsys_state *css; diff --git a/mm/shrinker_debug.c b/mm/shrinker_debug.c index 7ef16a0b2959..affa64437302 100644 --- a/mm/shrinker_debug.c +++ b/mm/shrinker_debug.c @@ -70,7 +70,7 @@ static int shrinker_debugfs_count_show(struct seq_file *m, void *v) memcg_aware ? memcg : NULL, count_per_node); if (total) { - seq_printf(m, "%llu", mem_cgroup_ino(memcg)); + seq_printf(m, "%llu", mem_cgroup_id(memcg)); for_each_node(nid) seq_printf(m, " %lu", count_per_node[nid]); seq_putc(m, '\n'); @@ -107,7 +107,7 @@ static ssize_t shrinker_debugfs_scan_write(struct file *file, { struct shrinker *shrinker = file->private_data; unsigned long nr_to_scan = 0, read_len; - u64 ino; + u64 id; struct shrink_control sc = { .gfp_mask = GFP_KERNEL, }; @@ -120,7 +120,7 @@ static ssize_t shrinker_debugfs_scan_write(struct file *file, return -EFAULT; kbuf[read_len] = '\0'; - if (sscanf(kbuf, "%llu %d %lu", &ino, &nid, &nr_to_scan) != 3) + if (sscanf(kbuf, "%llu %d %lu", &id, &nid, &nr_to_scan) != 3) return -EINVAL; if (nid < 0 || nid >= nr_node_ids) @@ -130,7 +130,7 @@ static ssize_t shrinker_debugfs_scan_write(struct file *file, return size; if (shrinker->flags & SHRINKER_MEMCG_AWARE) { - memcg = mem_cgroup_get_from_ino(ino); + memcg = mem_cgroup_get_from_id(id); if (!memcg) return -ENOENT; @@ -138,7 +138,7 @@ static ssize_t shrinker_debugfs_scan_write(struct file *file, mem_cgroup_put(memcg); return -ENOENT; } - } else if (ino != 0) { + } else if (id != 0) { return -EINVAL; } diff --git a/mm/vmscan.c b/mm/vmscan.c index d78043c7e4af..9ad2c2f06bfa 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -5426,7 +5426,7 @@ static int lru_gen_seq_show(struct seq_file *m, void *v) if (memcg) cgroup_path(memcg->css.cgroup, m->private, PATH_MAX); #endif - seq_printf(m, "memcg %llu %s\n", mem_cgroup_ino(memcg), path); + seq_printf(m, "memcg %llu %s\n", mem_cgroup_id(memcg), path); } seq_printf(m, " node %5d\n", nid); @@ -5522,12 +5522,12 @@ static int run_cmd(char cmd, u64 memcg_id, int nid, unsigned long seq, return -EINVAL; if (!mem_cgroup_disabled()) { - memcg = mem_cgroup_get_from_ino(memcg_id); + memcg = mem_cgroup_get_from_id(memcg_id); if (!memcg) return -EINVAL; } - if (memcg_id != mem_cgroup_ino(memcg)) + if (memcg_id != mem_cgroup_id(memcg)) goto done; sc->target_mem_cgroup = memcg; -- 2.47.3