It was found that a dying mm_struct where the owning task has exited
can stay on as active_mm of kernel threads as long as no other user
tasks run on those CPUs that use it as active_mm. This prolongs the
life time of dying mm holding up some resources that cannot be freed
on a mostly idle system.
Fix that by forcing the kernel threads to use init_mm as the active_mm
during a kernel thread to kernel thread transition if the previous
active_mm is dying (!mm_users). This will allows the freeing of resources
associated with the dying mm ASAP.
The presence of a kernel-to-kernel thread transition indicates that
the cpu is probably idling with no higher priority user task to run.
So the overhead of loading the mm_users cacheline should not really
matter in this case.
My testing on an x86 system showed that the mm_struct was freed within
seconds after the task exited instead of staying alive for minutes or
even longer on a mostly idle system before this patch.
Signed-off-by: Waiman Long <[email protected]>
---
kernel/sched/core.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 795077af4f1a..41997e676251 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3214,6 +3214,8 @@ static __always_inline struct rq *
context_switch(struct rq *rq, struct task_struct *prev,
struct task_struct *next, struct rq_flags *rf)
{
+ struct mm_struct *next_mm = next->mm;
+
prepare_task_switch(rq, prev, next);
/*
@@ -3229,8 +3231,22 @@ context_switch(struct rq *rq, struct task_struct *prev,
*
* kernel -> user switch + mmdrop() active
* user -> user switch
+ *
+ * kernel -> kernel and !prev->active_mm->mm_users:
+ * switch to init_mm + mmgrab() + mmdrop()
*/
- if (!next->mm) { // to kernel
+ if (!next_mm) { // to kernel
+ /*
+ * Checking is only done on kernel -> kernel transition
+ * to avoid any performance overhead while user tasks
+ * are running.
+ */
+ if (unlikely(!prev->mm &&
+ !atomic_read(&prev->active_mm->mm_users))) {
+ next_mm = next->active_mm = &init_mm;
+ mmgrab(next_mm);
+ goto mm_switch;
+ }
enter_lazy_tlb(prev->active_mm, next);
next->active_mm = prev->active_mm;
@@ -3239,6 +3255,7 @@ context_switch(struct rq *rq, struct task_struct *prev,
else
prev->active_mm = NULL;
} else { // to user
+mm_switch:
/*
* sys_membarrier() requires an smp_mb() between setting
* rq->curr and returning to userspace.
@@ -3248,7 +3265,7 @@ context_switch(struct rq *rq, struct task_struct *prev,
* finish_task_switch()'s mmdrop().
*/
- switch_mm_irqs_off(prev->active_mm, next->mm, next);
+ switch_mm_irqs_off(prev->active_mm, next_mm, next);
if (!prev->mm) { // from kernel
/* will mmdrop() in finish_task_switch(). */
--
2.18.1
On Mon, 2019-07-29 at 17:07 -0400, Waiman Long wrote:
> It was found that a dying mm_struct where the owning task has exited
> can stay on as active_mm of kernel threads as long as no other user
> tasks run on those CPUs that use it as active_mm. This prolongs the
> life time of dying mm holding up some resources that cannot be freed
> on a mostly idle system.
On what kernels does this happen?
Don't we explicitly flush all lazy TLB CPUs at exit
time, when we are about to free page tables?
Does this happen only on the CPU where the task in
question is exiting, or also on other CPUs?
If it is only on the CPU where the task is exiting,
would the TASK_DEAD handling in finish_task_switch()
be a better place to handle this?
> Fix that by forcing the kernel threads to use init_mm as the
> active_mm
> during a kernel thread to kernel thread transition if the previous
> active_mm is dying (!mm_users). This will allows the freeing of
> resources
> associated with the dying mm ASAP.
>
> The presence of a kernel-to-kernel thread transition indicates that
> the cpu is probably idling with no higher priority user task to run.
> So the overhead of loading the mm_users cacheline should not really
> matter in this case.
>
> My testing on an x86 system showed that the mm_struct was freed
> within
> seconds after the task exited instead of staying alive for minutes or
> even longer on a mostly idle system before this patch.
>
> Signed-off-by: Waiman Long <[email protected]>
> ---
> kernel/sched/core.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 795077af4f1a..41997e676251 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3214,6 +3214,8 @@ static __always_inline struct rq *
> context_switch(struct rq *rq, struct task_struct *prev,
> struct task_struct *next, struct rq_flags *rf)
> {
> + struct mm_struct *next_mm = next->mm;
> +
> prepare_task_switch(rq, prev, next);
>
> /*
> @@ -3229,8 +3231,22 @@ context_switch(struct rq *rq, struct
> task_struct *prev,
> *
> * kernel -> user switch + mmdrop() active
> * user -> user switch
> + *
> + * kernel -> kernel and !prev->active_mm->mm_users:
> + * switch to init_mm + mmgrab() + mmdrop()
> */
> - if (!next->mm) { // to kernel
> + if (!next_mm) { // to kernel
> + /*
> + * Checking is only done on kernel -> kernel transition
> + * to avoid any performance overhead while user tasks
> + * are running.
> + */
> + if (unlikely(!prev->mm &&
> + !atomic_read(&prev->active_mm->mm_users)))
> {
> + next_mm = next->active_mm = &init_mm;
> + mmgrab(next_mm);
> + goto mm_switch;
> + }
> enter_lazy_tlb(prev->active_mm, next);
>
> next->active_mm = prev->active_mm;
> @@ -3239,6 +3255,7 @@ context_switch(struct rq *rq, struct
> task_struct *prev,
> else
> prev->active_mm = NULL;
> } else { // to user
> +mm_switch:
> /*
> * sys_membarrier() requires an smp_mb() between
> setting
> * rq->curr and returning to userspace.
> @@ -3248,7 +3265,7 @@ context_switch(struct rq *rq, struct
> task_struct *prev,
> * finish_task_switch()'s mmdrop().
> */
>
> - switch_mm_irqs_off(prev->active_mm, next->mm, next);
> + switch_mm_irqs_off(prev->active_mm, next_mm, next);
>
> if (!prev->mm) { // from kernel
> /* will mmdrop() in finish_task_switch(). */
--
All Rights Reversed.
On 7/29/19 5:21 PM, Rik van Riel wrote:
> On Mon, 2019-07-29 at 17:07 -0400, Waiman Long wrote:
>> It was found that a dying mm_struct where the owning task has exited
>> can stay on as active_mm of kernel threads as long as no other user
>> tasks run on those CPUs that use it as active_mm. This prolongs the
>> life time of dying mm holding up some resources that cannot be freed
>> on a mostly idle system.
> On what kernels does this happen?
>
> Don't we explicitly flush all lazy TLB CPUs at exit
> time, when we are about to free page tables?
There are still a couple of calls that will be done until mm_count
reaches 0:
- mm_free_pgd(mm);
- destroy_context(mm);
- mmu_notifier_mm_destroy(mm);
- check_mm(mm);
- put_user_ns(mm->user_ns);
These are not big items, but holding it off for a long time is still not
a good thing.
> Does this happen only on the CPU where the task in
> question is exiting, or also on other CPUs?
What I have found is that a long running process on a mostly idle system
with many CPUs is likely to cycle through a lot of the CPUs during its
lifetime and leave behind its mm in the active_mm of those CPUs. My
2-socket test system have 96 logical CPUs. After running the test
program for a minute or so, it leaves behind its mm in about half of the
CPUs with a mm_count of 45 after exit. So the dying mm will stay until
all those 45 CPUs get new user tasks to run.
>
> If it is only on the CPU where the task is exiting,
> would the TASK_DEAD handling in finish_task_switch()
> be a better place to handle this?
I need to switch the mm off the dying one. mm switching is only done in
context_switch(). I don't think finish_task_switch() is the right place.
-Longman
On Mon 29-07-19 17:42:20, Waiman Long wrote:
> On 7/29/19 5:21 PM, Rik van Riel wrote:
> > On Mon, 2019-07-29 at 17:07 -0400, Waiman Long wrote:
> >> It was found that a dying mm_struct where the owning task has exited
> >> can stay on as active_mm of kernel threads as long as no other user
> >> tasks run on those CPUs that use it as active_mm. This prolongs the
> >> life time of dying mm holding up some resources that cannot be freed
> >> on a mostly idle system.
> > On what kernels does this happen?
> >
> > Don't we explicitly flush all lazy TLB CPUs at exit
> > time, when we are about to free page tables?
>
> There are still a couple of calls that will be done until mm_count
> reaches 0:
>
> - mm_free_pgd(mm);
> - destroy_context(mm);
> - mmu_notifier_mm_destroy(mm);
> - check_mm(mm);
> - put_user_ns(mm->user_ns);
>
> These are not big items, but holding it off for a long time is still not
> a good thing.
It would be helpful to give a ball park estimation of how much that
actually is. If we are talking about few pages worth of pages per idle
cpu in the worst case then I am not sure we want to find an elaborate
way around that. We are quite likely having more in per-cpu caches in
different subsystems already. It is also quite likely that large
machines with many CPUs will have a lot of memory as well.
--
Michal Hocko
SUSE Labs
On Mon, Jul 29, 2019 at 05:07:28PM -0400, Waiman Long wrote:
> It was found that a dying mm_struct where the owning task has exited
> can stay on as active_mm of kernel threads as long as no other user
> tasks run on those CPUs that use it as active_mm. This prolongs the
> life time of dying mm holding up some resources that cannot be freed
> on a mostly idle system.
>
> Fix that by forcing the kernel threads to use init_mm as the active_mm
> during a kernel thread to kernel thread transition if the previous
> active_mm is dying (!mm_users). This will allows the freeing of resources
> associated with the dying mm ASAP.
>
> The presence of a kernel-to-kernel thread transition indicates that
> the cpu is probably idling with no higher priority user task to run.
> So the overhead of loading the mm_users cacheline should not really
> matter in this case.
>
> My testing on an x86 system showed that the mm_struct was freed within
> seconds after the task exited instead of staying alive for minutes or
> even longer on a mostly idle system before this patch.
>
> Signed-off-by: Waiman Long <[email protected]>
> ---
> kernel/sched/core.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 795077af4f1a..41997e676251 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3214,6 +3214,8 @@ static __always_inline struct rq *
> context_switch(struct rq *rq, struct task_struct *prev,
> struct task_struct *next, struct rq_flags *rf)
> {
> + struct mm_struct *next_mm = next->mm;
> +
> prepare_task_switch(rq, prev, next);
>
> /*
> @@ -3229,8 +3231,22 @@ context_switch(struct rq *rq, struct task_struct *prev,
> *
> * kernel -> user switch + mmdrop() active
> * user -> user switch
> + *
> + * kernel -> kernel and !prev->active_mm->mm_users:
> + * switch to init_mm + mmgrab() + mmdrop()
> */
> - if (!next->mm) { // to kernel
> + if (!next_mm) { // to kernel
> + /*
> + * Checking is only done on kernel -> kernel transition
> + * to avoid any performance overhead while user tasks
> + * are running.
> + */
> + if (unlikely(!prev->mm &&
> + !atomic_read(&prev->active_mm->mm_users))) {
> + next_mm = next->active_mm = &init_mm;
> + mmgrab(next_mm);
> + goto mm_switch;
> + }
> enter_lazy_tlb(prev->active_mm, next);
>
> next->active_mm = prev->active_mm;
So I _really_ hate this complication. I'm thinking if you really care
about this the time is much better spend getting rid of the active_mm
tracking for x86 entirely.
On 7/30/19 4:43 AM, Peter Zijlstra wrote:
> On Mon, Jul 29, 2019 at 05:07:28PM -0400, Waiman Long wrote:
>> It was found that a dying mm_struct where the owning task has exited
>> can stay on as active_mm of kernel threads as long as no other user
>> tasks run on those CPUs that use it as active_mm. This prolongs the
>> life time of dying mm holding up some resources that cannot be freed
>> on a mostly idle system.
>>
>> Fix that by forcing the kernel threads to use init_mm as the active_mm
>> during a kernel thread to kernel thread transition if the previous
>> active_mm is dying (!mm_users). This will allows the freeing of resources
>> associated with the dying mm ASAP.
>>
>> The presence of a kernel-to-kernel thread transition indicates that
>> the cpu is probably idling with no higher priority user task to run.
>> So the overhead of loading the mm_users cacheline should not really
>> matter in this case.
>>
>> My testing on an x86 system showed that the mm_struct was freed within
>> seconds after the task exited instead of staying alive for minutes or
>> even longer on a mostly idle system before this patch.
>>
>> Signed-off-by: Waiman Long <[email protected]>
>> ---
>> kernel/sched/core.c | 21 +++++++++++++++++++--
>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 795077af4f1a..41997e676251 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -3214,6 +3214,8 @@ static __always_inline struct rq *
>> context_switch(struct rq *rq, struct task_struct *prev,
>> struct task_struct *next, struct rq_flags *rf)
>> {
>> + struct mm_struct *next_mm = next->mm;
>> +
>> prepare_task_switch(rq, prev, next);
>>
>> /*
>> @@ -3229,8 +3231,22 @@ context_switch(struct rq *rq, struct task_struct *prev,
>> *
>> * kernel -> user switch + mmdrop() active
>> * user -> user switch
>> + *
>> + * kernel -> kernel and !prev->active_mm->mm_users:
>> + * switch to init_mm + mmgrab() + mmdrop()
>> */
>> - if (!next->mm) { // to kernel
>> + if (!next_mm) { // to kernel
>> + /*
>> + * Checking is only done on kernel -> kernel transition
>> + * to avoid any performance overhead while user tasks
>> + * are running.
>> + */
>> + if (unlikely(!prev->mm &&
>> + !atomic_read(&prev->active_mm->mm_users))) {
>> + next_mm = next->active_mm = &init_mm;
>> + mmgrab(next_mm);
>> + goto mm_switch;
>> + }
>> enter_lazy_tlb(prev->active_mm, next);
>>
>> next->active_mm = prev->active_mm;
> So I _really_ hate this complication. I'm thinking if you really care
> about this the time is much better spend getting rid of the active_mm
> tracking for x86 entirely.
>
That is fine. I won't pursue further. I will take a look at your
suggestion when I have time, but it will probably be a while :-)
Cheers,
Longman
On 7/30/19 3:24 AM, Michal Hocko wrote:
> On Mon 29-07-19 17:42:20, Waiman Long wrote:
>> On 7/29/19 5:21 PM, Rik van Riel wrote:
>>> On Mon, 2019-07-29 at 17:07 -0400, Waiman Long wrote:
>>>> It was found that a dying mm_struct where the owning task has exited
>>>> can stay on as active_mm of kernel threads as long as no other user
>>>> tasks run on those CPUs that use it as active_mm. This prolongs the
>>>> life time of dying mm holding up some resources that cannot be freed
>>>> on a mostly idle system.
>>> On what kernels does this happen?
>>>
>>> Don't we explicitly flush all lazy TLB CPUs at exit
>>> time, when we are about to free page tables?
>> There are still a couple of calls that will be done until mm_count
>> reaches 0:
>>
>> - mm_free_pgd(mm);
>> - destroy_context(mm);
>> - mmu_notifier_mm_destroy(mm);
>> - check_mm(mm);
>> - put_user_ns(mm->user_ns);
>>
>> These are not big items, but holding it off for a long time is still not
>> a good thing.
> It would be helpful to give a ball park estimation of how much that
> actually is. If we are talking about few pages worth of pages per idle
> cpu in the worst case then I am not sure we want to find an elaborate
> way around that. We are quite likely having more in per-cpu caches in
> different subsystems already. It is also quite likely that large
> machines with many CPUs will have a lot of memory as well.
I think they are relatively small. So I am not going to pursue it
further at this point.
Cheers,
Longman
On 7/31/19 11:07 AM, Rik van Riel wrote:
> On Wed, 2019-07-31 at 10:15 -0400, Waiman Long wrote:
>> On 7/31/19 9:48 AM, Rik van Riel wrote:
>>> On Tue, 2019-07-30 at 17:01 -0400, Waiman Long wrote:
>>>> On 7/29/19 8:26 PM, Rik van Riel wrote:
>>>>> On Mon, 2019-07-29 at 17:42 -0400, Waiman Long wrote:
>>>>>
>>>>>> What I have found is that a long running process on a mostly
>>>>>> idle
>>>>>> system
>>>>>> with many CPUs is likely to cycle through a lot of the CPUs
>>>>>> during
>>>>>> its
>>>>>> lifetime and leave behind its mm in the active_mm of those
>>>>>> CPUs. My
>>>>>> 2-socket test system have 96 logical CPUs. After running the
>>>>>> test
>>>>>> program for a minute or so, it leaves behind its mm in about
>>>>>> half
>>>>>> of
>>>>>> the
>>>>>> CPUs with a mm_count of 45 after exit. So the dying mm will
>>>>>> stay
>>>>>> until
>>>>>> all those 45 CPUs get new user tasks to run.
>>>>> OK. On what kernel are you seeing this?
>>>>>
>>>>> On current upstream, the code in native_flush_tlb_others()
>>>>> will send a TLB flush to every CPU in mm_cpumask() if page
>>>>> table pages have been freed.
>>>>>
>>>>> That should cause the lazy TLB CPUs to switch to init_mm
>>>>> when the exit->zap_page_range path gets to the point where
>>>>> it frees page tables.
>>>>>
>>>> I was using the latest upstream 5.3-rc2 kernel. It may be the
>>>> case
>>>> that
>>>> the mm has been switched, but the mm_count field of the active_mm
>>>> of
>>>> the
>>>> kthread is not being decremented until a user task runs on a CPU.
>>> Is that something we could fix from the TLB flushing
>>> code?
>>>
>>> When switching to init_mm, drop the refcount on the
>>> lazy mm?
>>>
>>> That way that overhead is not added to the context
>>> switching code.
>> I have thought about that. That will require changing the active_mm
>> of
>> the current task to point to init_mm, for example. Since TLB flush is
>> done in interrupt context, proper coordination between interrupt and
>> process context will require some atomic instruction which will
>> defect
>> the purpose.
> Would it be possible to work around that by scheduling
> a work item that drops the active_mm?
>
> After all, a work item runs in a kernel thread, so by
> the time the work item is run, either the kernel will
> still be running the mm you want to get rid of as
> active_mm, or it will have already gotten rid of it
> earlier.
Yes, that may work.
Thanks,
Longman