diff mbox

powerpc/mm: Use tlbiel only if we ever ran on the current cpu

Message ID 20161024032043.22455-1-aneesh.kumar@linux.vnet.ibm.com (mailing list archive)
State Accepted
Headers show

Commit Message

Aneesh Kumar K.V Oct. 24, 2016, 3:20 a.m. UTC
Before this patch, we used tlbiel, if we ever ran only on this core.
That was mostly derived from the nohash usage of the same. But the
ISA 3.0 clarifies tlbiel such that

"All TLB entries that have all of the following properties are made
invalid on the thread executing the tlbiel instruction"

Hence use tlbiel, if we only ever ran on just the current cpu.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---

NOTE: May be the nohash usage also need to switch to mm_is_thread_local ?
I didn't check the spec, hence I left it unchanged.

I am also not adding a Fixes: line because there is no one commit. But
this is needed for radix enabled kernel. So a backport to 4.8 is needed.


 arch/powerpc/include/asm/tlb.h | 12 ++++++++++++
 arch/powerpc/mm/tlb-radix.c    |  8 ++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

Comments

Balbir Singh Oct. 24, 2016, 3:40 a.m. UTC | #1
On 24/10/16 14:20, Aneesh Kumar K.V wrote:
> Before this patch, we used tlbiel, if we ever ran only on this core.
> That was mostly derived from the nohash usage of the same. But the
> ISA 3.0 clarifies tlbiel such that
> 
> "All TLB entries that have all of the following properties are made
> invalid on the thread executing the tlbiel instruction"
> 
> Hence use tlbiel, if we only ever ran on just the current cpu.
> 

Could you clarify the impact. The impact I see is that it could
lead to us thinking we invalidated the TLB across the core whereas
we did it only on the current thread? This could leave others threads
in the same core with invalid TLB's, if cpumask reported we ran on
other threads in the same core?

Balbir Singh.
Aneesh Kumar K.V Oct. 24, 2016, 6:01 a.m. UTC | #2
Balbir Singh <bsingharora@gmail.com> writes:

> On 24/10/16 14:20, Aneesh Kumar K.V wrote:
>> Before this patch, we used tlbiel, if we ever ran only on this core.
>> That was mostly derived from the nohash usage of the same. But the
>> ISA 3.0 clarifies tlbiel such that
>> 
>> "All TLB entries that have all of the following properties are made
>> invalid on the thread executing the tlbiel instruction"
>> 
>> Hence use tlbiel, if we only ever ran on just the current cpu.
>> 
>
> Could you clarify the impact. The impact I see is that it could
> lead to us thinking we invalidated the TLB across the core whereas
> we did it only on the current thread? This could leave others threads
> in the same core with invalid TLB's, if cpumask reported we ran on
> other threads in the same core?

Correct.

-aneesh
Michael Ellerman Oct. 27, 2016, 11:38 a.m. UTC | #3
On Mon, 2016-24-10 at 03:20:43 UTC, "Aneesh Kumar K.V" wrote:
> Before this patch, we used tlbiel, if we ever ran only on this core.
> That was mostly derived from the nohash usage of the same. But the
> ISA 3.0 clarifies tlbiel such that
> 
> "All TLB entries that have all of the following properties are made
> invalid on the thread executing the tlbiel instruction"
> 
> Hence use tlbiel, if we only ever ran on just the current cpu.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/bd77c4498616e27d5725b5959d880c

cheers
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
index f6f68f73e858..99e1397b71da 100644
--- a/arch/powerpc/include/asm/tlb.h
+++ b/arch/powerpc/include/asm/tlb.h
@@ -52,11 +52,23 @@  static inline int mm_is_core_local(struct mm_struct *mm)
 	return cpumask_subset(mm_cpumask(mm),
 			      topology_sibling_cpumask(smp_processor_id()));
 }
+
+static inline int mm_is_thread_local(struct mm_struct *mm)
+{
+	return cpumask_equal(mm_cpumask(mm),
+			      cpumask_of(smp_processor_id()));
+}
+
 #else
 static inline int mm_is_core_local(struct mm_struct *mm)
 {
 	return 1;
 }
+
+static inline int mm_is_thread_local(struct mm_struct *mm)
+{
+	return 1;
+}
 #endif
 
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index 0e49ec541ab5..bda8c43be78a 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -175,7 +175,7 @@  void radix__flush_tlb_mm(struct mm_struct *mm)
 	if (unlikely(pid == MMU_NO_CONTEXT))
 		goto no_context;
 
-	if (!mm_is_core_local(mm)) {
+	if (!mm_is_thread_local(mm)) {
 		int lock_tlbie = !mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE);
 
 		if (lock_tlbie)
@@ -201,7 +201,7 @@  void radix__flush_tlb_pwc(struct mmu_gather *tlb, unsigned long addr)
 	if (unlikely(pid == MMU_NO_CONTEXT))
 		goto no_context;
 
-	if (!mm_is_core_local(mm)) {
+	if (!mm_is_thread_local(mm)) {
 		int lock_tlbie = !mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE);
 
 		if (lock_tlbie)
@@ -226,7 +226,7 @@  void radix__flush_tlb_page_psize(struct mm_struct *mm, unsigned long vmaddr,
 	pid = mm ? mm->context.id : 0;
 	if (unlikely(pid == MMU_NO_CONTEXT))
 		goto bail;
-	if (!mm_is_core_local(mm)) {
+	if (!mm_is_thread_local(mm)) {
 		int lock_tlbie = !mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE);
 
 		if (lock_tlbie)
@@ -321,7 +321,7 @@  void radix__flush_tlb_range_psize(struct mm_struct *mm, unsigned long start,
 {
 	unsigned long pid;
 	unsigned long addr;
-	int local = mm_is_core_local(mm);
+	int local = mm_is_thread_local(mm);
 	unsigned long ap = mmu_get_ap(psize);
 	int lock_tlbie = !mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE);
 	unsigned long page_size = 1UL << mmu_psize_defs[psize].shift;