| Message ID | 20220509183820.573666-2-roman.gushchin@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: introduce shrinker debugfs interface | expand |
On Mon, May 09, 2022 at 11:38:15AM -0700, Roman Gushchin wrote: > Shrinker debugfs requires a way to represent memory cgroups without > using full paths, both for displaying information and getting input > from a user. > > Cgroup inode number is a perfect way, already used by bpf. > > This commit adds a couple of helper functions which will be used > to handle memcg-aware shrinkers. > > Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev> > --- > include/linux/memcontrol.h | 21 +++++++++++++++++++++ > mm/memcontrol.c | 23 +++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index fe580cb96683..a6de9e5c1549 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -831,6 +831,15 @@ 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) > { > return mem_cgroup_from_css(seq_css(m)); > @@ -1324,6 +1333,18 @@ static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id) > return NULL; > } > > +#ifdef CONFIG_SHRINKER_DEBUG > +static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg) > +{ > + return 0; > +} > + > +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) > { > return NULL; > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 04cea4fa362a..e6472728fa66 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -5018,6 +5018,29 @@ struct mem_cgroup *mem_cgroup_from_id(unsigned short id) > return idr_find(&mem_cgroup_idr, id); > } > > +#ifdef CONFIG_SHRINKER_DEBUG > +struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) > +{ > + struct cgroup *cgrp; > + struct cgroup_subsys_state *css; > + struct mem_cgroup *memcg; > + > + cgrp = cgroup_get_from_id(ino); > + if (!cgrp) > + return ERR_PTR(-ENOENT); > + > + 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); I think it's better to use css_put() here since the refcount is get via cgroup_get_e_css() which returns a css struct. Thanks. > + > + return memcg; > +} > +#endif > + > static int alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node) > { > struct mem_cgroup_per_node *pn; > -- > 2.35.3 > >
On Sun, May 22, 2022 at 03:05:33PM +0800, Muchun Song wrote: > On Mon, May 09, 2022 at 11:38:15AM -0700, Roman Gushchin wrote: > > Shrinker debugfs requires a way to represent memory cgroups without > > using full paths, both for displaying information and getting input > > from a user. > > > > Cgroup inode number is a perfect way, already used by bpf. > > > > This commit adds a couple of helper functions which will be used > > to handle memcg-aware shrinkers. > > > > Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev> > > --- > > include/linux/memcontrol.h | 21 +++++++++++++++++++++ > > mm/memcontrol.c | 23 +++++++++++++++++++++++ > > 2 files changed, 44 insertions(+) > > > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > > index fe580cb96683..a6de9e5c1549 100644 > > --- a/include/linux/memcontrol.h > > +++ b/include/linux/memcontrol.h > > @@ -831,6 +831,15 @@ 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) > > { > > return mem_cgroup_from_css(seq_css(m)); > > @@ -1324,6 +1333,18 @@ static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id) > > return NULL; > > } > > > > +#ifdef CONFIG_SHRINKER_DEBUG > > +static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg) > > +{ > > + return 0; > > +} > > + > > +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) > > { > > return NULL; > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 04cea4fa362a..e6472728fa66 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -5018,6 +5018,29 @@ struct mem_cgroup *mem_cgroup_from_id(unsigned short id) > > return idr_find(&mem_cgroup_idr, id); > > } > > > > +#ifdef CONFIG_SHRINKER_DEBUG > > +struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) > > +{ > > + struct cgroup *cgrp; > > + struct cgroup_subsys_state *css; > > + struct mem_cgroup *memcg; > > + > > + cgrp = cgroup_get_from_id(ino); > > + if (!cgrp) > > + return ERR_PTR(-ENOENT); > > + > > + 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); > > I think it's better to use css_put() here since the refcount is get > via cgroup_get_e_css() which returns a css struct. cgroup_put() is matching cgroup_get_from_id(). The reference grabbed by cgroup_get_e_css() shouldn't be dropped because mem_cgroup_get_from_ino() has a "get" semantics. Thanks!
On Mon, May 23, 2022 at 11:12:12AM -0700, Roman Gushchin wrote: > On Sun, May 22, 2022 at 03:05:33PM +0800, Muchun Song wrote: > > On Mon, May 09, 2022 at 11:38:15AM -0700, Roman Gushchin wrote: > > > Shrinker debugfs requires a way to represent memory cgroups without > > > using full paths, both for displaying information and getting input > > > from a user. > > > > > > Cgroup inode number is a perfect way, already used by bpf. > > > > > > This commit adds a couple of helper functions which will be used > > > to handle memcg-aware shrinkers. > > > > > > Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev> > > > --- > > > include/linux/memcontrol.h | 21 +++++++++++++++++++++ > > > mm/memcontrol.c | 23 +++++++++++++++++++++++ > > > 2 files changed, 44 insertions(+) > > > > > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > > > index fe580cb96683..a6de9e5c1549 100644 > > > --- a/include/linux/memcontrol.h > > > +++ b/include/linux/memcontrol.h > > > @@ -831,6 +831,15 @@ 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) > > > { > > > return mem_cgroup_from_css(seq_css(m)); > > > @@ -1324,6 +1333,18 @@ static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id) > > > return NULL; > > > } > > > > > > +#ifdef CONFIG_SHRINKER_DEBUG > > > +static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg) > > > +{ > > > + return 0; > > > +} > > > + > > > +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) > > > { > > > return NULL; > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > > index 04cea4fa362a..e6472728fa66 100644 > > > --- a/mm/memcontrol.c > > > +++ b/mm/memcontrol.c > > > @@ -5018,6 +5018,29 @@ struct mem_cgroup *mem_cgroup_from_id(unsigned short id) > > > return idr_find(&mem_cgroup_idr, id); > > > } > > > > > > +#ifdef CONFIG_SHRINKER_DEBUG > > > +struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) > > > +{ > > > + struct cgroup *cgrp; > > > + struct cgroup_subsys_state *css; > > > + struct mem_cgroup *memcg; > > > + > > > + cgrp = cgroup_get_from_id(ino); > > > + if (!cgrp) > > > + return ERR_PTR(-ENOENT); > > > + > > > + 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); > > > > I think it's better to use css_put() here since the refcount is get > > via cgroup_get_e_css() which returns a css struct. > > cgroup_put() is matching cgroup_get_from_id(). > > The reference grabbed by cgroup_get_e_css() shouldn't be dropped > because mem_cgroup_get_from_ino() has a "get" semantics. > My bad. I have misread it here. Thanks.
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index fe580cb96683..a6de9e5c1549 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -831,6 +831,15 @@ 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) { return mem_cgroup_from_css(seq_css(m)); @@ -1324,6 +1333,18 @@ static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id) return NULL; } +#ifdef CONFIG_SHRINKER_DEBUG +static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg) +{ + return 0; +} + +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) { return NULL; diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 04cea4fa362a..e6472728fa66 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5018,6 +5018,29 @@ struct mem_cgroup *mem_cgroup_from_id(unsigned short id) return idr_find(&mem_cgroup_idr, id); } +#ifdef CONFIG_SHRINKER_DEBUG +struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) +{ + struct cgroup *cgrp; + struct cgroup_subsys_state *css; + struct mem_cgroup *memcg; + + cgrp = cgroup_get_from_id(ino); + if (!cgrp) + return ERR_PTR(-ENOENT); + + 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); + + return memcg; +} +#endif + static int alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node) { struct mem_cgroup_per_node *pn;
Shrinker debugfs requires a way to represent memory cgroups without using full paths, both for displaying information and getting input from a user. Cgroup inode number is a perfect way, already used by bpf. This commit adds a couple of helper functions which will be used to handle memcg-aware shrinkers. Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev> --- include/linux/memcontrol.h | 21 +++++++++++++++++++++ mm/memcontrol.c | 23 +++++++++++++++++++++++ 2 files changed, 44 insertions(+)