From: Waiman Long <longman@redhat.com> To: Johannes Weiner <hannes@cmpxchg.org>, Michal Hocko <mhocko@kernel.org>, Vladimir Davydov <vdavydov.dev@gmail.com>, Andrew Morton <akpm@linux-foundation.org>, Vlastimil Babka <vbabka@suse.cz>, Roman Gushchin <guro@fb.com> Cc: linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-mm@kvack.org, Shakeel Butt <shakeelb@google.com>, Muchun Song <songmuchun@bytedance.com>, Luis Goncalves <lgoncalv@redhat.com>, Waiman Long <longman@redhat.com> Subject: [PATCH] mm/memcg: Disable task obj_stock for PREEMPT_RT Date: Tue, 3 Aug 2021 13:55:19 -0400 [thread overview] Message-ID: <20210803175519.22298-1-longman@redhat.com> (raw) For PREEMPT_RT kernel, preempt_disable() and local_irq_save() are typically converted to local_lock() and local_lock_irqsave() respectively. These two variants of local_lock() are essentially the same. Thus, there is no performance advantage in choosing one over the other. As there is no point in maintaining two different sets of obj_stock, it is simpler and more efficient to just disable task_obj and use only irq_obj for PREEMPT_RT. However, task_obj will still be there in the memcg_stock_pcp structure even though it is not used in this configuration. Signed-off-by: Waiman Long <longman@redhat.com> --- mm/memcontrol.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 87c883227f90..4f80770cb97b 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2120,12 +2120,22 @@ static bool obj_stock_flush_required(struct memcg_stock_pcp *stock, * which is cheap in non-preempt kernel. The interrupt context object stock * can only be accessed after disabling interrupt. User context code can * access interrupt object stock, but not vice versa. + * + * For PREEMPT_RT kernel, preempt_disable() and local_irq_save() may have + * to be changed to variants of local_lock(). This eliminates the + * performance advantage of using preempt_disable(). Fall back to always + * use local_irq_save() and use only irq_obj for simplicity. */ +static inline bool use_task_obj_stock(void) +{ + return !IS_ENABLED(CONFIG_PREEMPT_RT) && likely(in_task()); +} + static inline struct obj_stock *get_obj_stock(unsigned long *pflags) { struct memcg_stock_pcp *stock; - if (likely(in_task())) { + if (use_task_obj_stock()) { *pflags = 0UL; preempt_disable(); stock = this_cpu_ptr(&memcg_stock); @@ -2139,7 +2149,7 @@ static inline struct obj_stock *get_obj_stock(unsigned long *pflags) static inline void put_obj_stock(unsigned long flags) { - if (likely(in_task())) + if (use_task_obj_stock()) preempt_enable(); else local_irq_restore(flags); @@ -2212,7 +2222,7 @@ static void drain_local_stock(struct work_struct *dummy) stock = this_cpu_ptr(&memcg_stock); drain_obj_stock(&stock->irq_obj); - if (in_task()) + if (use_task_obj_stock()) drain_obj_stock(&stock->task_obj); drain_stock(stock); clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags); @@ -3217,7 +3227,7 @@ static bool obj_stock_flush_required(struct memcg_stock_pcp *stock, { struct mem_cgroup *memcg; - if (in_task() && stock->task_obj.cached_objcg) { + if (use_task_obj_stock() && stock->task_obj.cached_objcg) { memcg = obj_cgroup_memcg(stock->task_obj.cached_objcg); if (memcg && mem_cgroup_is_descendant(memcg, root_memcg)) return true; -- 2.18.1
WARNING: multiple messages have this Message-ID (diff)
From: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> To: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>, Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Vladimir Davydov <vdavydov.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, Vlastimil Babka <vbabka-AlSwsSmVLrQ@public.gmane.org>, Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Shakeel Butt <shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Muchun Song <songmuchun-EC8Uxl6Npydl57MIdRCFDg@public.gmane.org>, Luis Goncalves <lgoncalv-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Subject: [PATCH] mm/memcg: Disable task obj_stock for PREEMPT_RT Date: Tue, 3 Aug 2021 13:55:19 -0400 [thread overview] Message-ID: <20210803175519.22298-1-longman@redhat.com> (raw) For PREEMPT_RT kernel, preempt_disable() and local_irq_save() are typically converted to local_lock() and local_lock_irqsave() respectively. These two variants of local_lock() are essentially the same. Thus, there is no performance advantage in choosing one over the other. As there is no point in maintaining two different sets of obj_stock, it is simpler and more efficient to just disable task_obj and use only irq_obj for PREEMPT_RT. However, task_obj will still be there in the memcg_stock_pcp structure even though it is not used in this configuration. Signed-off-by: Waiman Long <longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> --- mm/memcontrol.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 87c883227f90..4f80770cb97b 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2120,12 +2120,22 @@ static bool obj_stock_flush_required(struct memcg_stock_pcp *stock, * which is cheap in non-preempt kernel. The interrupt context object stock * can only be accessed after disabling interrupt. User context code can * access interrupt object stock, but not vice versa. + * + * For PREEMPT_RT kernel, preempt_disable() and local_irq_save() may have + * to be changed to variants of local_lock(). This eliminates the + * performance advantage of using preempt_disable(). Fall back to always + * use local_irq_save() and use only irq_obj for simplicity. */ +static inline bool use_task_obj_stock(void) +{ + return !IS_ENABLED(CONFIG_PREEMPT_RT) && likely(in_task()); +} + static inline struct obj_stock *get_obj_stock(unsigned long *pflags) { struct memcg_stock_pcp *stock; - if (likely(in_task())) { + if (use_task_obj_stock()) { *pflags = 0UL; preempt_disable(); stock = this_cpu_ptr(&memcg_stock); @@ -2139,7 +2149,7 @@ static inline struct obj_stock *get_obj_stock(unsigned long *pflags) static inline void put_obj_stock(unsigned long flags) { - if (likely(in_task())) + if (use_task_obj_stock()) preempt_enable(); else local_irq_restore(flags); @@ -2212,7 +2222,7 @@ static void drain_local_stock(struct work_struct *dummy) stock = this_cpu_ptr(&memcg_stock); drain_obj_stock(&stock->irq_obj); - if (in_task()) + if (use_task_obj_stock()) drain_obj_stock(&stock->task_obj); drain_stock(stock); clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags); @@ -3217,7 +3227,7 @@ static bool obj_stock_flush_required(struct memcg_stock_pcp *stock, { struct mem_cgroup *memcg; - if (in_task() && stock->task_obj.cached_objcg) { + if (use_task_obj_stock() && stock->task_obj.cached_objcg) { memcg = obj_cgroup_memcg(stock->task_obj.cached_objcg); if (memcg && mem_cgroup_is_descendant(memcg, root_memcg)) return true; -- 2.18.1
next reply other threads:[~2021-08-03 17:55 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-03 17:55 Waiman Long [this message] 2021-08-03 17:55 ` [PATCH] mm/memcg: Disable task obj_stock for PREEMPT_RT Waiman Long 2021-08-03 23:21 ` Thomas Gleixner 2021-08-03 23:21 ` Thomas Gleixner 2021-08-03 23:21 ` Thomas Gleixner 2021-08-04 1:40 ` Waiman Long 2021-08-04 1:40 ` Waiman Long 2021-08-04 8:52 ` Michal Hocko 2021-08-04 8:52 ` Michal Hocko 2021-08-04 16:00 ` Waiman Long 2021-08-04 16:00 ` Waiman Long 2021-08-04 7:39 ` Vlastimil Babka 2021-08-04 7:39 ` Vlastimil Babka 2021-08-04 8:33 ` Michal Hocko 2021-08-09 9:07 ` Michal Hocko 2021-08-09 9:07 ` Michal Hocko 2021-08-09 9:28 ` Vlastimil Babka 2021-08-09 9:28 ` Vlastimil Babka
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210803175519.22298-1-longman@redhat.com \ --to=longman@redhat.com \ --cc=akpm@linux-foundation.org \ --cc=cgroups@vger.kernel.org \ --cc=guro@fb.com \ --cc=hannes@cmpxchg.org \ --cc=lgoncalv@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mhocko@kernel.org \ --cc=shakeelb@google.com \ --cc=songmuchun@bytedance.com \ --cc=vbabka@suse.cz \ --cc=vdavydov.dev@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.