| Message ID | 20210301062227.59292-5-songmuchun@bytedance.com |
|---|---|
| State | New |
| Headers | show |
| Series | Use obj_cgroup APIs to change kmem pages | expand |
On Mon, Mar 01, 2021 at 02:22:26PM +0800, Muchun Song wrote: > The remote memcg charing APIs is a mechanism to charge kernel memory > to a given memcg. So we can move the infrastructure to the scope of > the CONFIG_MEMCG_KMEM. This is not a good idea, because there is nothing kmem-specific in the idea of remote charging, and we definitely will see cases when user memory is charged to the process different from the current. > > As a bonus, on !CONFIG_MEMCG_KMEM build some functions and variables > can be compiled out. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > --- > include/linux/sched.h | 2 ++ > include/linux/sched/mm.h | 2 +- > kernel/fork.c | 2 +- > mm/memcontrol.c | 4 ++++ > 4 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/include/linux/sched.h b/include/linux/sched.h > index ee46f5cab95b..c2d488eddf85 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1314,7 +1314,9 @@ struct task_struct { > > /* Number of pages to reclaim on returning to userland: */ > unsigned int memcg_nr_pages_over_high; > +#endif > > +#ifdef CONFIG_MEMCG_KMEM > /* Used by memcontrol for targeted memcg charge: */ > struct mem_cgroup *active_memcg; > #endif > diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h > index 1ae08b8462a4..64a72975270e 100644 > --- a/include/linux/sched/mm.h > +++ b/include/linux/sched/mm.h > @@ -294,7 +294,7 @@ static inline void memalloc_nocma_restore(unsigned int flags) > } > #endif > > -#ifdef CONFIG_MEMCG > +#ifdef CONFIG_MEMCG_KMEM > DECLARE_PER_CPU(struct mem_cgroup *, int_active_memcg); > /** > * set_active_memcg - Starts the remote memcg charging scope. > diff --git a/kernel/fork.c b/kernel/fork.c > index d66cd1014211..d66718bc82d5 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -942,7 +942,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node) > tsk->use_memdelay = 0; > #endif > > -#ifdef CONFIG_MEMCG > +#ifdef CONFIG_MEMCG_KMEM > tsk->active_memcg = NULL; > #endif > return tsk; > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 39cb8c5bf8b2..092dc4588b43 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -76,8 +76,10 @@ EXPORT_SYMBOL(memory_cgrp_subsys); > > struct mem_cgroup *root_mem_cgroup __read_mostly; > > +#ifdef CONFIG_MEMCG_KMEM > /* Active memory cgroup to use from an interrupt context */ > DEFINE_PER_CPU(struct mem_cgroup *, int_active_memcg); > +#endif > > /* Socket memory accounting disabled? */ > static bool cgroup_memory_nosocket; > @@ -1054,6 +1056,7 @@ struct mem_cgroup *get_mem_cgroup_from_mm(struct mm_struct *mm) > } > EXPORT_SYMBOL(get_mem_cgroup_from_mm); > > +#ifdef CONFIG_MEMCG_KMEM > static __always_inline struct mem_cgroup *active_memcg(void) > { > if (in_interrupt()) > @@ -1074,6 +1077,7 @@ static __always_inline bool memcg_kmem_bypass(void) > > return false; > } > +#endif > > /** > * mem_cgroup_iter - iterate over memory cgroup hierarchy > -- > 2.11.0 >
On Mon, Mar 1, 2021 at 5:16 PM Roman Gushchin <guro@fb.com> wrote: > > On Mon, Mar 01, 2021 at 02:22:26PM +0800, Muchun Song wrote: > > The remote memcg charing APIs is a mechanism to charge kernel memory > > to a given memcg. So we can move the infrastructure to the scope of > > the CONFIG_MEMCG_KMEM. > > This is not a good idea, because there is nothing kmem-specific > in the idea of remote charging, and we definitely will see cases > when user memory is charged to the process different from the current. > Indeed and which remind me: what happened to the "Charge loop device i/o to issuing cgroup" series? That series was doing remote charging for user pages.
On Mon, Mar 01, 2021 at 07:43:27PM -0800, Shakeel Butt wrote: > On Mon, Mar 1, 2021 at 5:16 PM Roman Gushchin <guro@fb.com> wrote: > > > > On Mon, Mar 01, 2021 at 02:22:26PM +0800, Muchun Song wrote: > > > The remote memcg charing APIs is a mechanism to charge kernel memory > > > to a given memcg. So we can move the infrastructure to the scope of > > > the CONFIG_MEMCG_KMEM. > > > > This is not a good idea, because there is nothing kmem-specific > > in the idea of remote charging, and we definitely will see cases > > when user memory is charged to the process different from the current. > > > > Indeed and which remind me: what happened to the "Charge loop device > i/o to issuing cgroup" series? That series was doing remote charging > for user pages. Yeah, this is exactly what I minded. We're using it internally, and as I remember there were no obstacles to upstream it too. I'll ping Dan when after the merge window. Thanks!
On Tue, Mar 2, 2021 at 9:15 AM Roman Gushchin <guro@fb.com> wrote: > > On Mon, Mar 01, 2021 at 02:22:26PM +0800, Muchun Song wrote: > > The remote memcg charing APIs is a mechanism to charge kernel memory > > to a given memcg. So we can move the infrastructure to the scope of > > the CONFIG_MEMCG_KMEM. > > This is not a good idea, because there is nothing kmem-specific > in the idea of remote charging, and we definitely will see cases > when user memory is charged to the process different from the current. Got it. Thanks for your reminder. > > > > > As a bonus, on !CONFIG_MEMCG_KMEM build some functions and variables > > can be compiled out. > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > --- > > include/linux/sched.h | 2 ++ > > include/linux/sched/mm.h | 2 +- > > kernel/fork.c | 2 +- > > mm/memcontrol.c | 4 ++++ > > 4 files changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/sched.h b/include/linux/sched.h > > index ee46f5cab95b..c2d488eddf85 100644 > > --- a/include/linux/sched.h > > +++ b/include/linux/sched.h > > @@ -1314,7 +1314,9 @@ struct task_struct { > > > > /* Number of pages to reclaim on returning to userland: */ > > unsigned int memcg_nr_pages_over_high; > > +#endif > > > > +#ifdef CONFIG_MEMCG_KMEM > > /* Used by memcontrol for targeted memcg charge: */ > > struct mem_cgroup *active_memcg; > > #endif > > diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h > > index 1ae08b8462a4..64a72975270e 100644 > > --- a/include/linux/sched/mm.h > > +++ b/include/linux/sched/mm.h > > @@ -294,7 +294,7 @@ static inline void memalloc_nocma_restore(unsigned int flags) > > } > > #endif > > > > -#ifdef CONFIG_MEMCG > > +#ifdef CONFIG_MEMCG_KMEM > > DECLARE_PER_CPU(struct mem_cgroup *, int_active_memcg); > > /** > > * set_active_memcg - Starts the remote memcg charging scope. > > diff --git a/kernel/fork.c b/kernel/fork.c > > index d66cd1014211..d66718bc82d5 100644 > > --- a/kernel/fork.c > > +++ b/kernel/fork.c > > @@ -942,7 +942,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node) > > tsk->use_memdelay = 0; > > #endif > > > > -#ifdef CONFIG_MEMCG > > +#ifdef CONFIG_MEMCG_KMEM > > tsk->active_memcg = NULL; > > #endif > > return tsk; > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 39cb8c5bf8b2..092dc4588b43 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -76,8 +76,10 @@ EXPORT_SYMBOL(memory_cgrp_subsys); > > > > struct mem_cgroup *root_mem_cgroup __read_mostly; > > > > +#ifdef CONFIG_MEMCG_KMEM > > /* Active memory cgroup to use from an interrupt context */ > > DEFINE_PER_CPU(struct mem_cgroup *, int_active_memcg); > > +#endif > > > > /* Socket memory accounting disabled? */ > > static bool cgroup_memory_nosocket; > > @@ -1054,6 +1056,7 @@ struct mem_cgroup *get_mem_cgroup_from_mm(struct mm_struct *mm) > > } > > EXPORT_SYMBOL(get_mem_cgroup_from_mm); > > > > +#ifdef CONFIG_MEMCG_KMEM > > static __always_inline struct mem_cgroup *active_memcg(void) > > { > > if (in_interrupt()) > > @@ -1074,6 +1077,7 @@ static __always_inline bool memcg_kmem_bypass(void) > > > > return false; > > } > > +#endif > > > > /** > > * mem_cgroup_iter - iterate over memory cgroup hierarchy > > -- > > 2.11.0 > >
diff --git a/include/linux/sched.h b/include/linux/sched.h index ee46f5cab95b..c2d488eddf85 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1314,7 +1314,9 @@ struct task_struct { /* Number of pages to reclaim on returning to userland: */ unsigned int memcg_nr_pages_over_high; +#endif +#ifdef CONFIG_MEMCG_KMEM /* Used by memcontrol for targeted memcg charge: */ struct mem_cgroup *active_memcg; #endif diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h index 1ae08b8462a4..64a72975270e 100644 --- a/include/linux/sched/mm.h +++ b/include/linux/sched/mm.h @@ -294,7 +294,7 @@ static inline void memalloc_nocma_restore(unsigned int flags) } #endif -#ifdef CONFIG_MEMCG +#ifdef CONFIG_MEMCG_KMEM DECLARE_PER_CPU(struct mem_cgroup *, int_active_memcg); /** * set_active_memcg - Starts the remote memcg charging scope. diff --git a/kernel/fork.c b/kernel/fork.c index d66cd1014211..d66718bc82d5 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -942,7 +942,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node) tsk->use_memdelay = 0; #endif -#ifdef CONFIG_MEMCG +#ifdef CONFIG_MEMCG_KMEM tsk->active_memcg = NULL; #endif return tsk; diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 39cb8c5bf8b2..092dc4588b43 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -76,8 +76,10 @@ EXPORT_SYMBOL(memory_cgrp_subsys); struct mem_cgroup *root_mem_cgroup __read_mostly; +#ifdef CONFIG_MEMCG_KMEM /* Active memory cgroup to use from an interrupt context */ DEFINE_PER_CPU(struct mem_cgroup *, int_active_memcg); +#endif /* Socket memory accounting disabled? */ static bool cgroup_memory_nosocket; @@ -1054,6 +1056,7 @@ struct mem_cgroup *get_mem_cgroup_from_mm(struct mm_struct *mm) } EXPORT_SYMBOL(get_mem_cgroup_from_mm); +#ifdef CONFIG_MEMCG_KMEM static __always_inline struct mem_cgroup *active_memcg(void) { if (in_interrupt()) @@ -1074,6 +1077,7 @@ static __always_inline bool memcg_kmem_bypass(void) return false; } +#endif /** * mem_cgroup_iter - iterate over memory cgroup hierarchy
The remote memcg charing APIs is a mechanism to charge kernel memory to a given memcg. So we can move the infrastructure to the scope of the CONFIG_MEMCG_KMEM. As a bonus, on !CONFIG_MEMCG_KMEM build some functions and variables can be compiled out. Signed-off-by: Muchun Song <songmuchun@bytedance.com> --- include/linux/sched.h | 2 ++ include/linux/sched/mm.h | 2 +- kernel/fork.c | 2 +- mm/memcontrol.c | 4 ++++ 4 files changed, 8 insertions(+), 2 deletions(-)