[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20200913203708.ec97aab4ec42b22cac532f38@linux-foundation.org>
Date: Sun, 13 Sep 2020 20:37:08 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: "Isaac J. Manjarres" <isaacm@...eaurora.org>
Cc: linux-kernel@...r.kernel.org, christian.brauner@...ntu.com,
mingo@...nel.org, peterz@...radead.org, ebiederm@...ssion.com,
esyr@...hat.com, tglx@...utronix.de, christian@...lner.me,
areber@...hat.com, shakeelb@...gle.com, cyphar@...har.com,
psodagud@...eaurora.org, pratikp@...eaurora.org
Subject: Re: [RFC PATCH] fork: Free per-cpu cached vmalloc'ed thread stacks
with
On Sat, 5 Sep 2020 00:12:29 +0000 "Isaac J. Manjarres" <isaacm@...eaurora.org> wrote:
> The per-cpu cached vmalloc'ed stacks are currently freed in the
> CPU hotplug teardown path by the free_vm_stack_cache() callback,
> which invokes vfree(), which may result in purging the list of
> lazily freed vmap areas.
>
> Purging all of the lazily freed vmap areas can take a long time
> when the list of vmap areas is large. This is problematic, as
> free_vm_stack_cache() is invoked prior to the offline CPU's timers
> being migrated. This is not desirable as it can lead to timer
> migration delays in the CPU hotplug teardown path, and timer callbacks
> will be invoked long after the timer has expired.
>
> For example, on a system that has only one online CPU (CPU 1) that is
> running a heavy workload, and another CPU that is being offlined,
> the online CPU will invoke free_vm_stack_cache() to free the cached
> vmalloc'ed stacks for the CPU being offlined. When there are 2702
> vmap areas that total to 13498 pages, free_vm_stack_cache() takes
> over 2 seconds to execute:
>
> [001] 399.335808: cpuhp_enter: cpu: 0005 target: 0 step: 67 (free_vm_stack_cache)
>
> /* The first vmap area to be freed */
> [001] 399.337157: __purge_vmap_area_lazy: [0:2702] 0xffffffc033da8000 - 0xffffffc033dad000 (5 : 13498)
>
> /* After two seconds */
> [001] 401.528010: __purge_vmap_area_lazy: [1563:2702] 0xffffffc02fe10000 - 0xffffffc02fe15000 (5 : 5765)
>
> Instead of freeing the per-cpu cached vmalloc'ed stacks synchronously
> with respect to the CPU hotplug teardown state machine, free them
> asynchronously to help move along the CPU hotplug teardown state machine
> quickly.
>
> ...
>
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -202,7 +202,7 @@ static int free_vm_stack_cache(unsigned int cpu)
> if (!vm_stack)
> continue;
>
> - vfree(vm_stack->addr);
> + vfree_atomic(vm_stack->addr);
> cached_vm_stacks[i] = NULL;
> }
I guess that makes sense, although perhaps we shouldn't be permitting
purge_list to get so large - such latency issues will still appear in
other situations.
If we go with this fix-just-fork approach, can we please have a comment
in there explaining why vfree_atomic() is being used?
Powered by blists - more mailing lists