* [PATCH] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 7:28 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-13 7:28 UTC (permalink / raw) To: Johannes Weiner, Michal Hocko, Vladimir Davydov Cc: Cgroups, linux-mm, Andrew Morton While trying to use remote memcg charging in an out-of-tree kernel module I found it's not working, because the current thread is a workqueue thread. Signed-off-by: Zefan Li <lizefan@huawei.com> --- No need to queue this for v5.7 as currently no upstream users of this memcg feature suffer from this bug. --- mm/memcontrol.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index a3b97f1..db836fc 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2802,6 +2802,8 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, static inline bool memcg_kmem_bypass(void) { + if (unlikely(current->active_memcg)) + return false; if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) return true; return false; -- 2.7.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 7:28 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-13 7:28 UTC (permalink / raw) To: Johannes Weiner, Michal Hocko, Vladimir Davydov Cc: Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton While trying to use remote memcg charging in an out-of-tree kernel module I found it's not working, because the current thread is a workqueue thread. Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> --- No need to queue this for v5.7 as currently no upstream users of this memcg feature suffer from this bug. --- mm/memcontrol.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index a3b97f1..db836fc 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2802,6 +2802,8 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, static inline bool memcg_kmem_bypass(void) { + if (unlikely(current->active_memcg)) + return false; if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) return true; return false; -- 2.7.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 9:05 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-13 9:05 UTC (permalink / raw) To: Zefan Li Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton On Wed 13-05-20 15:28:28, Li Zefan wrote: > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > Signed-off-by: Zefan Li <lizefan@huawei.com> > --- > > No need to queue this for v5.7 as currently no upstream users of this memcg > feature suffer from this bug. > > --- > mm/memcontrol.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a3b97f1..db836fc 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2802,6 +2802,8 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > static inline bool memcg_kmem_bypass(void) > { > + if (unlikely(current->active_memcg)) > + return false; I am confused. Why the check below is insufficient? It checks for both mm and PF_KTHREAD? > if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > return true; > return false; > -- > 2.7.4 -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 9:05 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-13 9:05 UTC (permalink / raw) To: Zefan Li Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton On Wed 13-05-20 15:28:28, Li Zefan wrote: > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> > --- > > No need to queue this for v5.7 as currently no upstream users of this memcg > feature suffer from this bug. > > --- > mm/memcontrol.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a3b97f1..db836fc 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2802,6 +2802,8 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > static inline bool memcg_kmem_bypass(void) > { > + if (unlikely(current->active_memcg)) > + return false; I am confused. Why the check below is insufficient? It checks for both mm and PF_KTHREAD? > if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > return true; > return false; > -- > 2.7.4 -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 11:19 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-13 11:19 UTC (permalink / raw) To: Michal Hocko Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton On 2020/5/13 17:05, Michal Hocko wrote: > On Wed 13-05-20 15:28:28, Li Zefan wrote: >> While trying to use remote memcg charging in an out-of-tree kernel module >> I found it's not working, because the current thread is a workqueue thread. >> >> Signed-off-by: Zefan Li <lizefan@huawei.com> >> --- >> >> No need to queue this for v5.7 as currently no upstream users of this memcg >> feature suffer from this bug. >> >> --- >> mm/memcontrol.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index a3b97f1..db836fc 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -2802,6 +2802,8 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, >> >> static inline bool memcg_kmem_bypass(void) >> { >> + if (unlikely(current->active_memcg)) >> + return false; > > I am confused. Why the check below is insufficient? It checks for both mm > and PF_KTHREAD? > memalloc_use_memcg(memcg); alloc_page(GFP_KERNEL_ACCOUNT); memalloc_unuse_memcg(); If we run above code in a workqueue thread the memory won't be charged to the specific memcg, because memcg_kmem_bypass() returns true in this case. >> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) >> return true; >> return false; ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 11:19 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-13 11:19 UTC (permalink / raw) To: Michal Hocko Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton On 2020/5/13 17:05, Michal Hocko wrote: > On Wed 13-05-20 15:28:28, Li Zefan wrote: >> While trying to use remote memcg charging in an out-of-tree kernel module >> I found it's not working, because the current thread is a workqueue thread. >> >> Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> >> --- >> >> No need to queue this for v5.7 as currently no upstream users of this memcg >> feature suffer from this bug. >> >> --- >> mm/memcontrol.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index a3b97f1..db836fc 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -2802,6 +2802,8 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, >> >> static inline bool memcg_kmem_bypass(void) >> { >> + if (unlikely(current->active_memcg)) >> + return false; > > I am confused. Why the check below is insufficient? It checks for both mm > and PF_KTHREAD? > memalloc_use_memcg(memcg); alloc_page(GFP_KERNEL_ACCOUNT); memalloc_unuse_memcg(); If we run above code in a workqueue thread the memory won't be charged to the specific memcg, because memcg_kmem_bypass() returns true in this case. >> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) >> return true; >> return false; ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 11:29 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-13 11:29 UTC (permalink / raw) To: Zefan Li Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton [Cc Roman - initial email is http://lkml.kernel.org/r/e6927a82-949c-bdfd-d717-0a14743c6759@huawei.com] On Wed 13-05-20 19:19:56, Li Zefan wrote: > On 2020/5/13 17:05, Michal Hocko wrote: > > On Wed 13-05-20 15:28:28, Li Zefan wrote: > >> While trying to use remote memcg charging in an out-of-tree kernel module > >> I found it's not working, because the current thread is a workqueue thread. > >> > >> Signed-off-by: Zefan Li <lizefan@huawei.com> > >> --- > >> > >> No need to queue this for v5.7 as currently no upstream users of this memcg > >> feature suffer from this bug. > >> > >> --- > >> mm/memcontrol.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c > >> index a3b97f1..db836fc 100644 > >> --- a/mm/memcontrol.c > >> +++ b/mm/memcontrol.c > >> @@ -2802,6 +2802,8 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > >> > >> static inline bool memcg_kmem_bypass(void) > >> { > >> + if (unlikely(current->active_memcg)) > >> + return false; > > > > I am confused. Why the check below is insufficient? It checks for both mm > > and PF_KTHREAD? > > > > memalloc_use_memcg(memcg); > alloc_page(GFP_KERNEL_ACCOUNT); > memalloc_unuse_memcg(); > > If we run above code in a workqueue thread the memory won't be charged to the specific > memcg, because memcg_kmem_bypass() returns true in this case. Ohh, right I have misread your patch. Sorry about that. A comment for the above branch would make it more clear. Something like /* Allow memalloc_use_memcg usage from kthread contexts */ On the other hand adding a code for an out of tree code is usually not welcome. But in this particular case the branch is correct for the existing code already so I am OK with it. Roman is de-facto kmem implementation maintainer so I will defer to him. > >> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > >> return true; > >> return false; -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 11:29 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-13 11:29 UTC (permalink / raw) To: Zefan Li Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton [Cc Roman - initial email is http://lkml.kernel.org/r/e6927a82-949c-bdfd-d717-0a14743c6759-hv44wF8Li93QT0dZR+AlfA@public.gmane.org] On Wed 13-05-20 19:19:56, Li Zefan wrote: > On 2020/5/13 17:05, Michal Hocko wrote: > > On Wed 13-05-20 15:28:28, Li Zefan wrote: > >> While trying to use remote memcg charging in an out-of-tree kernel module > >> I found it's not working, because the current thread is a workqueue thread. > >> > >> Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> > >> --- > >> > >> No need to queue this for v5.7 as currently no upstream users of this memcg > >> feature suffer from this bug. > >> > >> --- > >> mm/memcontrol.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c > >> index a3b97f1..db836fc 100644 > >> --- a/mm/memcontrol.c > >> +++ b/mm/memcontrol.c > >> @@ -2802,6 +2802,8 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > >> > >> static inline bool memcg_kmem_bypass(void) > >> { > >> + if (unlikely(current->active_memcg)) > >> + return false; > > > > I am confused. Why the check below is insufficient? It checks for both mm > > and PF_KTHREAD? > > > > memalloc_use_memcg(memcg); > alloc_page(GFP_KERNEL_ACCOUNT); > memalloc_unuse_memcg(); > > If we run above code in a workqueue thread the memory won't be charged to the specific > memcg, because memcg_kmem_bypass() returns true in this case. Ohh, right I have misread your patch. Sorry about that. A comment for the above branch would make it more clear. Something like /* Allow memalloc_use_memcg usage from kthread contexts */ On the other hand adding a code for an out of tree code is usually not welcome. But in this particular case the branch is correct for the existing code already so I am OK with it. Roman is de-facto kmem implementation maintainer so I will defer to him. > >> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > >> return true; > >> return false; -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 11:47 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-13 11:47 UTC (permalink / raw) To: Michal Hocko Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton, Roman Gushchin, Shakeel Butt While trying to use remote memcg charging in an out-of-tree kernel module I found it's not working, because the current thread is a workqueue thread. As we will probably encounter this issue in the future as the users of memalloc_use_memcg() grow, it's better we fix it now. Signed-off-by: Zefan Li <lizefan@huawei.com> --- v2: add a comment as sugguested by Michal. and add changelog to explain why upstream kernel needs this fix. --- mm/memcontrol.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index a3b97f1..43a12ed 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, static inline bool memcg_kmem_bypass(void) { + /* Allow remote memcg charging in kthread contexts. */ + if (unlikely(current->active_memcg)) + return false; if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) return true; return false; -- 2.7.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 11:47 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-13 11:47 UTC (permalink / raw) To: Michal Hocko Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Roman Gushchin, Shakeel Butt While trying to use remote memcg charging in an out-of-tree kernel module I found it's not working, because the current thread is a workqueue thread. As we will probably encounter this issue in the future as the users of memalloc_use_memcg() grow, it's better we fix it now. Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> --- v2: add a comment as sugguested by Michal. and add changelog to explain why upstream kernel needs this fix. --- mm/memcontrol.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index a3b97f1..43a12ed 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, static inline bool memcg_kmem_bypass(void) { + /* Allow remote memcg charging in kthread contexts. */ + if (unlikely(current->active_memcg)) + return false; if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) return true; return false; -- 2.7.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 12:22 ` Shakeel Butt 0 siblings, 0 replies; 40+ messages in thread From: Shakeel Butt @ 2020-05-13 12:22 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Johannes Weiner, Vladimir Davydov, Cgroups, Linux MM, Andrew Morton, Roman Gushchin On Wed, May 13, 2020 at 4:47 AM Zefan Li <lizefan@huawei.com> wrote: > > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, it's better we fix it now. > > Signed-off-by: Zefan Li <lizefan@huawei.com> > --- > > v2: add a comment as sugguested by Michal. and add changelog to explain why > upstream kernel needs this fix. > > --- > > mm/memcontrol.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a3b97f1..43a12ed 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > static inline bool memcg_kmem_bypass(void) > { > + /* Allow remote memcg charging in kthread contexts. */ > + if (unlikely(current->active_memcg)) > + return false; What about __GFP_ACCOUNT allocations in the interrupt context? e.g. memalloc_use_memcg(memcg); --->interrupt --->alloc_page(GFP_KERNEL_ACCOUNT) in interrupt context. alloc_page(GFP_KERNEL_ACCOUNT); memalloc_unuse_memcg(); > if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > return true; > return false; > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 12:22 ` Shakeel Butt 0 siblings, 0 replies; 40+ messages in thread From: Shakeel Butt @ 2020-05-13 12:22 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Johannes Weiner, Vladimir Davydov, Cgroups, Linux MM, Andrew Morton, Roman Gushchin On Wed, May 13, 2020 at 4:47 AM Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote: > > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, it's better we fix it now. > > Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> > --- > > v2: add a comment as sugguested by Michal. and add changelog to explain why > upstream kernel needs this fix. > > --- > > mm/memcontrol.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a3b97f1..43a12ed 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > static inline bool memcg_kmem_bypass(void) > { > + /* Allow remote memcg charging in kthread contexts. */ > + if (unlikely(current->active_memcg)) > + return false; What about __GFP_ACCOUNT allocations in the interrupt context? e.g. memalloc_use_memcg(memcg); --->interrupt --->alloc_page(GFP_KERNEL_ACCOUNT) in interrupt context. alloc_page(GFP_KERNEL_ACCOUNT); memalloc_unuse_memcg(); > if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > return true; > return false; > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 13:05 ` Johannes Weiner 0 siblings, 0 replies; 40+ messages in thread From: Johannes Weiner @ 2020-05-13 13:05 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton, Roman Gushchin, Shakeel Butt On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, it's better we fix it now. > > Signed-off-by: Zefan Li <lizefan@huawei.com> Acked-by: Johannes Weiner <hannes@cmpxchg.org> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 13:05 ` Johannes Weiner 0 siblings, 0 replies; 40+ messages in thread From: Johannes Weiner @ 2020-05-13 13:05 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Roman Gushchin, Shakeel Butt On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, it's better we fix it now. > > Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> Acked-by: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 16:11 ` Roman Gushchin 0 siblings, 0 replies; 40+ messages in thread From: Roman Gushchin @ 2020-05-13 16:11 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton, Shakeel Butt On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, it's better we fix it now. > > Signed-off-by: Zefan Li <lizefan@huawei.com> > --- > > v2: add a comment as sugguested by Michal. and add changelog to explain why > upstream kernel needs this fix. > > --- > > mm/memcontrol.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a3b97f1..43a12ed 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > static inline bool memcg_kmem_bypass(void) > { > + /* Allow remote memcg charging in kthread contexts. */ > + if (unlikely(current->active_memcg)) > + return false; > if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > return true; Shakeel is right about interrupts. How about something like this? static inline bool memcg_kmem_bypass(void) { if (in_interrupt()) return true; if ((!current->mm || current->flags & PF_KTHREAD) && !current->active_memcg) return true; return false; } Thanks! ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-13 16:11 ` Roman Gushchin 0 siblings, 0 replies; 40+ messages in thread From: Roman Gushchin @ 2020-05-13 16:11 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Shakeel Butt On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, it's better we fix it now. > > Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> > --- > > v2: add a comment as sugguested by Michal. and add changelog to explain why > upstream kernel needs this fix. > > --- > > mm/memcontrol.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a3b97f1..43a12ed 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > static inline bool memcg_kmem_bypass(void) > { > + /* Allow remote memcg charging in kthread contexts. */ > + if (unlikely(current->active_memcg)) > + return false; > if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > return true; Shakeel is right about interrupts. How about something like this? static inline bool memcg_kmem_bypass(void) { if (in_interrupt()) return true; if ((!current->mm || current->flags & PF_KTHREAD) && !current->active_memcg) return true; return false; } Thanks! ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-14 1:16 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-14 1:16 UTC (permalink / raw) To: Roman Gushchin Cc: Michal Hocko, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton, Shakeel Butt On 2020/5/14 0:11, Roman Gushchin wrote: > On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: >> While trying to use remote memcg charging in an out-of-tree kernel module >> I found it's not working, because the current thread is a workqueue thread. >> >> As we will probably encounter this issue in the future as the users of >> memalloc_use_memcg() grow, it's better we fix it now. >> >> Signed-off-by: Zefan Li <lizefan@huawei.com> >> --- >> >> v2: add a comment as sugguested by Michal. and add changelog to explain why >> upstream kernel needs this fix. >> >> --- >> >> mm/memcontrol.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index a3b97f1..43a12ed 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, >> >> static inline bool memcg_kmem_bypass(void) >> { >> + /* Allow remote memcg charging in kthread contexts. */ >> + if (unlikely(current->active_memcg)) >> + return false; >> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) >> return true; > > Shakeel is right about interrupts. How about something like this? > > static inline bool memcg_kmem_bypass(void) > { > if (in_interrupt()) > return true; > > if ((!current->mm || current->flags & PF_KTHREAD) && !current->active_memcg) > return true; > > return false; > } > I thought the user should ensure not do this, but now I think it makes sense to just bypass the interrupt case. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-14 1:16 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-14 1:16 UTC (permalink / raw) To: Roman Gushchin Cc: Michal Hocko, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Shakeel Butt On 2020/5/14 0:11, Roman Gushchin wrote: > On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: >> While trying to use remote memcg charging in an out-of-tree kernel module >> I found it's not working, because the current thread is a workqueue thread. >> >> As we will probably encounter this issue in the future as the users of >> memalloc_use_memcg() grow, it's better we fix it now. >> >> Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> >> --- >> >> v2: add a comment as sugguested by Michal. and add changelog to explain why >> upstream kernel needs this fix. >> >> --- >> >> mm/memcontrol.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index a3b97f1..43a12ed 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, >> >> static inline bool memcg_kmem_bypass(void) >> { >> + /* Allow remote memcg charging in kthread contexts. */ >> + if (unlikely(current->active_memcg)) >> + return false; >> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) >> return true; > > Shakeel is right about interrupts. How about something like this? > > static inline bool memcg_kmem_bypass(void) > { > if (in_interrupt()) > return true; > > if ((!current->mm || current->flags & PF_KTHREAD) && !current->active_memcg) > return true; > > return false; > } > I thought the user should ensure not do this, but now I think it makes sense to just bypass the interrupt case. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-14 22:52 ` Roman Gushchin 0 siblings, 0 replies; 40+ messages in thread From: Roman Gushchin @ 2020-05-14 22:52 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton, Shakeel Butt On Thu, May 14, 2020 at 09:16:29AM +0800, Zefan Li wrote: > On 2020/5/14 0:11, Roman Gushchin wrote: > > On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: > >> While trying to use remote memcg charging in an out-of-tree kernel module > >> I found it's not working, because the current thread is a workqueue thread. > >> > >> As we will probably encounter this issue in the future as the users of > >> memalloc_use_memcg() grow, it's better we fix it now. > >> > >> Signed-off-by: Zefan Li <lizefan@huawei.com> > >> --- > >> > >> v2: add a comment as sugguested by Michal. and add changelog to explain why > >> upstream kernel needs this fix. > >> > >> --- > >> > >> mm/memcontrol.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c > >> index a3b97f1..43a12ed 100644 > >> --- a/mm/memcontrol.c > >> +++ b/mm/memcontrol.c > >> @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > >> > >> static inline bool memcg_kmem_bypass(void) > >> { > >> + /* Allow remote memcg charging in kthread contexts. */ > >> + if (unlikely(current->active_memcg)) > >> + return false; > >> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > >> return true; > > > > Shakeel is right about interrupts. How about something like this? > > > > static inline bool memcg_kmem_bypass(void) > > { > > if (in_interrupt()) > > return true; > > > > if ((!current->mm || current->flags & PF_KTHREAD) && !current->active_memcg) > > return true; > > > > return false; > > } > > > > I thought the user should ensure not do this, but now I think it makes sense to just bypass > the interrupt case. I think now it's mostly a legacy of the opt-out kernel memory accounting. Actually we can relax this requirement by forcibly overcommit the memory cgroup if the allocation is happening from the irq context, and punish it afterwards. Idk how much we wanna this, hopefully nobody is allocating large non-temporarily objects from an irq. Will you send a v3? Thanks! ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-14 22:52 ` Roman Gushchin 0 siblings, 0 replies; 40+ messages in thread From: Roman Gushchin @ 2020-05-14 22:52 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Shakeel Butt On Thu, May 14, 2020 at 09:16:29AM +0800, Zefan Li wrote: > On 2020/5/14 0:11, Roman Gushchin wrote: > > On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: > >> While trying to use remote memcg charging in an out-of-tree kernel module > >> I found it's not working, because the current thread is a workqueue thread. > >> > >> As we will probably encounter this issue in the future as the users of > >> memalloc_use_memcg() grow, it's better we fix it now. > >> > >> Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> > >> --- > >> > >> v2: add a comment as sugguested by Michal. and add changelog to explain why > >> upstream kernel needs this fix. > >> > >> --- > >> > >> mm/memcontrol.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c > >> index a3b97f1..43a12ed 100644 > >> --- a/mm/memcontrol.c > >> +++ b/mm/memcontrol.c > >> @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > >> > >> static inline bool memcg_kmem_bypass(void) > >> { > >> + /* Allow remote memcg charging in kthread contexts. */ > >> + if (unlikely(current->active_memcg)) > >> + return false; > >> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > >> return true; > > > > Shakeel is right about interrupts. How about something like this? > > > > static inline bool memcg_kmem_bypass(void) > > { > > if (in_interrupt()) > > return true; > > > > if ((!current->mm || current->flags & PF_KTHREAD) && !current->active_memcg) > > return true; > > > > return false; > > } > > > > I thought the user should ensure not do this, but now I think it makes sense to just bypass > the interrupt case. I think now it's mostly a legacy of the opt-out kernel memory accounting. Actually we can relax this requirement by forcibly overcommit the memory cgroup if the allocation is happening from the irq context, and punish it afterwards. Idk how much we wanna this, hopefully nobody is allocating large non-temporarily objects from an irq. Will you send a v3? Thanks! ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-15 6:56 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-15 6:56 UTC (permalink / raw) To: Roman Gushchin Cc: Zefan Li, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton, Shakeel Butt On Thu 14-05-20 15:52:59, Roman Gushchin wrote: > On Thu, May 14, 2020 at 09:16:29AM +0800, Zefan Li wrote: > > On 2020/5/14 0:11, Roman Gushchin wrote: > > > On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: > > >> While trying to use remote memcg charging in an out-of-tree kernel module > > >> I found it's not working, because the current thread is a workqueue thread. > > >> > > >> As we will probably encounter this issue in the future as the users of > > >> memalloc_use_memcg() grow, it's better we fix it now. > > >> > > >> Signed-off-by: Zefan Li <lizefan@huawei.com> > > >> --- > > >> > > >> v2: add a comment as sugguested by Michal. and add changelog to explain why > > >> upstream kernel needs this fix. > > >> > > >> --- > > >> > > >> mm/memcontrol.c | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > >> index a3b97f1..43a12ed 100644 > > >> --- a/mm/memcontrol.c > > >> +++ b/mm/memcontrol.c > > >> @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > >> > > >> static inline bool memcg_kmem_bypass(void) > > >> { > > >> + /* Allow remote memcg charging in kthread contexts. */ > > >> + if (unlikely(current->active_memcg)) > > >> + return false; > > >> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > > >> return true; > > > > > > Shakeel is right about interrupts. How about something like this? > > > > > > static inline bool memcg_kmem_bypass(void) > > > { > > > if (in_interrupt()) > > > return true; > > > > > > if ((!current->mm || current->flags & PF_KTHREAD) && !current->active_memcg) > > > return true; > > > > > > return false; > > > } > > > > > > > I thought the user should ensure not do this, but now I think it makes sense to just bypass > > the interrupt case. > > I think now it's mostly a legacy of the opt-out kernel memory accounting. > Actually we can relax this requirement by forcibly overcommit the memory cgroup > if the allocation is happening from the irq context, and punish it afterwards. > Idk how much we wanna this, hopefully nobody is allocating large non-temporarily > objects from an irq. I do not think we want to pretend that remote charging from the IRQ context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > > Will you send a v3? > > Thanks! -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-15 6:56 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-15 6:56 UTC (permalink / raw) To: Roman Gushchin Cc: Zefan Li, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Shakeel Butt On Thu 14-05-20 15:52:59, Roman Gushchin wrote: > On Thu, May 14, 2020 at 09:16:29AM +0800, Zefan Li wrote: > > On 2020/5/14 0:11, Roman Gushchin wrote: > > > On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: > > >> While trying to use remote memcg charging in an out-of-tree kernel module > > >> I found it's not working, because the current thread is a workqueue thread. > > >> > > >> As we will probably encounter this issue in the future as the users of > > >> memalloc_use_memcg() grow, it's better we fix it now. > > >> > > >> Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> > > >> --- > > >> > > >> v2: add a comment as sugguested by Michal. and add changelog to explain why > > >> upstream kernel needs this fix. > > >> > > >> --- > > >> > > >> mm/memcontrol.c | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > >> index a3b97f1..43a12ed 100644 > > >> --- a/mm/memcontrol.c > > >> +++ b/mm/memcontrol.c > > >> @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > >> > > >> static inline bool memcg_kmem_bypass(void) > > >> { > > >> + /* Allow remote memcg charging in kthread contexts. */ > > >> + if (unlikely(current->active_memcg)) > > >> + return false; > > >> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > > >> return true; > > > > > > Shakeel is right about interrupts. How about something like this? > > > > > > static inline bool memcg_kmem_bypass(void) > > > { > > > if (in_interrupt()) > > > return true; > > > > > > if ((!current->mm || current->flags & PF_KTHREAD) && !current->active_memcg) > > > return true; > > > > > > return false; > > > } > > > > > > > I thought the user should ensure not do this, but now I think it makes sense to just bypass > > the interrupt case. > > I think now it's mostly a legacy of the opt-out kernel memory accounting. > Actually we can relax this requirement by forcibly overcommit the memory cgroup > if the allocation is happening from the irq context, and punish it afterwards. > Idk how much we wanna this, hopefully nobody is allocating large non-temporarily > objects from an irq. I do not think we want to pretend that remote charging from the IRQ context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > > Will you send a v3? > > Thanks! -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-15 8:20 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-15 8:20 UTC (permalink / raw) To: Michal Hocko, Roman Gushchin Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton, Shakeel Butt On 2020/5/15 14:56, Michal Hocko wrote: > On Thu 14-05-20 15:52:59, Roman Gushchin wrote: >> On Thu, May 14, 2020 at 09:16:29AM +0800, Zefan Li wrote: >>> On 2020/5/14 0:11, Roman Gushchin wrote: >>>> On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: >>>>> While trying to use remote memcg charging in an out-of-tree kernel module >>>>> I found it's not working, because the current thread is a workqueue thread. >>>>> >>>>> As we will probably encounter this issue in the future as the users of >>>>> memalloc_use_memcg() grow, it's better we fix it now. >>>>> >>>>> Signed-off-by: Zefan Li <lizefan@huawei.com> >>>>> --- >>>>> >>>>> v2: add a comment as sugguested by Michal. and add changelog to explain why >>>>> upstream kernel needs this fix. >>>>> >>>>> --- >>>>> >>>>> mm/memcontrol.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >>>>> index a3b97f1..43a12ed 100644 >>>>> --- a/mm/memcontrol.c >>>>> +++ b/mm/memcontrol.c >>>>> @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, >>>>> >>>>> static inline bool memcg_kmem_bypass(void) >>>>> { >>>>> + /* Allow remote memcg charging in kthread contexts. */ >>>>> + if (unlikely(current->active_memcg)) >>>>> + return false; >>>>> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) >>>>> return true; >>>> >>>> Shakeel is right about interrupts. How about something like this? >>>> >>>> static inline bool memcg_kmem_bypass(void) >>>> { >>>> if (in_interrupt()) >>>> return true; >>>> >>>> if ((!current->mm || current->flags & PF_KTHREAD) && !current->active_memcg) >>>> return true; >>>> >>>> return false; >>>> } >>>> >>> >>> I thought the user should ensure not do this, but now I think it makes sense to just bypass >>> the interrupt case. >> >> I think now it's mostly a legacy of the opt-out kernel memory accounting. >> Actually we can relax this requirement by forcibly overcommit the memory cgroup >> if the allocation is happening from the irq context, and punish it afterwards. >> Idk how much we wanna this, hopefully nobody is allocating large non-temporarily >> objects from an irq. > > I do not think we want to pretend that remote charging from the IRQ > context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > How about: static inline bool memcg_kmem_bypass(void) { if (in_interrupt()) { WARN_ON(current->active_memcg); return true; } /* Allow remote memcg charging in kthread contexts. */ if ((!current->mm || (current->flags & PF_KTHREAD)) && !current->active_memcg) return true; return false; } ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-15 8:20 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-15 8:20 UTC (permalink / raw) To: Michal Hocko, Roman Gushchin Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Shakeel Butt On 2020/5/15 14:56, Michal Hocko wrote: > On Thu 14-05-20 15:52:59, Roman Gushchin wrote: >> On Thu, May 14, 2020 at 09:16:29AM +0800, Zefan Li wrote: >>> On 2020/5/14 0:11, Roman Gushchin wrote: >>>> On Wed, May 13, 2020 at 07:47:49PM +0800, Zefan Li wrote: >>>>> While trying to use remote memcg charging in an out-of-tree kernel module >>>>> I found it's not working, because the current thread is a workqueue thread. >>>>> >>>>> As we will probably encounter this issue in the future as the users of >>>>> memalloc_use_memcg() grow, it's better we fix it now. >>>>> >>>>> Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> >>>>> --- >>>>> >>>>> v2: add a comment as sugguested by Michal. and add changelog to explain why >>>>> upstream kernel needs this fix. >>>>> >>>>> --- >>>>> >>>>> mm/memcontrol.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >>>>> index a3b97f1..43a12ed 100644 >>>>> --- a/mm/memcontrol.c >>>>> +++ b/mm/memcontrol.c >>>>> @@ -2802,6 +2802,9 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, >>>>> >>>>> static inline bool memcg_kmem_bypass(void) >>>>> { >>>>> + /* Allow remote memcg charging in kthread contexts. */ >>>>> + if (unlikely(current->active_memcg)) >>>>> + return false; >>>>> if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) >>>>> return true; >>>> >>>> Shakeel is right about interrupts. How about something like this? >>>> >>>> static inline bool memcg_kmem_bypass(void) >>>> { >>>> if (in_interrupt()) >>>> return true; >>>> >>>> if ((!current->mm || current->flags & PF_KTHREAD) && !current->active_memcg) >>>> return true; >>>> >>>> return false; >>>> } >>>> >>> >>> I thought the user should ensure not do this, but now I think it makes sense to just bypass >>> the interrupt case. >> >> I think now it's mostly a legacy of the opt-out kernel memory accounting. >> Actually we can relax this requirement by forcibly overcommit the memory cgroup >> if the allocation is happening from the irq context, and punish it afterwards. >> Idk how much we wanna this, hopefully nobody is allocating large non-temporarily >> objects from an irq. > > I do not think we want to pretend that remote charging from the IRQ > context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > How about: static inline bool memcg_kmem_bypass(void) { if (in_interrupt()) { WARN_ON(current->active_memcg); return true; } /* Allow remote memcg charging in kthread contexts. */ if ((!current->mm || (current->flags & PF_KTHREAD)) && !current->active_memcg) return true; return false; } ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-15 8:34 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-15 8:34 UTC (permalink / raw) To: Zefan Li Cc: Roman Gushchin, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Andrew Morton, Shakeel Butt On Fri 15-05-20 16:20:04, Li Zefan wrote: > On 2020/5/15 14:56, Michal Hocko wrote: > > On Thu 14-05-20 15:52:59, Roman Gushchin wrote: [...] > >>> I thought the user should ensure not do this, but now I think it makes sense to just bypass > >>> the interrupt case. > >> > >> I think now it's mostly a legacy of the opt-out kernel memory accounting. > >> Actually we can relax this requirement by forcibly overcommit the memory cgroup > >> if the allocation is happening from the irq context, and punish it afterwards. > >> Idk how much we wanna this, hopefully nobody is allocating large non-temporarily > >> objects from an irq. > > > > I do not think we want to pretend that remote charging from the IRQ > > context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > > > > How about: > > static inline bool memcg_kmem_bypass(void) > { > if (in_interrupt()) { > WARN_ON(current->active_memcg); > return true; > } Why not simply if (WARN_ON_ONCE(in_interrupt()) return true; the idea is that we want to catch any __GFP_ACCOUNT user from IRQ context because this just doesn't work and we do not plan to support it for now and foreseeable future. If this is reduced only to active_memcg then we are only getting a partial picture. > > /* Allow remote memcg charging in kthread contexts. */ > if ((!current->mm || (current->flags & PF_KTHREAD)) && !current->active_memcg) > return true; > return false; > } -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-15 8:34 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-15 8:34 UTC (permalink / raw) To: Zefan Li Cc: Roman Gushchin, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Shakeel Butt On Fri 15-05-20 16:20:04, Li Zefan wrote: > On 2020/5/15 14:56, Michal Hocko wrote: > > On Thu 14-05-20 15:52:59, Roman Gushchin wrote: [...] > >>> I thought the user should ensure not do this, but now I think it makes sense to just bypass > >>> the interrupt case. > >> > >> I think now it's mostly a legacy of the opt-out kernel memory accounting. > >> Actually we can relax this requirement by forcibly overcommit the memory cgroup > >> if the allocation is happening from the irq context, and punish it afterwards. > >> Idk how much we wanna this, hopefully nobody is allocating large non-temporarily > >> objects from an irq. > > > > I do not think we want to pretend that remote charging from the IRQ > > context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > > > > How about: > > static inline bool memcg_kmem_bypass(void) > { > if (in_interrupt()) { > WARN_ON(current->active_memcg); > return true; > } Why not simply if (WARN_ON_ONCE(in_interrupt()) return true; the idea is that we want to catch any __GFP_ACCOUNT user from IRQ context because this just doesn't work and we do not plan to support it for now and foreseeable future. If this is reduced only to active_memcg then we are only getting a partial picture. > > /* Allow remote memcg charging in kthread contexts. */ > if ((!current->mm || (current->flags & PF_KTHREAD)) && !current->active_memcg) > return true; > return false; > } -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-15 16:22 ` Shakeel Butt 0 siblings, 0 replies; 40+ messages in thread From: Shakeel Butt @ 2020-05-15 16:22 UTC (permalink / raw) To: Michal Hocko Cc: Zefan Li, Roman Gushchin, Johannes Weiner, Vladimir Davydov, Cgroups, Linux MM, Andrew Morton On Fri, May 15, 2020 at 1:35 AM Michal Hocko <mhocko@kernel.org> wrote: > > On Fri 15-05-20 16:20:04, Li Zefan wrote: > > On 2020/5/15 14:56, Michal Hocko wrote: > > > On Thu 14-05-20 15:52:59, Roman Gushchin wrote: > [...] > > >>> I thought the user should ensure not do this, but now I think it makes sense to just bypass > > >>> the interrupt case. > > >> > > >> I think now it's mostly a legacy of the opt-out kernel memory accounting. > > >> Actually we can relax this requirement by forcibly overcommit the memory cgroup > > >> if the allocation is happening from the irq context, and punish it afterwards. > > >> Idk how much we wanna this, hopefully nobody is allocating large non-temporarily > > >> objects from an irq. > > > > > > I do not think we want to pretend that remote charging from the IRQ > > > context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > > > > > > > How about: > > > > static inline bool memcg_kmem_bypass(void) > > { > > if (in_interrupt()) { > > WARN_ON(current->active_memcg); > > return true; > > } > > Why not simply > > if (WARN_ON_ONCE(in_interrupt()) > return true; > > the idea is that we want to catch any __GFP_ACCOUNT user from IRQ > context because this just doesn't work and we do not plan to support it > for now and foreseeable future. If this is reduced only to active_memcg > then we are only getting a partial picture. > There are SLAB_ACCOUNT kmem caches which do allocations in IRQ context (see sk_prot_alloc()), so, either we make charging work in IRQ or no warnings at all. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-15 16:22 ` Shakeel Butt 0 siblings, 0 replies; 40+ messages in thread From: Shakeel Butt @ 2020-05-15 16:22 UTC (permalink / raw) To: Michal Hocko Cc: Zefan Li, Roman Gushchin, Johannes Weiner, Vladimir Davydov, Cgroups, Linux MM, Andrew Morton On Fri, May 15, 2020 at 1:35 AM Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: > > On Fri 15-05-20 16:20:04, Li Zefan wrote: > > On 2020/5/15 14:56, Michal Hocko wrote: > > > On Thu 14-05-20 15:52:59, Roman Gushchin wrote: > [...] > > >>> I thought the user should ensure not do this, but now I think it makes sense to just bypass > > >>> the interrupt case. > > >> > > >> I think now it's mostly a legacy of the opt-out kernel memory accounting. > > >> Actually we can relax this requirement by forcibly overcommit the memory cgroup > > >> if the allocation is happening from the irq context, and punish it afterwards. > > >> Idk how much we wanna this, hopefully nobody is allocating large non-temporarily > > >> objects from an irq. > > > > > > I do not think we want to pretend that remote charging from the IRQ > > > context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > > > > > > > How about: > > > > static inline bool memcg_kmem_bypass(void) > > { > > if (in_interrupt()) { > > WARN_ON(current->active_memcg); > > return true; > > } > > Why not simply > > if (WARN_ON_ONCE(in_interrupt()) > return true; > > the idea is that we want to catch any __GFP_ACCOUNT user from IRQ > context because this just doesn't work and we do not plan to support it > for now and foreseeable future. If this is reduced only to active_memcg > then we are only getting a partial picture. > There are SLAB_ACCOUNT kmem caches which do allocations in IRQ context (see sk_prot_alloc()), so, either we make charging work in IRQ or no warnings at all. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-15 17:31 ` Roman Gushchin 0 siblings, 0 replies; 40+ messages in thread From: Roman Gushchin @ 2020-05-15 17:31 UTC (permalink / raw) To: Shakeel Butt Cc: Michal Hocko, Zefan Li, Johannes Weiner, Vladimir Davydov, Cgroups, Linux MM, Andrew Morton On Fri, May 15, 2020 at 09:22:25AM -0700, Shakeel Butt wrote: > On Fri, May 15, 2020 at 1:35 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > On Fri 15-05-20 16:20:04, Li Zefan wrote: > > > On 2020/5/15 14:56, Michal Hocko wrote: > > > > On Thu 14-05-20 15:52:59, Roman Gushchin wrote: > > [...] > > > >>> I thought the user should ensure not do this, but now I think it makes sense to just bypass > > > >>> the interrupt case. > > > >> > > > >> I think now it's mostly a legacy of the opt-out kernel memory accounting. > > > >> Actually we can relax this requirement by forcibly overcommit the memory cgroup > > > >> if the allocation is happening from the irq context, and punish it afterwards. > > > >> Idk how much we wanna this, hopefully nobody is allocating large non-temporarily > > > >> objects from an irq. > > > > > > > > I do not think we want to pretend that remote charging from the IRQ > > > > context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > > > > > > > > > > How about: > > > > > > static inline bool memcg_kmem_bypass(void) > > > { > > > if (in_interrupt()) { > > > WARN_ON(current->active_memcg); > > > return true; > > > } > > > > Why not simply > > > > if (WARN_ON_ONCE(in_interrupt()) > > return true; > > > > the idea is that we want to catch any __GFP_ACCOUNT user from IRQ > > context because this just doesn't work and we do not plan to support it > > for now and foreseeable future. Actually, why not? It should be fairly simple, especially after the rework of the slab controller. > If this is reduced only to active_memcg > > then we are only getting a partial picture. > > > > There are SLAB_ACCOUNT kmem caches which do allocations in IRQ context > (see sk_prot_alloc()), so, either we make charging work in IRQ or no > warnings at all. I agree. Actually, there is nothing wrong to warn about, it's just a limitation of the current accounting implementation. Thanks! ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-15 17:31 ` Roman Gushchin 0 siblings, 0 replies; 40+ messages in thread From: Roman Gushchin @ 2020-05-15 17:31 UTC (permalink / raw) To: Shakeel Butt Cc: Michal Hocko, Zefan Li, Johannes Weiner, Vladimir Davydov, Cgroups, Linux MM, Andrew Morton On Fri, May 15, 2020 at 09:22:25AM -0700, Shakeel Butt wrote: > On Fri, May 15, 2020 at 1:35 AM Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: > > > > On Fri 15-05-20 16:20:04, Li Zefan wrote: > > > On 2020/5/15 14:56, Michal Hocko wrote: > > > > On Thu 14-05-20 15:52:59, Roman Gushchin wrote: > > [...] > > > >>> I thought the user should ensure not do this, but now I think it makes sense to just bypass > > > >>> the interrupt case. > > > >> > > > >> I think now it's mostly a legacy of the opt-out kernel memory accounting. > > > >> Actually we can relax this requirement by forcibly overcommit the memory cgroup > > > >> if the allocation is happening from the irq context, and punish it afterwards. > > > >> Idk how much we wanna this, hopefully nobody is allocating large non-temporarily > > > >> objects from an irq. > > > > > > > > I do not think we want to pretend that remote charging from the IRQ > > > > context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > > > > > > > > > > How about: > > > > > > static inline bool memcg_kmem_bypass(void) > > > { > > > if (in_interrupt()) { > > > WARN_ON(current->active_memcg); > > > return true; > > > } > > > > Why not simply > > > > if (WARN_ON_ONCE(in_interrupt()) > > return true; > > > > the idea is that we want to catch any __GFP_ACCOUNT user from IRQ > > context because this just doesn't work and we do not plan to support it > > for now and foreseeable future. Actually, why not? It should be fairly simple, especially after the rework of the slab controller. > If this is reduced only to active_memcg > > then we are only getting a partial picture. > > > > There are SLAB_ACCOUNT kmem caches which do allocations in IRQ context > (see sk_prot_alloc()), so, either we make charging work in IRQ or no > warnings at all. I agree. Actually, there is nothing wrong to warn about, it's just a limitation of the current accounting implementation. Thanks! ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-18 9:13 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-18 9:13 UTC (permalink / raw) To: Shakeel Butt Cc: Zefan Li, Roman Gushchin, Johannes Weiner, Vladimir Davydov, Cgroups, Linux MM, Andrew Morton On Fri 15-05-20 09:22:25, Shakeel Butt wrote: > On Fri, May 15, 2020 at 1:35 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > On Fri 15-05-20 16:20:04, Li Zefan wrote: > > > On 2020/5/15 14:56, Michal Hocko wrote: > > > > On Thu 14-05-20 15:52:59, Roman Gushchin wrote: > > [...] > > > >>> I thought the user should ensure not do this, but now I think it makes sense to just bypass > > > >>> the interrupt case. > > > >> > > > >> I think now it's mostly a legacy of the opt-out kernel memory accounting. > > > >> Actually we can relax this requirement by forcibly overcommit the memory cgroup > > > >> if the allocation is happening from the irq context, and punish it afterwards. > > > >> Idk how much we wanna this, hopefully nobody is allocating large non-temporarily > > > >> objects from an irq. > > > > > > > > I do not think we want to pretend that remote charging from the IRQ > > > > context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > > > > > > > > > > How about: > > > > > > static inline bool memcg_kmem_bypass(void) > > > { > > > if (in_interrupt()) { > > > WARN_ON(current->active_memcg); > > > return true; > > > } > > > > Why not simply > > > > if (WARN_ON_ONCE(in_interrupt()) > > return true; > > > > the idea is that we want to catch any __GFP_ACCOUNT user from IRQ > > context because this just doesn't work and we do not plan to support it > > for now and foreseeable future. If this is reduced only to active_memcg > > then we are only getting a partial picture. > > > > There are SLAB_ACCOUNT kmem caches which do allocations in IRQ context > (see sk_prot_alloc()), so, either we make charging work in IRQ or no > warnings at all. OK, I see. I wasn't aware that those caches are used from IRQ context. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-18 9:13 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-18 9:13 UTC (permalink / raw) To: Shakeel Butt Cc: Zefan Li, Roman Gushchin, Johannes Weiner, Vladimir Davydov, Cgroups, Linux MM, Andrew Morton On Fri 15-05-20 09:22:25, Shakeel Butt wrote: > On Fri, May 15, 2020 at 1:35 AM Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: > > > > On Fri 15-05-20 16:20:04, Li Zefan wrote: > > > On 2020/5/15 14:56, Michal Hocko wrote: > > > > On Thu 14-05-20 15:52:59, Roman Gushchin wrote: > > [...] > > > >>> I thought the user should ensure not do this, but now I think it makes sense to just bypass > > > >>> the interrupt case. > > > >> > > > >> I think now it's mostly a legacy of the opt-out kernel memory accounting. > > > >> Actually we can relax this requirement by forcibly overcommit the memory cgroup > > > >> if the allocation is happening from the irq context, and punish it afterwards. > > > >> Idk how much we wanna this, hopefully nobody is allocating large non-temporarily > > > >> objects from an irq. > > > > > > > > I do not think we want to pretend that remote charging from the IRQ > > > > context is supported. Why don't we simply WARN_ON(in_interrupt()) there? > > > > > > > > > > How about: > > > > > > static inline bool memcg_kmem_bypass(void) > > > { > > > if (in_interrupt()) { > > > WARN_ON(current->active_memcg); > > > return true; > > > } > > > > Why not simply > > > > if (WARN_ON_ONCE(in_interrupt()) > > return true; > > > > the idea is that we want to catch any __GFP_ACCOUNT user from IRQ > > context because this just doesn't work and we do not plan to support it > > for now and foreseeable future. If this is reduced only to active_memcg > > then we are only getting a partial picture. > > > > There are SLAB_ACCOUNT kmem caches which do allocations in IRQ context > (see sk_prot_alloc()), so, either we make charging work in IRQ or no > warnings at all. OK, I see. I wasn't aware that those caches are used from IRQ context. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-26 1:25 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-26 1:25 UTC (permalink / raw) To: Michal Hocko, Andrew Morton Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Roman Gushchin, Shakeel Butt While trying to use remote memcg charging in an out-of-tree kernel module I found it's not working, because the current thread is a workqueue thread. As we will probably encounter this issue in the future as the users of memalloc_use_memcg() grow, and it's nothing wrong for this usage, it's better we fix it now. Acked-by: Johannes Weiner <hannes@cmpxchg.org> Signed-off-by: Zefan Li <lizefan@huawei.com> --- v3: bypass __GFP_ACCOUNT allocations in interrupt contexts. --- mm/memcontrol.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index a3b97f1..3c7717a 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2802,7 +2802,12 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, static inline bool memcg_kmem_bypass(void) { - if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) + if (in_interrupt()) + return true; + + /* Allow remote memcg charging in kthread contexts. */ + if ((!current->mm || (current->flags & PF_KTHREAD)) && + !current->active_memcg) return true; return false; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-26 1:25 ` Zefan Li 0 siblings, 0 replies; 40+ messages in thread From: Zefan Li @ 2020-05-26 1:25 UTC (permalink / raw) To: Michal Hocko, Andrew Morton Cc: Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Roman Gushchin, Shakeel Butt While trying to use remote memcg charging in an out-of-tree kernel module I found it's not working, because the current thread is a workqueue thread. As we will probably encounter this issue in the future as the users of memalloc_use_memcg() grow, and it's nothing wrong for this usage, it's better we fix it now. Acked-by: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> --- v3: bypass __GFP_ACCOUNT allocations in interrupt contexts. --- mm/memcontrol.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index a3b97f1..3c7717a 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2802,7 +2802,12 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, static inline bool memcg_kmem_bypass(void) { - if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) + if (in_interrupt()) + return true; + + /* Allow remote memcg charging in kthread contexts. */ + if ((!current->mm || (current->flags & PF_KTHREAD)) && + !current->active_memcg) return true; return false; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v3] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-26 15:53 ` Roman Gushchin 0 siblings, 0 replies; 40+ messages in thread From: Roman Gushchin @ 2020-05-26 15:53 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Andrew Morton, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Shakeel Butt On Tue, May 26, 2020 at 09:25:25AM +0800, Zefan Li wrote: > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, and it's nothing wrong for this usage, it's > better we fix it now. > > Acked-by: Johannes Weiner <hannes@cmpxchg.org> > Signed-off-by: Zefan Li <lizefan@huawei.com> > --- Reviewed-by: Roman Gushchin <guro@fb.com> Thanks! > > v3: bypass __GFP_ACCOUNT allocations in interrupt contexts. > > --- > mm/memcontrol.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a3b97f1..3c7717a 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2802,7 +2802,12 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > static inline bool memcg_kmem_bypass(void) > { > - if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > + if (in_interrupt()) > + return true; > + > + /* Allow remote memcg charging in kthread contexts. */ > + if ((!current->mm || (current->flags & PF_KTHREAD)) && > + !current->active_memcg) > return true; > return false; > } > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-26 15:53 ` Roman Gushchin 0 siblings, 0 replies; 40+ messages in thread From: Roman Gushchin @ 2020-05-26 15:53 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Andrew Morton, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Shakeel Butt On Tue, May 26, 2020 at 09:25:25AM +0800, Zefan Li wrote: > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, and it's nothing wrong for this usage, it's > better we fix it now. > > Acked-by: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> > Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> > --- Reviewed-by: Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org> Thanks! > > v3: bypass __GFP_ACCOUNT allocations in interrupt contexts. > > --- > mm/memcontrol.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a3b97f1..3c7717a 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2802,7 +2802,12 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > static inline bool memcg_kmem_bypass(void) > { > - if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > + if (in_interrupt()) > + return true; > + > + /* Allow remote memcg charging in kthread contexts. */ > + if ((!current->mm || (current->flags & PF_KTHREAD)) && > + !current->active_memcg) > return true; > return false; > } > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-27 16:50 ` Shakeel Butt 0 siblings, 0 replies; 40+ messages in thread From: Shakeel Butt @ 2020-05-27 16:50 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Andrew Morton, Johannes Weiner, Vladimir Davydov, Cgroups, Linux MM, Roman Gushchin On Mon, May 25, 2020 at 6:25 PM Zefan Li <lizefan@huawei.com> wrote: > > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, and it's nothing wrong for this usage, it's > better we fix it now. > > Acked-by: Johannes Weiner <hannes@cmpxchg.org> > Signed-off-by: Zefan Li <lizefan@huawei.com> Reviewed-by: Shakeel Butt <shakeelb@google.com> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-27 16:50 ` Shakeel Butt 0 siblings, 0 replies; 40+ messages in thread From: Shakeel Butt @ 2020-05-27 16:50 UTC (permalink / raw) To: Zefan Li Cc: Michal Hocko, Andrew Morton, Johannes Weiner, Vladimir Davydov, Cgroups, Linux MM, Roman Gushchin On Mon, May 25, 2020 at 6:25 PM Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote: > > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, and it's nothing wrong for this usage, it's > better we fix it now. > > Acked-by: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> > Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> Reviewed-by: Shakeel Butt <shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-28 14:44 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-28 14:44 UTC (permalink / raw) To: Zefan Li Cc: Andrew Morton, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm, Roman Gushchin, Shakeel Butt On Tue 26-05-20 09:25:25, Li Zefan wrote: > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, and it's nothing wrong for this usage, it's > better we fix it now. > > Acked-by: Johannes Weiner <hannes@cmpxchg.org> > Signed-off-by: Zefan Li <lizefan@huawei.com> Acked-by: Michal Hocko <mhocko@suse.com> > --- > > v3: bypass __GFP_ACCOUNT allocations in interrupt contexts. > > --- > mm/memcontrol.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a3b97f1..3c7717a 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2802,7 +2802,12 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > static inline bool memcg_kmem_bypass(void) > { > - if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > + if (in_interrupt()) > + return true; > + > + /* Allow remote memcg charging in kthread contexts. */ > + if ((!current->mm || (current->flags & PF_KTHREAD)) && > + !current->active_memcg) > return true; > return false; > } > -- > 2.7.4 -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3] memcg: Fix memcg_kmem_bypass() for remote memcg charging @ 2020-05-28 14:44 ` Michal Hocko 0 siblings, 0 replies; 40+ messages in thread From: Michal Hocko @ 2020-05-28 14:44 UTC (permalink / raw) To: Zefan Li Cc: Andrew Morton, Johannes Weiner, Vladimir Davydov, Cgroups, linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Roman Gushchin, Shakeel Butt On Tue 26-05-20 09:25:25, Li Zefan wrote: > While trying to use remote memcg charging in an out-of-tree kernel module > I found it's not working, because the current thread is a workqueue thread. > > As we will probably encounter this issue in the future as the users of > memalloc_use_memcg() grow, and it's nothing wrong for this usage, it's > better we fix it now. > > Acked-by: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> > Signed-off-by: Zefan Li <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> Acked-by: Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org> > --- > > v3: bypass __GFP_ACCOUNT allocations in interrupt contexts. > > --- > mm/memcontrol.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a3b97f1..3c7717a 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2802,7 +2802,12 @@ static void memcg_schedule_kmem_cache_create(struct mem_cgroup *memcg, > > static inline bool memcg_kmem_bypass(void) > { > - if (in_interrupt() || !current->mm || (current->flags & PF_KTHREAD)) > + if (in_interrupt()) > + return true; > + > + /* Allow remote memcg charging in kthread contexts. */ > + if ((!current->mm || (current->flags & PF_KTHREAD)) && > + !current->active_memcg) > return true; > return false; > } > -- > 2.7.4 -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2020-05-28 14:44 UTC | newest] Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-13 7:28 [PATCH] memcg: Fix memcg_kmem_bypass() for remote memcg charging Zefan Li 2020-05-13 7:28 ` Zefan Li 2020-05-13 9:05 ` Michal Hocko 2020-05-13 9:05 ` Michal Hocko 2020-05-13 11:19 ` Zefan Li 2020-05-13 11:19 ` Zefan Li 2020-05-13 11:29 ` Michal Hocko 2020-05-13 11:29 ` Michal Hocko 2020-05-13 11:47 ` [PATCH v2] " Zefan Li 2020-05-13 11:47 ` Zefan Li 2020-05-13 12:22 ` Shakeel Butt 2020-05-13 12:22 ` Shakeel Butt 2020-05-13 13:05 ` Johannes Weiner 2020-05-13 13:05 ` Johannes Weiner 2020-05-13 16:11 ` Roman Gushchin 2020-05-13 16:11 ` Roman Gushchin 2020-05-14 1:16 ` Zefan Li 2020-05-14 1:16 ` Zefan Li 2020-05-14 22:52 ` Roman Gushchin 2020-05-14 22:52 ` Roman Gushchin 2020-05-15 6:56 ` Michal Hocko 2020-05-15 6:56 ` Michal Hocko 2020-05-15 8:20 ` Zefan Li 2020-05-15 8:20 ` Zefan Li 2020-05-15 8:34 ` Michal Hocko 2020-05-15 8:34 ` Michal Hocko 2020-05-15 16:22 ` Shakeel Butt 2020-05-15 16:22 ` Shakeel Butt 2020-05-15 17:31 ` Roman Gushchin 2020-05-15 17:31 ` Roman Gushchin 2020-05-18 9:13 ` Michal Hocko 2020-05-18 9:13 ` Michal Hocko 2020-05-26 1:25 ` [PATCH v3] " Zefan Li 2020-05-26 1:25 ` Zefan Li 2020-05-26 15:53 ` Roman Gushchin 2020-05-26 15:53 ` Roman Gushchin 2020-05-27 16:50 ` Shakeel Butt 2020-05-27 16:50 ` Shakeel Butt 2020-05-28 14:44 ` Michal Hocko 2020-05-28 14:44 ` Michal Hocko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.