Re: [PATCH 4/4] mm/memcg: refine mem_cgroup_threshold_ary->current_threshold calculation

From: Michal Hocko
Date: Tue Jan 11 2022 - 03:44:50 EST


On Tue 11-01-22 01:03:02, Wei Yang wrote:
> mem_cgroup_threshold_ary->current_threshold points to the last entry
> who's threshold is less or equal to usage.
>
> Instead of iterating entries to get the correct index, we can leverage
> primary->current_threshold to get it. If the threshold added is less or
> equal to usage, current_threshold should increase by one. Otherwise, it
> doesn't change.

Why do we want/need this change?

> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
> ---
> mm/memcontrol.c | 31 +++++++++++++++++--------------
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a504616f904a..ce7060907df2 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4161,7 +4161,7 @@ static int __mem_cgroup_usage_register_event(struct mem_cgroup *memcg,
> struct mem_cgroup_threshold_ary *new;
> unsigned long threshold;
> unsigned long usage;
> - int i, size, ret;
> + int size, ret;
>
> ret = page_counter_memparse(args, "-1", &threshold);
> if (ret)
> @@ -4193,9 +4193,13 @@ static int __mem_cgroup_usage_register_event(struct mem_cgroup *memcg,
> new->size = size;
>
> /* Copy thresholds (if any) to new array */
> - if (thresholds->primary)
> + if (thresholds->primary) {
> memcpy(new->entries, thresholds->primary->entries,
> flex_array_size(new, entries, size - 1));
> + new->current_threshold = thresholds->primary->current_threshold;
> + } else {
> + new->current_threshold = -1;
> + }
>
> /* Add new threshold */
> new->entries[size - 1].eventfd = eventfd;
> @@ -4205,18 +4209,17 @@ static int __mem_cgroup_usage_register_event(struct mem_cgroup *memcg,
> sort(new->entries, size, sizeof(*new->entries),
> compare_thresholds, NULL);
>
> - /* Find current threshold */
> - new->current_threshold = -1;
> - for (i = 0; i < size; i++) {
> - if (new->entries[i].threshold <= usage) {
> - /*
> - * new->current_threshold will not be used until
> - * rcu_assign_pointer(), so it's safe to increment
> - * it here.
> - */
> - ++new->current_threshold;
> - } else
> - break;
> + /*
> + * If the threshold added here is less or equal to usage, this means
> + * current_threshold need to increase by one.
> + */
> + if (threshold <= usage) {
> + /*
> + * new->current_threshold will not be used until
> + * rcu_assign_pointer(), so it's safe to increment
> + * it here.
> + */
> + new->current_threshold++;
> }
>
> /* Free old spare buffer and save old primary buffer as spare */
> --
> 2.33.1

--
Michal Hocko
SUSE Labs