From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> To: "Michal Koutný" <mkoutny@suse.com> Cc: akpm@linux-foundation.org, cgroups@vger.kernel.org, hannes@cmpxchg.org, linux-mm@kvack.org, longman@redhat.com, mhocko@kernel.org, peterz@infradead.org, tglx@linutronix.de, vdavydov.dev@gmail.com Subject: Re: [PATCH] mm/memcg: Do not check v1 event counter when not needed Date: Tue, 18 Jan 2022 20:57:17 +0100 [thread overview] Message-ID: <YecbnYDuBXgAjPs1@linutronix.de> (raw) In-Reply-To: <20220118182600.15007-1-mkoutny@suse.com> On 2022-01-18 19:26:00 [+0100], Michal Koutný wrote: > I think it would make sense inserting the patch into your series and > subsequently reject enabling on PREEMPT_RT -- provided this patch makes sense > to others too -- the justification is rather functionality splitting for > this PREEMPT_RT effort. Interesting. So while looking at this today I came up with the patch at the bottom. The other things I had looked way uglier and then since nobody probably will use it… Let me know how you want it to be integrated. ------>8------ From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Date: Tue, 18 Jan 2022 17:28:07 +0100 Subject: [PATCH] mm/memcg: Disable threshold event handlers on PREEMPT_RT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During the integration of PREEMPT_RT support, the code flow around memcg_check_events() resulted in `twisted code'. Moving the code around and avoiding then would then lead to an additional local-irq-save section within memcg_check_events(). While looking better, it adds a local-irq-save section to code flow which is usually within an local-irq-save block. The threshold event handler is a deprecated memcg v1 feature. Instead of trying to get it to work under PREEMPT_RT just disable it. There should have not been any users on PREEMPT_RT. From that perspective makes it even less sense to get it to work under PREEMPT_RT while having zero users. Make memory.soft_limit_in_bytes and cgroup.event_control return -EOPNOTSUPP on PREEMPT_RT. Make memcg_check_events() empty on PREEMPT_RT since it won't do anything. Document that the two knobs are disabled on PREEMPT_RT. Suggested-by: Michal Hocko <mhocko@kernel.org> Suggested-by: Michal Koutný <mkoutny@suse.com> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- Documentation/admin-guide/cgroup-v1/memory.rst | 2 ++ mm/memcontrol.c | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/Documentation/admin-guide/cgroup-v1/memory.rst b/Documentation/admin-guide/cgroup-v1/memory.rst index faac50149a222..2cc502a75ef64 100644 --- a/Documentation/admin-guide/cgroup-v1/memory.rst +++ b/Documentation/admin-guide/cgroup-v1/memory.rst @@ -64,6 +64,7 @@ Brief summary of control files. threads cgroup.procs show list of processes cgroup.event_control an interface for event_fd() + This knob is not available on CONFIG_PREEMPT_RT systems. memory.usage_in_bytes show current usage for memory (See 5.5 for details) memory.memsw.usage_in_bytes show current usage for memory+Swap @@ -75,6 +76,7 @@ Brief summary of control files. memory.max_usage_in_bytes show max memory usage recorded memory.memsw.max_usage_in_bytes show max memory+Swap usage recorded memory.soft_limit_in_bytes set/show soft limit of memory usage + This knob is not available on CONFIG_PREEMPT_RT systems. memory.stat show various statistics memory.use_hierarchy set/show hierarchical account enabled This knob is deprecated and shouldn't be diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 2ed5f2a0879d3..3c4f7a0fd0039 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -821,6 +821,7 @@ static void mem_cgroup_charge_statistics(struct mem_cgroup *memcg, __this_cpu_add(memcg->vmstats_percpu->nr_page_events, nr_pages); } +#ifndef CONFIG_PREEMPT_RT static bool mem_cgroup_event_ratelimit(struct mem_cgroup *memcg, enum mem_cgroup_events_target target) { @@ -864,6 +865,9 @@ static void memcg_check_events(struct mem_cgroup *memcg, int nid) mem_cgroup_update_tree(memcg, nid); } } +#else +static void memcg_check_events(struct mem_cgroup *memcg, int nid) { } +#endif struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p) { @@ -3751,8 +3755,12 @@ static ssize_t mem_cgroup_write(struct kernfs_open_file *of, } break; case RES_SOFT_LIMIT: +#ifndef CONFIG_PREEMPT_RT memcg->soft_limit = nr_pages; ret = 0; +#else + ret = -EOPNOTSUPP; +#endif break; } return ret ?: nbytes; @@ -4717,6 +4725,7 @@ static void memcg_event_ptable_queue_proc(struct file *file, static ssize_t memcg_write_event_control(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off) { +#ifndef CONFIG_PREEMPT_RT struct cgroup_subsys_state *css = of_css(of); struct mem_cgroup *memcg = mem_cgroup_from_css(css); struct mem_cgroup_event *event; @@ -4843,6 +4852,9 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of, kfree(event); return ret; +#else + return -EOPNOTSUPP; +#endif } static struct cftype mem_cgroup_legacy_files[] = { -- 2.34.1
WARNING: multiple messages have this Message-ID (diff)
From: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> To: "Michal Koutný" <mkoutny-IBi9RG/b67k@public.gmane.org> Cc: akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, longman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org, vdavydov.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org Subject: Re: [PATCH] mm/memcg: Do not check v1 event counter when not needed Date: Tue, 18 Jan 2022 20:57:17 +0100 [thread overview] Message-ID: <YecbnYDuBXgAjPs1@linutronix.de> (raw) In-Reply-To: <20220118182600.15007-1-mkoutny-IBi9RG/b67k@public.gmane.org> On 2022-01-18 19:26:00 [+0100], Michal Koutný wrote: > I think it would make sense inserting the patch into your series and > subsequently reject enabling on PREEMPT_RT -- provided this patch makes sense > to others too -- the justification is rather functionality splitting for > this PREEMPT_RT effort. Interesting. So while looking at this today I came up with the patch at the bottom. The other things I had looked way uglier and then since nobody probably will use it… Let me know how you want it to be integrated. ------>8------ From: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> Date: Tue, 18 Jan 2022 17:28:07 +0100 Subject: [PATCH] mm/memcg: Disable threshold event handlers on PREEMPT_RT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During the integration of PREEMPT_RT support, the code flow around memcg_check_events() resulted in `twisted code'. Moving the code around and avoiding then would then lead to an additional local-irq-save section within memcg_check_events(). While looking better, it adds a local-irq-save section to code flow which is usually within an local-irq-save block. The threshold event handler is a deprecated memcg v1 feature. Instead of trying to get it to work under PREEMPT_RT just disable it. There should have not been any users on PREEMPT_RT. From that perspective makes it even less sense to get it to work under PREEMPT_RT while having zero users. Make memory.soft_limit_in_bytes and cgroup.event_control return -EOPNOTSUPP on PREEMPT_RT. Make memcg_check_events() empty on PREEMPT_RT since it won't do anything. Document that the two knobs are disabled on PREEMPT_RT. Suggested-by: Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Suggested-by: Michal Koutný <mkoutny-IBi9RG/b67k@public.gmane.org> Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> --- Documentation/admin-guide/cgroup-v1/memory.rst | 2 ++ mm/memcontrol.c | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/Documentation/admin-guide/cgroup-v1/memory.rst b/Documentation/admin-guide/cgroup-v1/memory.rst index faac50149a222..2cc502a75ef64 100644 --- a/Documentation/admin-guide/cgroup-v1/memory.rst +++ b/Documentation/admin-guide/cgroup-v1/memory.rst @@ -64,6 +64,7 @@ Brief summary of control files. threads cgroup.procs show list of processes cgroup.event_control an interface for event_fd() + This knob is not available on CONFIG_PREEMPT_RT systems. memory.usage_in_bytes show current usage for memory (See 5.5 for details) memory.memsw.usage_in_bytes show current usage for memory+Swap @@ -75,6 +76,7 @@ Brief summary of control files. memory.max_usage_in_bytes show max memory usage recorded memory.memsw.max_usage_in_bytes show max memory+Swap usage recorded memory.soft_limit_in_bytes set/show soft limit of memory usage + This knob is not available on CONFIG_PREEMPT_RT systems. memory.stat show various statistics memory.use_hierarchy set/show hierarchical account enabled This knob is deprecated and shouldn't be diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 2ed5f2a0879d3..3c4f7a0fd0039 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -821,6 +821,7 @@ static void mem_cgroup_charge_statistics(struct mem_cgroup *memcg, __this_cpu_add(memcg->vmstats_percpu->nr_page_events, nr_pages); } +#ifndef CONFIG_PREEMPT_RT static bool mem_cgroup_event_ratelimit(struct mem_cgroup *memcg, enum mem_cgroup_events_target target) { @@ -864,6 +865,9 @@ static void memcg_check_events(struct mem_cgroup *memcg, int nid) mem_cgroup_update_tree(memcg, nid); } } +#else +static void memcg_check_events(struct mem_cgroup *memcg, int nid) { } +#endif struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p) { @@ -3751,8 +3755,12 @@ static ssize_t mem_cgroup_write(struct kernfs_open_file *of, } break; case RES_SOFT_LIMIT: +#ifndef CONFIG_PREEMPT_RT memcg->soft_limit = nr_pages; ret = 0; +#else + ret = -EOPNOTSUPP; +#endif break; } return ret ?: nbytes; @@ -4717,6 +4725,7 @@ static void memcg_event_ptable_queue_proc(struct file *file, static ssize_t memcg_write_event_control(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off) { +#ifndef CONFIG_PREEMPT_RT struct cgroup_subsys_state *css = of_css(of); struct mem_cgroup *memcg = mem_cgroup_from_css(css); struct mem_cgroup_event *event; @@ -4843,6 +4852,9 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of, kfree(event); return ret; +#else + return -EOPNOTSUPP; +#endif } static struct cftype mem_cgroup_legacy_files[] = { -- 2.34.1
next prev parent reply other threads:[~2022-01-18 19:57 UTC|newest] Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-22 11:41 [RFC PATCH 0/3] mm/memcg: Address PREEMPT_RT problems instead of disabling it Sebastian Andrzej Siewior 2021-12-22 11:41 ` Sebastian Andrzej Siewior 2021-12-22 11:41 ` [RFC PATCH 1/3] mm/memcg: Protect per-CPU counter by disabling preemption on PREEMPT_RT Sebastian Andrzej Siewior 2021-12-22 11:41 ` Sebastian Andrzej Siewior 2021-12-23 2:31 ` Waiman Long 2021-12-23 2:31 ` Waiman Long 2021-12-23 7:34 ` Sebastian Andrzej Siewior 2021-12-23 7:34 ` Sebastian Andrzej Siewior 2021-12-23 16:01 ` Waiman Long 2021-12-23 16:01 ` Waiman Long 2022-01-05 14:16 ` Michal Koutný 2022-01-05 14:16 ` Michal Koutný 2022-01-13 13:08 ` Sebastian Andrzej Siewior 2022-01-13 13:08 ` Sebastian Andrzej Siewior 2022-01-13 14:48 ` Michal Koutný 2022-01-13 14:48 ` Michal Koutný 2022-01-14 9:09 ` Sebastian Andrzej Siewior 2022-01-14 9:09 ` Sebastian Andrzej Siewior 2022-01-18 18:26 ` [PATCH] mm/memcg: Do not check v1 event counter when not needed Michal Koutný 2022-01-18 18:26 ` Michal Koutný 2022-01-18 19:57 ` Sebastian Andrzej Siewior [this message] 2022-01-18 19:57 ` Sebastian Andrzej Siewior 2021-12-22 11:41 ` [RFC PATCH 2/3] mm/memcg: Add a local_lock_t for IRQ and TASK object Sebastian Andrzej Siewior 2021-12-22 11:41 ` Sebastian Andrzej Siewior 2021-12-23 21:38 ` Waiman Long 2021-12-23 21:38 ` Waiman Long 2022-01-03 16:34 ` Sebastian Andrzej Siewior 2022-01-03 16:34 ` Sebastian Andrzej Siewior 2022-01-03 17:09 ` Waiman Long 2022-01-03 17:09 ` Waiman Long 2021-12-22 11:41 ` [RFC PATCH 3/3] mm/memcg: Allow the task_obj optimization only on non-PREEMPTIBLE kernels Sebastian Andrzej Siewior 2021-12-22 11:41 ` Sebastian Andrzej Siewior 2021-12-23 21:48 ` Waiman Long 2021-12-23 21:48 ` Waiman Long 2022-01-03 14:44 ` Sebastian Andrzej Siewior 2022-01-03 14:44 ` Sebastian Andrzej Siewior 2022-01-03 15:04 ` Waiman Long 2022-01-03 15:04 ` Waiman Long 2022-01-05 20:22 ` Sebastian Andrzej Siewior 2022-01-05 20:22 ` Sebastian Andrzej Siewior 2022-01-06 3:28 ` Waiman Long 2022-01-06 3:28 ` Waiman Long 2022-01-13 15:26 ` Sebastian Andrzej Siewior 2022-01-13 15:26 ` Sebastian Andrzej Siewior 2022-01-05 14:59 ` [RFC PATCH 0/3] mm/memcg: Address PREEMPT_RT problems instead of disabling it Michal Koutný 2022-01-05 14:59 ` Michal Koutný 2022-01-05 15:06 ` Sebastian Andrzej Siewior 2022-01-05 15:06 ` Sebastian Andrzej Siewior
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=YecbnYDuBXgAjPs1@linutronix.de \ --to=bigeasy@linutronix.de \ --cc=akpm@linux-foundation.org \ --cc=cgroups@vger.kernel.org \ --cc=hannes@cmpxchg.org \ --cc=linux-mm@kvack.org \ --cc=longman@redhat.com \ --cc=mhocko@kernel.org \ --cc=mkoutny@suse.com \ --cc=peterz@infradead.org \ --cc=tglx@linutronix.de \ --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.