2022-04-27 09:58:46

by Shakeel Butt

Subject: Re: [memcg] a8c49af3be: hackbench.throughput -13.7% regression

Hi Yin,

On Tue, Apr 26, 2022 at 7:53 PM Yin Fengwei <[email protected]> wrote:
>
[...]
>
> If I comment out one line added by the commit <a8c49af3be> :
> static void memcg_account_kmem(struct mem_cgroup *memcg, int nr_pages)
> {
> /* mod_memcg_state(memcg, MEMCG_KMEM, nr_pages); */ <--- comment out this line.

Can you to comment out the following block instead of the above line
and see if there is any impact?

> if (!cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
> if (nr_pages > 0)
> page_counter_charge(&memcg->kmem, nr_pages);
> else
> page_counter_uncharge(&memcg->kmem, -nr_pages);
> }
> }
>

2022-04-28 15:47:02

by Yin, Fengwei

Subject: Re: [memcg] a8c49af3be: hackbench.throughput -13.7% regression

Hi Shakeel,

On 4/27/2022 1:02 PM, Shakeel Butt wrote:
> Can you to comment out the following block instead of the above line
> and see if there is any impact?

Changed the code as following:

+static void memcg_account_kmem(struct mem_cgroup *memcg, int nr_pages)
+{
+ mod_memcg_state(memcg, MEMCG_KMEM, nr_pages);
+
+#if 0
+ if (!cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
+ if (nr_pages > 0)
+ page_counter_charge(&memcg->kmem, nr_pages);
+ else
+ page_counter_uncharge(&memcg->kmem, -nr_pages);
+ }
+#endif
+}
+

The hackbench test result is 147% better:

086f694a75e1a283 cee08cab8eefaaffdec62343e8d
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
8691:15 -52942% 750:6 dmesg.timestamp:last
:15 13% 2:6 last_state.booting
:15 13% 2:6 last_state.is_incomplete_run
1:15 -7% :6 kmsg.common_interrupt:#No_irq_handler_for_vector
2:15 -18% :6 kmsg.timestamp:common_interrupt:#No_irq_handler_for_vector
4656:15 -26039% 750:6 kmsg.timestamp:last
%stddev %change %stddev
\ | \
143619 ± 4% +147.0% 354807 ± 2% hackbench.throughput <------
475.91 ± 4% -58.8% 196.09 ± 2% hackbench.time.elapsed_time


Regards
Yin, Fengwei

2022-04-29 14:31:16

by Shakeel Butt

Subject: Re: [memcg] a8c49af3be: hackbench.throughput -13.7% regression

On Wed, Apr 27, 2022 at 10:12 PM Yin Fengwei <[email protected]> wrote:
>
> Hi Shakeel,
>
> On 4/27/2022 1:02 PM, Shakeel Butt wrote:
> > Can you to comment out the following block instead of the above line
> > and see if there is any impact?
>
> Changed the code as following:
>
> +static void memcg_account_kmem(struct mem_cgroup *memcg, int nr_pages)
> +{
> + mod_memcg_state(memcg, MEMCG_KMEM, nr_pages);
> +
> +#if 0
> + if (!cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
> + if (nr_pages > 0)
> + page_counter_charge(&memcg->kmem, nr_pages);
> + else
> + page_counter_uncharge(&memcg->kmem, -nr_pages);
> + }
> +#endif
> +}
> +
>
> The hackbench test result is 147% better:
>
> 086f694a75e1a283 cee08cab8eefaaffdec62343e8d
> ---------------- ---------------------------
> fail:runs %reproduction fail:runs
> | | |
> 8691:15 -52942% 750:6 dmesg.timestamp:last
> :15 13% 2:6 last_state.booting
> :15 13% 2:6 last_state.is_incomplete_run
> 1:15 -7% :6 kmsg.common_interrupt:#No_irq_handler_for_vector
> 2:15 -18% :6 kmsg.timestamp:common_interrupt:#No_irq_handler_for_vector
> 4656:15 -26039% 750:6 kmsg.timestamp:last
> %stddev %change %stddev
> \ | \
> 143619 ± 4% +147.0% 354807 ± 2% hackbench.throughput <------
> 475.91 ± 4% -58.8% 196.09 ± 2% hackbench.time.elapsed_time
>
>

Thanks for testing. We should spend time to deprecate memcg->kmem page
counter instead of optimizing MEMCG_KMEM.

Actually we can deprecate kmem page_counter usage, limit and failcnt
easily by replacing them with memcg_page_state(MEMCG_KMEM),
PAGE_COUNTER_MAX and 0. The kmem.max_usage_in_bytes would be a bit
tricky.