* [PATCH] powerpc/64s/radix: reset mm_cpumask for single thread process when possible
@ 2018-05-09 6:56 Nicholas Piggin
2018-05-09 8:23 ` Balbir Singh
2018-05-09 8:31 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 5+ messages in thread
From: Nicholas Piggin @ 2018-05-09 6:56 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
When a single-threaded process has a non-local mm_cpumask and requires
a full PID tlbie invalidation, use that as an opportunity to reset the
cpumask back to the current CPU we're running on.
No other thread can concurrently switch to this mm, because it must
have had a reference on mm_users before it could use_mm. mm_users can
be asynchronously incremented e.g., by mmget_not_zero, but those users
must not be doing use_mm.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/mmu_context.h | 19 +++++++++
arch/powerpc/include/asm/tlb.h | 8 ++++
arch/powerpc/mm/tlb-radix.c | 57 +++++++++++++++++++-------
3 files changed, 70 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 1835ca1505d6..df12a994529f 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -6,6 +6,7 @@
#include <linux/kernel.h>
#include <linux/mm.h>
#include <linux/sched.h>
+#include <linux/sched/mm.h>
#include <linux/spinlock.h>
#include <asm/mmu.h>
#include <asm/cputable.h>
@@ -201,6 +202,24 @@ static inline void activate_mm(struct mm_struct *prev, struct mm_struct *next)
static inline void enter_lazy_tlb(struct mm_struct *mm,
struct task_struct *tsk)
{
+#ifdef CONFIG_PPC_BOOK3S_64
+ /*
+ * Under radix, we do not want to keep lazy PIDs around because
+ * even if the CPU does not access userspace, it can still bring
+ * in translations through speculation and prefetching.
+ *
+ * Switching away here allows us to trim back the mm_cpumask in
+ * cases where we know the process is not running on some CPUs
+ * (see mm/tlb-radix.c).
+ */
+ if (radix_enabled() && mm != &init_mm) {
+ mmgrab(&init_mm);
+ tsk->active_mm = &init_mm;
+ switch_mm_irqs_off(mm, tsk->active_mm, tsk);
+ mmdrop(mm);
+ }
+#endif
+
/* 64-bit Book3E keeps track of current PGD in the PACA */
#ifdef CONFIG_PPC_BOOK3E_64
get_paca()->pgd = NULL;
diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
index a7eabff27a0f..006fce98c403 100644
--- a/arch/powerpc/include/asm/tlb.h
+++ b/arch/powerpc/include/asm/tlb.h
@@ -76,6 +76,14 @@ static inline int mm_is_thread_local(struct mm_struct *mm)
return false;
return cpumask_test_cpu(smp_processor_id(), mm_cpumask(mm));
}
+static inline void mm_reset_thread_local(struct mm_struct *mm)
+{
+ WARN_ON(atomic_read(&mm->context.copros) > 0);
+ WARN_ON(!(atomic_read(&mm->mm_users) == 1 && current->mm == mm));
+ atomic_set(&mm->context.active_cpus, 1);
+ cpumask_clear(mm_cpumask(mm));
+ cpumask_set_cpu(smp_processor_id(), mm_cpumask(mm));
+}
#else /* CONFIG_PPC_BOOK3S_64 */
static inline int mm_is_thread_local(struct mm_struct *mm)
{
diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index 5ac3206c51cc..d5593a78702a 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -504,6 +504,15 @@ void radix__local_flush_tlb_page(struct vm_area_struct *vma, unsigned long vmadd
}
EXPORT_SYMBOL(radix__local_flush_tlb_page);
+static bool mm_is_singlethreaded(struct mm_struct *mm)
+{
+ if (atomic_read(&mm->context.copros) > 0)
+ return false;
+ if (atomic_read(&mm->mm_users) == 1 && current->mm == mm)
+ return true;
+ return false;
+}
+
static bool mm_needs_flush_escalation(struct mm_struct *mm)
{
/*
@@ -511,7 +520,9 @@ static bool mm_needs_flush_escalation(struct mm_struct *mm)
* caching PTEs and not flushing them properly when
* RIC = 0 for a PID/LPID invalidate
*/
- return atomic_read(&mm->context.copros) != 0;
+ if (atomic_read(&mm->context.copros) > 0)
+ return true;
+ return false;
}
#ifdef CONFIG_SMP
@@ -525,12 +536,17 @@ void radix__flush_tlb_mm(struct mm_struct *mm)
preempt_disable();
if (!mm_is_thread_local(mm)) {
- if (mm_needs_flush_escalation(mm))
+ if (mm_is_singlethreaded(mm)) {
_tlbie_pid(pid, RIC_FLUSH_ALL);
- else
+ mm_reset_thread_local(mm);
+ } else if (mm_needs_flush_escalation(mm)) {
+ _tlbie_pid(pid, RIC_FLUSH_ALL);
+ } else {
_tlbie_pid(pid, RIC_FLUSH_TLB);
- } else
+ }
+ } else {
_tlbiel_pid(pid, RIC_FLUSH_TLB);
+ }
preempt_enable();
}
EXPORT_SYMBOL(radix__flush_tlb_mm);
@@ -544,10 +560,13 @@ void radix__flush_all_mm(struct mm_struct *mm)
return;
preempt_disable();
- if (!mm_is_thread_local(mm))
+ if (!mm_is_thread_local(mm)) {
_tlbie_pid(pid, RIC_FLUSH_ALL);
- else
+ if (mm_is_singlethreaded(mm))
+ mm_reset_thread_local(mm);
+ } else {
_tlbiel_pid(pid, RIC_FLUSH_ALL);
+ }
preempt_enable();
}
EXPORT_SYMBOL(radix__flush_all_mm);
@@ -644,10 +663,14 @@ void radix__flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
if (local) {
_tlbiel_pid(pid, RIC_FLUSH_TLB);
} else {
- if (mm_needs_flush_escalation(mm))
+ if (mm_is_singlethreaded(mm)) {
+ _tlbie_pid(pid, RIC_FLUSH_ALL);
+ mm_reset_thread_local(mm);
+ } else if (mm_needs_flush_escalation(mm)) {
_tlbie_pid(pid, RIC_FLUSH_ALL);
- else
+ } else {
_tlbie_pid(pid, RIC_FLUSH_TLB);
+ }
}
} else {
bool hflush = false;
@@ -802,13 +825,19 @@ static inline void __radix__flush_tlb_range_psize(struct mm_struct *mm,
}
if (full) {
- if (!local && mm_needs_flush_escalation(mm))
- also_pwc = true;
-
- if (local)
+ if (local) {
_tlbiel_pid(pid, also_pwc ? RIC_FLUSH_ALL : RIC_FLUSH_TLB);
- else
- _tlbie_pid(pid, also_pwc ? RIC_FLUSH_ALL: RIC_FLUSH_TLB);
+ } else {
+ if (mm_is_singlethreaded(mm)) {
+ _tlbie_pid(pid, RIC_FLUSH_ALL);
+ mm_reset_thread_local(mm);
+ } else {
+ if (mm_needs_flush_escalation(mm))
+ also_pwc = true;
+
+ _tlbie_pid(pid, also_pwc ? RIC_FLUSH_ALL : RIC_FLUSH_TLB);
+ }
+ }
} else {
if (local)
_tlbiel_va_range(start, end, pid, page_size, psize, also_pwc);
--
2.17.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc/64s/radix: reset mm_cpumask for single thread process when possible
2018-05-09 6:56 [PATCH] powerpc/64s/radix: reset mm_cpumask for single thread process when possible Nicholas Piggin
@ 2018-05-09 8:23 ` Balbir Singh
2018-05-09 11:22 ` Nicholas Piggin
2018-05-09 8:31 ` Benjamin Herrenschmidt
1 sibling, 1 reply; 5+ messages in thread
From: Balbir Singh @ 2018-05-09 8:23 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
On Wed, May 9, 2018 at 4:56 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> When a single-threaded process has a non-local mm_cpumask and requires
> a full PID tlbie invalidation, use that as an opportunity to reset the
> cpumask back to the current CPU we're running on.
>
> No other thread can concurrently switch to this mm, because it must
> have had a reference on mm_users before it could use_mm. mm_users can
> be asynchronously incremented e.g., by mmget_not_zero, but those users
> must not be doing use_mm.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/mmu_context.h | 19 +++++++++
> arch/powerpc/include/asm/tlb.h | 8 ++++
> arch/powerpc/mm/tlb-radix.c | 57 +++++++++++++++++++-------
> 3 files changed, 70 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 1835ca1505d6..df12a994529f 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -6,6 +6,7 @@
> #include <linux/kernel.h>
> #include <linux/mm.h>
> #include <linux/sched.h>
> +#include <linux/sched/mm.h>
> #include <linux/spinlock.h>
> #include <asm/mmu.h>
> #include <asm/cputable.h>
> @@ -201,6 +202,24 @@ static inline void activate_mm(struct mm_struct *prev, struct mm_struct *next)
> static inline void enter_lazy_tlb(struct mm_struct *mm,
> struct task_struct *tsk)
> {
> +#ifdef CONFIG_PPC_BOOK3S_64
> + /*
> + * Under radix, we do not want to keep lazy PIDs around because
> + * even if the CPU does not access userspace, it can still bring
> + * in translations through speculation and prefetching.
> + *
> + * Switching away here allows us to trim back the mm_cpumask in
> + * cases where we know the process is not running on some CPUs
> + * (see mm/tlb-radix.c).
> + */
> + if (radix_enabled() && mm != &init_mm) {
> + mmgrab(&init_mm);
This is called when a kernel thread decides to unuse a mm, I agree switching
to init_mm as active_mm is reasonable thing to do.
> + tsk->active_mm = &init_mm;
Are we called with irqs disabled? Don't we need it below?
> + switch_mm_irqs_off(mm, tsk->active_mm, tsk);
> + mmdrop(mm);
> + }
> +#endif
> +
> /* 64-bit Book3E keeps track of current PGD in the PACA */
> #ifdef CONFIG_PPC_BOOK3E_64
> get_paca()->pgd = NULL;
> diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
> index a7eabff27a0f..006fce98c403 100644
> --- a/arch/powerpc/include/asm/tlb.h
> +++ b/arch/powerpc/include/asm/tlb.h
> @@ -76,6 +76,14 @@ static inline int mm_is_thread_local(struct mm_struct *mm)
> return false;
> return cpumask_test_cpu(smp_processor_id(), mm_cpumask(mm));
> }
> +static inline void mm_reset_thread_local(struct mm_struct *mm)
reset_thread_local --> reset_to_thread_local?
> +{
> + WARN_ON(atomic_read(&mm->context.copros) > 0);
Can we put this under DEBUG_VM, VM_WARN_ON?
> + WARN_ON(!(atomic_read(&mm->mm_users) == 1 && current->mm == mm));
> + atomic_set(&mm->context.active_cpus, 1);
> + cpumask_clear(mm_cpumask(mm));
> + cpumask_set_cpu(smp_processor_id(), mm_cpumask(mm));
> +}
> #else /* CONFIG_PPC_BOOK3S_64 */
> static inline int mm_is_thread_local(struct mm_struct *mm)
> {
> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> index 5ac3206c51cc..d5593a78702a 100644
> --- a/arch/powerpc/mm/tlb-radix.c
> +++ b/arch/powerpc/mm/tlb-radix.c
> @@ -504,6 +504,15 @@ void radix__local_flush_tlb_page(struct vm_area_struct *vma, unsigned long vmadd
> }
> EXPORT_SYMBOL(radix__local_flush_tlb_page);
>
> +static bool mm_is_singlethreaded(struct mm_struct *mm)
> +{
mm_tlb_context_is_local? We should also skip init_mm from these checks
> + if (atomic_read(&mm->context.copros) > 0)
> + return false;
> + if (atomic_read(&mm->mm_users) == 1 && current->mm == mm)
> + return true;
> + return false;
> +}
> +
> static bool mm_needs_flush_escalation(struct mm_struct *mm)
> {
> /*
> @@ -511,7 +520,9 @@ static bool mm_needs_flush_escalation(struct mm_struct *mm)
> * caching PTEs and not flushing them properly when
> * RIC = 0 for a PID/LPID invalidate
> */
> - return atomic_read(&mm->context.copros) != 0;
> + if (atomic_read(&mm->context.copros) > 0)
> + return true;
> + return false;
> }
>
> #ifdef CONFIG_SMP
> @@ -525,12 +536,17 @@ void radix__flush_tlb_mm(struct mm_struct *mm)
>
> preempt_disable();
> if (!mm_is_thread_local(mm)) {
> - if (mm_needs_flush_escalation(mm))
> + if (mm_is_singlethreaded(mm)) {
> _tlbie_pid(pid, RIC_FLUSH_ALL);
> - else
> + mm_reset_thread_local(mm);
> + } else if (mm_needs_flush_escalation(mm)) {
> + _tlbie_pid(pid, RIC_FLUSH_ALL);
> + } else {
> _tlbie_pid(pid, RIC_FLUSH_TLB);
> - } else
> + }
> + } else {
> _tlbiel_pid(pid, RIC_FLUSH_TLB);
> + }
> preempt_enable();
> }
> EXPORT_SYMBOL(radix__flush_tlb_mm);
> @@ -544,10 +560,13 @@ void radix__flush_all_mm(struct mm_struct *mm)
> return;
>
> preempt_disable();
> - if (!mm_is_thread_local(mm))
> + if (!mm_is_thread_local(mm)) {
> _tlbie_pid(pid, RIC_FLUSH_ALL);
> - else
> + if (mm_is_singlethreaded(mm))
> + mm_reset_thread_local(mm);
> + } else {
> _tlbiel_pid(pid, RIC_FLUSH_ALL);
> + }
> preempt_enable();
> }
> EXPORT_SYMBOL(radix__flush_all_mm);
> @@ -644,10 +663,14 @@ void radix__flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
> if (local) {
> _tlbiel_pid(pid, RIC_FLUSH_TLB);
> } else {
> - if (mm_needs_flush_escalation(mm))
> + if (mm_is_singlethreaded(mm)) {
> + _tlbie_pid(pid, RIC_FLUSH_ALL);
> + mm_reset_thread_local(mm);
> + } else if (mm_needs_flush_escalation(mm)) {
> _tlbie_pid(pid, RIC_FLUSH_ALL);
> - else
> + } else {
> _tlbie_pid(pid, RIC_FLUSH_TLB);
> + }
> }
> } else {
> bool hflush = false;
> @@ -802,13 +825,19 @@ static inline void __radix__flush_tlb_range_psize(struct mm_struct *mm,
> }
>
> if (full) {
> - if (!local && mm_needs_flush_escalation(mm))
> - also_pwc = true;
> -
> - if (local)
> + if (local) {
> _tlbiel_pid(pid, also_pwc ? RIC_FLUSH_ALL : RIC_FLUSH_TLB);
> - else
> - _tlbie_pid(pid, also_pwc ? RIC_FLUSH_ALL: RIC_FLUSH_TLB);
> + } else {
> + if (mm_is_singlethreaded(mm)) {
> + _tlbie_pid(pid, RIC_FLUSH_ALL);
> + mm_reset_thread_local(mm);
> + } else {
> + if (mm_needs_flush_escalation(mm))
> + also_pwc = true;
> +
> + _tlbie_pid(pid, also_pwc ? RIC_FLUSH_ALL : RIC_FLUSH_TLB);
> + }
> + }
> } else {
> if (local)
> _tlbiel_va_range(start, end, pid, page_size, psize, also_pwc);
Looks good otherwise
Reviewed-by: Balbir Singh <bsingharora@gmail.com>
Balbir Singh.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc/64s/radix: reset mm_cpumask for single thread process when possible
2018-05-09 6:56 [PATCH] powerpc/64s/radix: reset mm_cpumask for single thread process when possible Nicholas Piggin
2018-05-09 8:23 ` Balbir Singh
@ 2018-05-09 8:31 ` Benjamin Herrenschmidt
2018-05-09 11:53 ` Nicholas Piggin
1 sibling, 1 reply; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-09 8:31 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
On Wed, 2018-05-09 at 16:56 +1000, Nicholas Piggin wrote:
> When a single-threaded process has a non-local mm_cpumask and requires
> a full PID tlbie invalidation, use that as an opportunity to reset the
> cpumask back to the current CPU we're running on.
>
> No other thread can concurrently switch to this mm, because it must
> have had a reference on mm_users before it could use_mm. mm_users can
> be asynchronously incremented e.g., by mmget_not_zero, but those users
> must not be doing use_mm.
What do you mean ? I don't fully understand how this isn't racy with
another thread being created, switching to that mm, and then having its
bit cleared by us ? Also why not use_mm ? what prevents it ?
Cheers,
Ben.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc/64s/radix: reset mm_cpumask for single thread process when possible
2018-05-09 8:23 ` Balbir Singh
@ 2018-05-09 11:22 ` Nicholas Piggin
0 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2018-05-09 11:22 UTC (permalink / raw)
To: Balbir Singh; +Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
On Wed, 9 May 2018 18:23:48 +1000
Balbir Singh <bsingharora@gmail.com> wrote:
> On Wed, May 9, 2018 at 4:56 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> > When a single-threaded process has a non-local mm_cpumask and requires
> > a full PID tlbie invalidation, use that as an opportunity to reset the
> > cpumask back to the current CPU we're running on.
> >
> > No other thread can concurrently switch to this mm, because it must
> > have had a reference on mm_users before it could use_mm. mm_users can
> > be asynchronously incremented e.g., by mmget_not_zero, but those users
> > must not be doing use_mm.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/include/asm/mmu_context.h | 19 +++++++++
> > arch/powerpc/include/asm/tlb.h | 8 ++++
> > arch/powerpc/mm/tlb-radix.c | 57 +++++++++++++++++++-------
> > 3 files changed, 70 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index 1835ca1505d6..df12a994529f 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -6,6 +6,7 @@
> > #include <linux/kernel.h>
> > #include <linux/mm.h>
> > #include <linux/sched.h>
> > +#include <linux/sched/mm.h>
> > #include <linux/spinlock.h>
> > #include <asm/mmu.h>
> > #include <asm/cputable.h>
> > @@ -201,6 +202,24 @@ static inline void activate_mm(struct mm_struct *prev, struct mm_struct *next)
> > static inline void enter_lazy_tlb(struct mm_struct *mm,
> > struct task_struct *tsk)
> > {
> > +#ifdef CONFIG_PPC_BOOK3S_64
> > + /*
> > + * Under radix, we do not want to keep lazy PIDs around because
> > + * even if the CPU does not access userspace, it can still bring
> > + * in translations through speculation and prefetching.
> > + *
> > + * Switching away here allows us to trim back the mm_cpumask in
> > + * cases where we know the process is not running on some CPUs
> > + * (see mm/tlb-radix.c).
> > + */
> > + if (radix_enabled() && mm != &init_mm) {
> > + mmgrab(&init_mm);
>
> This is called when a kernel thread decides to unuse a mm, I agree switching
> to init_mm as active_mm is reasonable thing to do.
We lose lazy PIDR switching. Should probably ifdef CONFIG_SMP it, and
possibly one day we could look into having an arch notification for
scheduler migrating tasks so we could restore the optimisation.
For now I could not measure a cost, but it will be a few cycles.
>
> > + tsk->active_mm = &init_mm;
>
> Are we called with irqs disabled? Don't we need it below?
Hmm, yeah you might be right.
> > + switch_mm_irqs_off(mm, tsk->active_mm, tsk);
> > + mmdrop(mm);
> > + }
> > +#endif
> > +
> > /* 64-bit Book3E keeps track of current PGD in the PACA */
> > #ifdef CONFIG_PPC_BOOK3E_64
> > get_paca()->pgd = NULL;
> > diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
> > index a7eabff27a0f..006fce98c403 100644
> > --- a/arch/powerpc/include/asm/tlb.h
> > +++ b/arch/powerpc/include/asm/tlb.h
> > @@ -76,6 +76,14 @@ static inline int mm_is_thread_local(struct mm_struct *mm)
> > return false;
> > return cpumask_test_cpu(smp_processor_id(), mm_cpumask(mm));
> > }
> > +static inline void mm_reset_thread_local(struct mm_struct *mm)
> reset_thread_local --> reset_to_thread_local?
>
> > +{
> > + WARN_ON(atomic_read(&mm->context.copros) > 0);
>
> Can we put this under DEBUG_VM, VM_WARN_ON?
Yeah we could, I worry nobody tests with them on...
>
> > + WARN_ON(!(atomic_read(&mm->mm_users) == 1 && current->mm == mm));
>
>
>
> > + atomic_set(&mm->context.active_cpus, 1);
> > + cpumask_clear(mm_cpumask(mm));
> > + cpumask_set_cpu(smp_processor_id(), mm_cpumask(mm));
> > +}
> > #else /* CONFIG_PPC_BOOK3S_64 */
> > static inline int mm_is_thread_local(struct mm_struct *mm)
> > {
> > diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> > index 5ac3206c51cc..d5593a78702a 100644
> > --- a/arch/powerpc/mm/tlb-radix.c
> > +++ b/arch/powerpc/mm/tlb-radix.c
> > @@ -504,6 +504,15 @@ void radix__local_flush_tlb_page(struct vm_area_struct *vma, unsigned long vmadd
> > }
> > EXPORT_SYMBOL(radix__local_flush_tlb_page);
> >
> > +static bool mm_is_singlethreaded(struct mm_struct *mm)
> > +{
>
> mm_tlb_context_is_local?
It's not the same thing as _local. _local means only local TLBs.
_singlethreaded means only this thread can access the mappings.
> We should also skip init_mm from these checks
I don't think we should see init_mm here ever -- it has no user
mappings so won't get this user tlb flushing.
>
> > + if (atomic_read(&mm->context.copros) > 0)
> > + return false;
> > + if (atomic_read(&mm->mm_users) == 1 && current->mm == mm)
> > + return true;
> > + return false;
> > +}
> > +
> > static bool mm_needs_flush_escalation(struct mm_struct *mm)
> > {
> > /*
> > @@ -511,7 +520,9 @@ static bool mm_needs_flush_escalation(struct mm_struct *mm)
> > * caching PTEs and not flushing them properly when
> > * RIC = 0 for a PID/LPID invalidate
> > */
> > - return atomic_read(&mm->context.copros) != 0;
> > + if (atomic_read(&mm->context.copros) > 0)
> > + return true;
> > + return false;
> > }
> >
> > #ifdef CONFIG_SMP
> > @@ -525,12 +536,17 @@ void radix__flush_tlb_mm(struct mm_struct *mm)
> >
> > preempt_disable();
> > if (!mm_is_thread_local(mm)) {
> > - if (mm_needs_flush_escalation(mm))
> > + if (mm_is_singlethreaded(mm)) {
> > _tlbie_pid(pid, RIC_FLUSH_ALL);
> > - else
> > + mm_reset_thread_local(mm);
> > + } else if (mm_needs_flush_escalation(mm)) {
> > + _tlbie_pid(pid, RIC_FLUSH_ALL);
> > + } else {
> > _tlbie_pid(pid, RIC_FLUSH_TLB);
> > - } else
> > + }
> > + } else {
> > _tlbiel_pid(pid, RIC_FLUSH_TLB);
> > + }
> > preempt_enable();
> > }
> > EXPORT_SYMBOL(radix__flush_tlb_mm);
> > @@ -544,10 +560,13 @@ void radix__flush_all_mm(struct mm_struct *mm)
> > return;
> >
> > preempt_disable();
> > - if (!mm_is_thread_local(mm))
> > + if (!mm_is_thread_local(mm)) {
> > _tlbie_pid(pid, RIC_FLUSH_ALL);
> > - else
> > + if (mm_is_singlethreaded(mm))
> > + mm_reset_thread_local(mm);
> > + } else {
> > _tlbiel_pid(pid, RIC_FLUSH_ALL);
> > + }
> > preempt_enable();
> > }
> > EXPORT_SYMBOL(radix__flush_all_mm);
> > @@ -644,10 +663,14 @@ void radix__flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
> > if (local) {
> > _tlbiel_pid(pid, RIC_FLUSH_TLB);
> > } else {
> > - if (mm_needs_flush_escalation(mm))
> > + if (mm_is_singlethreaded(mm)) {
> > + _tlbie_pid(pid, RIC_FLUSH_ALL);
> > + mm_reset_thread_local(mm);
> > + } else if (mm_needs_flush_escalation(mm)) {
> > _tlbie_pid(pid, RIC_FLUSH_ALL);
> > - else
> > + } else {
> > _tlbie_pid(pid, RIC_FLUSH_TLB);
> > + }
> > }
> > } else {
> > bool hflush = false;
> > @@ -802,13 +825,19 @@ static inline void __radix__flush_tlb_range_psize(struct mm_struct *mm,
> > }
> >
> > if (full) {
> > - if (!local && mm_needs_flush_escalation(mm))
> > - also_pwc = true;
> > -
> > - if (local)
> > + if (local) {
> > _tlbiel_pid(pid, also_pwc ? RIC_FLUSH_ALL : RIC_FLUSH_TLB);
> > - else
> > - _tlbie_pid(pid, also_pwc ? RIC_FLUSH_ALL: RIC_FLUSH_TLB);
> > + } else {
> > + if (mm_is_singlethreaded(mm)) {
> > + _tlbie_pid(pid, RIC_FLUSH_ALL);
> > + mm_reset_thread_local(mm);
> > + } else {
> > + if (mm_needs_flush_escalation(mm))
> > + also_pwc = true;
> > +
> > + _tlbie_pid(pid, also_pwc ? RIC_FLUSH_ALL : RIC_FLUSH_TLB);
> > + }
> > + }
> > } else {
> > if (local)
> > _tlbiel_va_range(start, end, pid, page_size, psize, also_pwc);
>
>
> Looks good otherwise
Thanks,
Nick
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc/64s/radix: reset mm_cpumask for single thread process when possible
2018-05-09 8:31 ` Benjamin Herrenschmidt
@ 2018-05-09 11:53 ` Nicholas Piggin
0 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2018-05-09 11:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
On Wed, 09 May 2018 18:31:55 +1000
Benjamin Herrenschmidt <benh@au1.ibm.com> wrote:
> On Wed, 2018-05-09 at 16:56 +1000, Nicholas Piggin wrote:
> > When a single-threaded process has a non-local mm_cpumask and requires
> > a full PID tlbie invalidation, use that as an opportunity to reset the
> > cpumask back to the current CPU we're running on.
> >
> > No other thread can concurrently switch to this mm, because it must
> > have had a reference on mm_users before it could use_mm. mm_users can
> > be asynchronously incremented e.g., by mmget_not_zero, but those users
> > must not be doing use_mm.
>
> What do you mean ? I don't fully understand how this isn't racy with
> another thread being created, switching to that mm, and then having its
> bit cleared by us ?
We are the single thread, so nothing else can call clone on our mm.
> Also why not use_mm ? what prevents it ?
I'm not sure if it has an assertion (it probably should), but existing
convention. Such things would be broken there are already other
architectures doing similar things with TLB flushing for example, alpha,
ia64, mips, sh... sparc does something very similar resetting its cpumask
by the look.
I grepped mmget_not_zero callers too, couldn't see any obvious use_mm
cases.
Thanks,
Nick
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-09 11:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-09 6:56 [PATCH] powerpc/64s/radix: reset mm_cpumask for single thread process when possible Nicholas Piggin
2018-05-09 8:23 ` Balbir Singh
2018-05-09 11:22 ` Nicholas Piggin
2018-05-09 8:31 ` Benjamin Herrenschmidt
2018-05-09 11:53 ` Nicholas Piggin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.