* [PATCH 2/2] mm: Extract memcg maxable seq_file logic to seq_show_memcg_tunable
@ 2019-01-24 19:41 Chris Down
2019-01-24 21:18 ` Johannes Weiner
2019-01-25 7:24 ` Michal Hocko
0 siblings, 2 replies; 3+ messages in thread
From: Chris Down @ 2019-01-24 19:41 UTC (permalink / raw)
To: Andrew Morton
Cc: Johannes Weiner, Tejun Heo, Roman Gushchin, linux-kernel,
cgroups, linux-mm, kernel-team
memcg has a significant number of files exposed to kernfs where their
value is either exposed directly or is "max" in the case of
PAGE_COUNTER_MAX.
This patch makes this generic by providing a single function to do this
work. In combination with the previous patch adding mem_cgroup_from_seq,
this makes all of the seq_show feeder functions significantly more
simple.
Signed-off-by: Chris Down <chris@chrisdown.name>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Roman Gushchin <guro@fb.com>
Cc: linux-kernel@vger.kernel.org
Cc: cgroups@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: kernel-team@fb.com
---
mm/memcontrol.c | 64 +++++++++++++++----------------------------------
1 file changed, 19 insertions(+), 45 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 98aad31f5226..81b6f752471a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5375,6 +5375,16 @@ static void mem_cgroup_bind(struct cgroup_subsys_state *root_css)
root_mem_cgroup->use_hierarchy = false;
}
+static int seq_puts_memcg_tunable(struct seq_file *m, unsigned long value)
+{
+ if (value == PAGE_COUNTER_MAX)
+ seq_puts(m, "max\n");
+ else
+ seq_printf(m, "%llu\n", (u64)value * PAGE_SIZE);
+
+ return 0;
+}
+
static u64 memory_current_read(struct cgroup_subsys_state *css,
struct cftype *cft)
{
@@ -5385,15 +5395,8 @@ static u64 memory_current_read(struct cgroup_subsys_state *css,
static int memory_min_show(struct seq_file *m, void *v)
{
- struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
- unsigned long min = READ_ONCE(memcg->memory.min);
-
- if (min == PAGE_COUNTER_MAX)
- seq_puts(m, "max\n");
- else
- seq_printf(m, "%llu\n", (u64)min * PAGE_SIZE);
-
- return 0;
+ return seq_puts_memcg_tunable(m,
+ READ_ONCE(mem_cgroup_from_seq(m)->memory.min));
}
static ssize_t memory_min_write(struct kernfs_open_file *of,
@@ -5415,15 +5418,8 @@ static ssize_t memory_min_write(struct kernfs_open_file *of,
static int memory_low_show(struct seq_file *m, void *v)
{
- struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
- unsigned long low = READ_ONCE(memcg->memory.low);
-
- if (low == PAGE_COUNTER_MAX)
- seq_puts(m, "max\n");
- else
- seq_printf(m, "%llu\n", (u64)low * PAGE_SIZE);
-
- return 0;
+ return seq_puts_memcg_tunable(m,
+ READ_ONCE(mem_cgroup_from_seq(m)->memory.low));
}
static ssize_t memory_low_write(struct kernfs_open_file *of,
@@ -5445,15 +5441,7 @@ static ssize_t memory_low_write(struct kernfs_open_file *of,
static int memory_high_show(struct seq_file *m, void *v)
{
- struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
- unsigned long high = READ_ONCE(memcg->high);
-
- if (high == PAGE_COUNTER_MAX)
- seq_puts(m, "max\n");
- else
- seq_printf(m, "%llu\n", (u64)high * PAGE_SIZE);
-
- return 0;
+ return seq_puts_memcg_tunable(m, READ_ONCE(mem_cgroup_from_seq(m)->high));
}
static ssize_t memory_high_write(struct kernfs_open_file *of,
@@ -5482,15 +5470,8 @@ static ssize_t memory_high_write(struct kernfs_open_file *of,
static int memory_max_show(struct seq_file *m, void *v)
{
- struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
- unsigned long max = READ_ONCE(memcg->memory.max);
-
- if (max == PAGE_COUNTER_MAX)
- seq_puts(m, "max\n");
- else
- seq_printf(m, "%llu\n", (u64)max * PAGE_SIZE);
-
- return 0;
+ return seq_puts_memcg_tunable(m,
+ READ_ONCE(mem_cgroup_from_seq(m)->memory.max));
}
static ssize_t memory_max_write(struct kernfs_open_file *of,
@@ -6622,15 +6603,8 @@ static u64 swap_current_read(struct cgroup_subsys_state *css,
static int swap_max_show(struct seq_file *m, void *v)
{
- struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
- unsigned long max = READ_ONCE(memcg->swap.max);
-
- if (max == PAGE_COUNTER_MAX)
- seq_puts(m, "max\n");
- else
- seq_printf(m, "%llu\n", (u64)max * PAGE_SIZE);
-
- return 0;
+ return seq_puts_memcg_tunable(m,
+ READ_ONCE(mem_cgroup_from_seq(m)->swap.max));
}
static ssize_t swap_max_write(struct kernfs_open_file *of,
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 2/2] mm: Extract memcg maxable seq_file logic to seq_show_memcg_tunable
2019-01-24 19:41 [PATCH 2/2] mm: Extract memcg maxable seq_file logic to seq_show_memcg_tunable Chris Down
@ 2019-01-24 21:18 ` Johannes Weiner
2019-01-25 7:24 ` Michal Hocko
1 sibling, 0 replies; 3+ messages in thread
From: Johannes Weiner @ 2019-01-24 21:18 UTC (permalink / raw)
To: Chris Down
Cc: Andrew Morton, Tejun Heo, Roman Gushchin, linux-kernel, cgroups,
linux-mm, kernel-team
On Thu, Jan 24, 2019 at 02:41:00PM -0500, Chris Down wrote:
> memcg has a significant number of files exposed to kernfs where their
> value is either exposed directly or is "max" in the case of
> PAGE_COUNTER_MAX.
>
> This patch makes this generic by providing a single function to do this
> work. In combination with the previous patch adding mem_cgroup_from_seq,
> this makes all of the seq_show feeder functions significantly more
> simple.
>
> Signed-off-by: Chris Down <chris@chrisdown.name>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Roman Gushchin <guro@fb.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: cgroups@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: kernel-team@fb.com
> ---
> mm/memcontrol.c | 64 +++++++++++++++----------------------------------
> 1 file changed, 19 insertions(+), 45 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 98aad31f5226..81b6f752471a 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5375,6 +5375,16 @@ static void mem_cgroup_bind(struct cgroup_subsys_state *root_css)
> root_mem_cgroup->use_hierarchy = false;
> }
>
> +static int seq_puts_memcg_tunable(struct seq_file *m, unsigned long value)
> +{
> + if (value == PAGE_COUNTER_MAX)
> + seq_puts(m, "max\n");
> + else
> + seq_printf(m, "%llu\n", (u64)value * PAGE_SIZE);
> +
> + return 0;
> +}
> +
> static u64 memory_current_read(struct cgroup_subsys_state *css,
> struct cftype *cft)
> {
> @@ -5385,15 +5395,8 @@ static u64 memory_current_read(struct cgroup_subsys_state *css,
>
> static int memory_min_show(struct seq_file *m, void *v)
> {
> - struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> - unsigned long min = READ_ONCE(memcg->memory.min);
> -
> - if (min == PAGE_COUNTER_MAX)
> - seq_puts(m, "max\n");
> - else
> - seq_printf(m, "%llu\n", (u64)min * PAGE_SIZE);
> -
> - return 0;
> + return seq_puts_memcg_tunable(m,
> + READ_ONCE(mem_cgroup_from_seq(m)->memory.min));
> }
Thanks for doing this.
The main thing that bugged me about the macro is that the data lookup
was split into two places:
DEFINE_MEMCG_THING(seq, memory_min_show, memory.min);
where memory.min is just a fragment of a designator and you have to
look at the macro to find out what exactly it gets turned into.
With this patch it's obvious, and you don't have to look at the
implementation of seq_puts_memcg_tunable() to understand what's being
printed out.
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 2/2] mm: Extract memcg maxable seq_file logic to seq_show_memcg_tunable
2019-01-24 19:41 [PATCH 2/2] mm: Extract memcg maxable seq_file logic to seq_show_memcg_tunable Chris Down
2019-01-24 21:18 ` Johannes Weiner
@ 2019-01-25 7:24 ` Michal Hocko
1 sibling, 0 replies; 3+ messages in thread
From: Michal Hocko @ 2019-01-25 7:24 UTC (permalink / raw)
To: Chris Down
Cc: Andrew Morton, Johannes Weiner, Tejun Heo, Roman Gushchin,
linux-kernel, cgroups, linux-mm, kernel-team
On Thu 24-01-19 14:41:00, Chris Down wrote:
> memcg has a significant number of files exposed to kernfs where their
> value is either exposed directly or is "max" in the case of
> PAGE_COUNTER_MAX.
>
> This patch makes this generic by providing a single function to do this
> work. In combination with the previous patch adding mem_cgroup_from_seq,
> this makes all of the seq_show feeder functions significantly more
> simple.
Yeah this is what I've had in mind when mentioning a helper in the
previous version of the patch. I like this more even though the
resulting savings are not that large.
> Signed-off-by: Chris Down <chris@chrisdown.name>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Roman Gushchin <guro@fb.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: cgroups@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: kernel-team@fb.com
Acked-by: Michal Hocko <mhocko@suse.com>
> ---
> mm/memcontrol.c | 64 +++++++++++++++----------------------------------
> 1 file changed, 19 insertions(+), 45 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 98aad31f5226..81b6f752471a 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5375,6 +5375,16 @@ static void mem_cgroup_bind(struct cgroup_subsys_state *root_css)
> root_mem_cgroup->use_hierarchy = false;
> }
>
> +static int seq_puts_memcg_tunable(struct seq_file *m, unsigned long value)
> +{
> + if (value == PAGE_COUNTER_MAX)
> + seq_puts(m, "max\n");
> + else
> + seq_printf(m, "%llu\n", (u64)value * PAGE_SIZE);
> +
> + return 0;
> +}
> +
> static u64 memory_current_read(struct cgroup_subsys_state *css,
> struct cftype *cft)
> {
> @@ -5385,15 +5395,8 @@ static u64 memory_current_read(struct cgroup_subsys_state *css,
>
> static int memory_min_show(struct seq_file *m, void *v)
> {
> - struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> - unsigned long min = READ_ONCE(memcg->memory.min);
> -
> - if (min == PAGE_COUNTER_MAX)
> - seq_puts(m, "max\n");
> - else
> - seq_printf(m, "%llu\n", (u64)min * PAGE_SIZE);
> -
> - return 0;
> + return seq_puts_memcg_tunable(m,
> + READ_ONCE(mem_cgroup_from_seq(m)->memory.min));
> }
>
> static ssize_t memory_min_write(struct kernfs_open_file *of,
> @@ -5415,15 +5418,8 @@ static ssize_t memory_min_write(struct kernfs_open_file *of,
>
> static int memory_low_show(struct seq_file *m, void *v)
> {
> - struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> - unsigned long low = READ_ONCE(memcg->memory.low);
> -
> - if (low == PAGE_COUNTER_MAX)
> - seq_puts(m, "max\n");
> - else
> - seq_printf(m, "%llu\n", (u64)low * PAGE_SIZE);
> -
> - return 0;
> + return seq_puts_memcg_tunable(m,
> + READ_ONCE(mem_cgroup_from_seq(m)->memory.low));
> }
>
> static ssize_t memory_low_write(struct kernfs_open_file *of,
> @@ -5445,15 +5441,7 @@ static ssize_t memory_low_write(struct kernfs_open_file *of,
>
> static int memory_high_show(struct seq_file *m, void *v)
> {
> - struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> - unsigned long high = READ_ONCE(memcg->high);
> -
> - if (high == PAGE_COUNTER_MAX)
> - seq_puts(m, "max\n");
> - else
> - seq_printf(m, "%llu\n", (u64)high * PAGE_SIZE);
> -
> - return 0;
> + return seq_puts_memcg_tunable(m, READ_ONCE(mem_cgroup_from_seq(m)->high));
> }
>
> static ssize_t memory_high_write(struct kernfs_open_file *of,
> @@ -5482,15 +5470,8 @@ static ssize_t memory_high_write(struct kernfs_open_file *of,
>
> static int memory_max_show(struct seq_file *m, void *v)
> {
> - struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> - unsigned long max = READ_ONCE(memcg->memory.max);
> -
> - if (max == PAGE_COUNTER_MAX)
> - seq_puts(m, "max\n");
> - else
> - seq_printf(m, "%llu\n", (u64)max * PAGE_SIZE);
> -
> - return 0;
> + return seq_puts_memcg_tunable(m,
> + READ_ONCE(mem_cgroup_from_seq(m)->memory.max));
> }
>
> static ssize_t memory_max_write(struct kernfs_open_file *of,
> @@ -6622,15 +6603,8 @@ static u64 swap_current_read(struct cgroup_subsys_state *css,
>
> static int swap_max_show(struct seq_file *m, void *v)
> {
> - struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> - unsigned long max = READ_ONCE(memcg->swap.max);
> -
> - if (max == PAGE_COUNTER_MAX)
> - seq_puts(m, "max\n");
> - else
> - seq_printf(m, "%llu\n", (u64)max * PAGE_SIZE);
> -
> - return 0;
> + return seq_puts_memcg_tunable(m,
> + READ_ONCE(mem_cgroup_from_seq(m)->swap.max));
> }
>
> static ssize_t swap_max_write(struct kernfs_open_file *of,
> --
> 2.20.1
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-25 7:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-24 19:41 [PATCH 2/2] mm: Extract memcg maxable seq_file logic to seq_show_memcg_tunable Chris Down
2019-01-24 21:18 ` Johannes Weiner
2019-01-25 7:24 ` 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.