On 22.02.2021 21:40, Vasily Averin wrote:
> When mem_cgroup is removes mem_cgroup_css_offline() calls
> memcg_deactivate_kmem() which disables kmem accounting.
> If memcg still have some charged kmem final css_put is not called,
> and delayed till last kmem will be uncharged.
>
> Usually kmem is uncharged by using memcg_uncharge_kmem() which have
> according checks and if required calls final css_put().
>
> Though patch added to vz7.162.14 kernel
> "mm/memcg: Use per-cpu stock charges for ->kmem and ->cache counters"
> enabled kmem charge/uncharge in refill_stock()/drain_stock()
> without using of memcg_uncharge_kmem(), as result nobody called
> final css_put() after last kmem uncharge.
>
> This patch adds css_get/put for safe access to memcg in drain_stock(),
> and calls an additional css_put() after last kmem uncharge.
>
> Fixes: "mm/memcg: Use per-cpu stock charges for ->kmem and ->cache counters"
> https://bugs.openvz.org/browse/OVZ-7250
> Signed-off-by: Vasily Averin <[email protected]>
> ---
> mm/memcontrol.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index e0a4309..24e3bd7 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -439,6 +439,8 @@ enum {
> KMEM_ACCOUNTED_DEAD, /* dead memcg with pending kmem charges */
> };
>
> +static void memcg_kmem_release_css(struct mem_cgroup *memcg);
> +
> static struct mem_cgroup_per_node *
> mem_cgroup_nodeinfo(struct mem_cgroup *memcg, int nid)
> {
> @@ -2928,11 +2930,15 @@ static void drain_stock(struct memcg_stock_pcp *stock)
> {
> struct mem_cgroup *old = stock->cached;
> unsigned long nr_pages = stock->nr_pages + stock->cache_nr_pages +
> stock->kmem_nr_pages;
> + u64 kmem = 1;
> +
> + if (!old)
> + return;
>
> if (stock->cache_nr_pages)
> page_counter_uncharge(&old->cache, stock->cache_nr_pages);
> if (stock->kmem_nr_pages)
> - page_counter_uncharge(&old->kmem, stock->kmem_nr_pages);
> + kmem = page_counter_uncharge(&old->kmem, stock->kmem_nr_pages);
>
> if (nr_pages) {
> page_counter_uncharge(&old->memory, nr_pages);
> @@ -2942,6 +2948,9 @@ static void drain_stock(struct memcg_stock_pcp *stock)
> stock->kmem_nr_pages = 0;
> stock->cache_nr_pages = 0;
> }
> + css_put(&old->css);
css_put()/css_get() pair looks excess. What is your arguments, why do we need
them?
We have a problem with kmem only, while the rest of accounting looks safe:
they are safely uncharged from css_offline->mem_cgroup_reparent_charges(), and
they
never own css counter.
> + if (kmem == 0)
> + memcg_kmem_release_css(old);
> stock->cached = NULL;
> }
>
> @@ -2978,6 +2987,7 @@ static void refill_stock(struct mem_cgroup *memcg,
> unsigned int nr_pages,
>
> if (stock->cached != memcg) { /* reset if necessary */
> drain_stock(stock);
> + css_get(&memcg->css);
> stock->cached = memcg;
> }
>
> @@ -3608,10 +3618,12 @@ void memcg_uncharge_kmem(struct mem_cgroup *memcg,
> if (do_swap_account)
> page_counter_uncharge(&memcg->memsw, nr_pages);
>
> - /* Not down to 0 */
> - if (kmem)
> - return;
> + if (kmem == 0)
> + memcg_kmem_release_css(memcg);
> +}
>
> +static void memcg_kmem_release_css(struct mem_cgroup *memcg)
> +{
We should not mix refactoring and functional changes. This should go in a
separate patch.
> /*
> * Releases a reference taken in memcg_deactivate_kmem in case
> * this last uncharge is racing with the offlining code or it is
>
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
