2020-09-08 10:26:52

by Alex Shi

[permalink] [raw]
Subject: [PATCH] mm/memcg: rename watermark as max_usage

The page_counter's watermark used to show as max_usage_in_bytes in memory
cgroup. named as watermark is a kind of misleadking. So, let's rename it
as it usage.

Signed-off-by: Alex Shi <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Vladimir Davydov <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
include/linux/page_counter.h | 6 +++---
mm/hugetlb_cgroup.c | 8 ++++----
mm/memcontrol.c | 4 ++--
mm/page_counter.c | 12 ++++++------
4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/linux/page_counter.h b/include/linux/page_counter.h
index 85bd413e784e..813f2da26c36 100644
--- a/include/linux/page_counter.h
+++ b/include/linux/page_counter.h
@@ -25,7 +25,7 @@ struct page_counter {
atomic_long_t children_low_usage;

/* legacy */
- unsigned long watermark;
+ unsigned long max_usage; /* max_usage_in_bytes */
unsigned long failcnt;
};

@@ -67,9 +67,9 @@ static inline void page_counter_set_high(struct page_counter *counter,
int page_counter_memparse(const char *buf, const char *max,
unsigned long *nr_pages);

-static inline void page_counter_reset_watermark(struct page_counter *counter)
+static inline void page_counter_reset_max_usage(struct page_counter *counter)
{
- counter->watermark = page_counter_read(counter);
+ counter->max_usage = page_counter_read(counter);
}

#endif /* _LINUX_PAGE_COUNTER_H */
diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
index 1f87aec9ab5c..c7484f5eb8ef 100644
--- a/mm/hugetlb_cgroup.c
+++ b/mm/hugetlb_cgroup.c
@@ -437,9 +437,9 @@ static u64 hugetlb_cgroup_read_u64(struct cgroup_subsys_state *css,
case RES_RSVD_LIMIT:
return (u64)rsvd_counter->max * PAGE_SIZE;
case RES_MAX_USAGE:
- return (u64)counter->watermark * PAGE_SIZE;
+ return (u64)counter->max_usage * PAGE_SIZE;
case RES_RSVD_MAX_USAGE:
- return (u64)rsvd_counter->watermark * PAGE_SIZE;
+ return (u64)rsvd_counter->max_usage * PAGE_SIZE;
case RES_FAILCNT:
return counter->failcnt;
case RES_RSVD_FAILCNT:
@@ -553,10 +553,10 @@ static ssize_t hugetlb_cgroup_reset(struct kernfs_open_file *of,

switch (MEMFILE_ATTR(of_cft(of)->private)) {
case RES_MAX_USAGE:
- page_counter_reset_watermark(counter);
+ page_counter_reset_max_usage(counter);
break;
case RES_RSVD_MAX_USAGE:
- page_counter_reset_watermark(rsvd_counter);
+ page_counter_reset_max_usage(rsvd_counter);
break;
case RES_FAILCNT:
counter->failcnt = 0;
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8c74f1200261..21b73de53073 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3559,7 +3559,7 @@ static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css,
case RES_LIMIT:
return (u64)counter->max * PAGE_SIZE;
case RES_MAX_USAGE:
- return (u64)counter->watermark * PAGE_SIZE;
+ return (u64)counter->max_usage * PAGE_SIZE;
case RES_FAILCNT:
return counter->failcnt;
case RES_SOFT_LIMIT:
@@ -3839,7 +3839,7 @@ static ssize_t mem_cgroup_reset(struct kernfs_open_file *of, char *buf,

switch (MEMFILE_ATTR(of_cft(of)->private)) {
case RES_MAX_USAGE:
- page_counter_reset_watermark(counter);
+ page_counter_reset_max_usage(counter);
break;
case RES_FAILCNT:
counter->failcnt = 0;
diff --git a/mm/page_counter.c b/mm/page_counter.c
index afe22ad335cc..d88ee074f4a6 100644
--- a/mm/page_counter.c
+++ b/mm/page_counter.c
@@ -75,10 +75,10 @@ void page_counter_charge(struct page_counter *counter, unsigned long nr_pages)
propagate_protected_usage(c, new);
/*
* This is indeed racy, but we can live with some
- * inaccuracy in the watermark.
+ * inaccuracy in the max_usage.
*/
- if (new > READ_ONCE(c->watermark))
- WRITE_ONCE(c->watermark, new);
+ if (new > READ_ONCE(c->max_usage))
+ WRITE_ONCE(c->max_usage, new);
}
}

@@ -129,10 +129,10 @@ bool page_counter_try_charge(struct page_counter *counter,
propagate_protected_usage(c, new);
/*
* Just like with failcnt, we can live with some
- * inaccuracy in the watermark.
+ * inaccuracy in the max_usage.
*/
- if (new > READ_ONCE(c->watermark))
- WRITE_ONCE(c->watermark, new);
+ if (new > READ_ONCE(c->max_usage))
+ WRITE_ONCE(c->max_usage, new);
}
return true;

--
1.8.3.1


2020-09-08 10:43:02

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH] mm/memcg: rename watermark as max_usage

On Tue 08-09-20 18:25:21, Alex Shi wrote:
> The page_counter's watermark used to show as max_usage_in_bytes in memory
> cgroup. named as watermark is a kind of misleadking. So, let's rename it
> as it usage.

Is this really necessary? This just adds a code churn for something that
is highly subjective.

> Signed-off-by: Alex Shi <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Johannes Weiner <[email protected]>
> Cc: Michal Hocko <[email protected]>
> Cc: Vladimir Davydov <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> include/linux/page_counter.h | 6 +++---
> mm/hugetlb_cgroup.c | 8 ++++----
> mm/memcontrol.c | 4 ++--
> mm/page_counter.c | 12 ++++++------
> 4 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/page_counter.h b/include/linux/page_counter.h
> index 85bd413e784e..813f2da26c36 100644
> --- a/include/linux/page_counter.h
> +++ b/include/linux/page_counter.h
> @@ -25,7 +25,7 @@ struct page_counter {
> atomic_long_t children_low_usage;
>
> /* legacy */
> - unsigned long watermark;
> + unsigned long max_usage; /* max_usage_in_bytes */
> unsigned long failcnt;
> };
>
> @@ -67,9 +67,9 @@ static inline void page_counter_set_high(struct page_counter *counter,
> int page_counter_memparse(const char *buf, const char *max,
> unsigned long *nr_pages);
>
> -static inline void page_counter_reset_watermark(struct page_counter *counter)
> +static inline void page_counter_reset_max_usage(struct page_counter *counter)
> {
> - counter->watermark = page_counter_read(counter);
> + counter->max_usage = page_counter_read(counter);
> }
>
> #endif /* _LINUX_PAGE_COUNTER_H */
> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> index 1f87aec9ab5c..c7484f5eb8ef 100644
> --- a/mm/hugetlb_cgroup.c
> +++ b/mm/hugetlb_cgroup.c
> @@ -437,9 +437,9 @@ static u64 hugetlb_cgroup_read_u64(struct cgroup_subsys_state *css,
> case RES_RSVD_LIMIT:
> return (u64)rsvd_counter->max * PAGE_SIZE;
> case RES_MAX_USAGE:
> - return (u64)counter->watermark * PAGE_SIZE;
> + return (u64)counter->max_usage * PAGE_SIZE;
> case RES_RSVD_MAX_USAGE:
> - return (u64)rsvd_counter->watermark * PAGE_SIZE;
> + return (u64)rsvd_counter->max_usage * PAGE_SIZE;
> case RES_FAILCNT:
> return counter->failcnt;
> case RES_RSVD_FAILCNT:
> @@ -553,10 +553,10 @@ static ssize_t hugetlb_cgroup_reset(struct kernfs_open_file *of,
>
> switch (MEMFILE_ATTR(of_cft(of)->private)) {
> case RES_MAX_USAGE:
> - page_counter_reset_watermark(counter);
> + page_counter_reset_max_usage(counter);
> break;
> case RES_RSVD_MAX_USAGE:
> - page_counter_reset_watermark(rsvd_counter);
> + page_counter_reset_max_usage(rsvd_counter);
> break;
> case RES_FAILCNT:
> counter->failcnt = 0;
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 8c74f1200261..21b73de53073 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3559,7 +3559,7 @@ static u64 mem_cgroup_read_u64(struct cgroup_subsys_state *css,
> case RES_LIMIT:
> return (u64)counter->max * PAGE_SIZE;
> case RES_MAX_USAGE:
> - return (u64)counter->watermark * PAGE_SIZE;
> + return (u64)counter->max_usage * PAGE_SIZE;
> case RES_FAILCNT:
> return counter->failcnt;
> case RES_SOFT_LIMIT:
> @@ -3839,7 +3839,7 @@ static ssize_t mem_cgroup_reset(struct kernfs_open_file *of, char *buf,
>
> switch (MEMFILE_ATTR(of_cft(of)->private)) {
> case RES_MAX_USAGE:
> - page_counter_reset_watermark(counter);
> + page_counter_reset_max_usage(counter);
> break;
> case RES_FAILCNT:
> counter->failcnt = 0;
> diff --git a/mm/page_counter.c b/mm/page_counter.c
> index afe22ad335cc..d88ee074f4a6 100644
> --- a/mm/page_counter.c
> +++ b/mm/page_counter.c
> @@ -75,10 +75,10 @@ void page_counter_charge(struct page_counter *counter, unsigned long nr_pages)
> propagate_protected_usage(c, new);
> /*
> * This is indeed racy, but we can live with some
> - * inaccuracy in the watermark.
> + * inaccuracy in the max_usage.
> */
> - if (new > READ_ONCE(c->watermark))
> - WRITE_ONCE(c->watermark, new);
> + if (new > READ_ONCE(c->max_usage))
> + WRITE_ONCE(c->max_usage, new);
> }
> }
>
> @@ -129,10 +129,10 @@ bool page_counter_try_charge(struct page_counter *counter,
> propagate_protected_usage(c, new);
> /*
> * Just like with failcnt, we can live with some
> - * inaccuracy in the watermark.
> + * inaccuracy in the max_usage.
> */
> - if (new > READ_ONCE(c->watermark))
> - WRITE_ONCE(c->watermark, new);
> + if (new > READ_ONCE(c->max_usage))
> + WRITE_ONCE(c->max_usage, new);
> }
> return true;
>
> --
> 1.8.3.1

--
Michal Hocko
SUSE Labs