Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v4 0/3] Optimize tlbflush path
@ 2019-08-22  7:51 Atish Patra
  2019-08-22  7:51 ` [PATCH v4 1/3] RISC-V: Do not invoke SBI call if cpumask is empty Atish Patra
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Atish Patra @ 2019-08-22  7:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Palmer Dabbelt, Atish Patra, Anup Patel,
	Paul Walmsley, linux-riscv, Christoph Hellwig

This series adds few optimizations to reduce the trap cost in the tlb
flush path. We should only make SBI calls to remote tlb flush only if
absolutely required. 

This series is based on Christoph's series:

http://lists.infradead.org/pipermail/linux-riscv/2019-August/006148.html

Changes from v3->v4
1. Simplify the local vs remote usecase.
2. Reorder the patches in the series.

Changes from v2->v3:
1. Split the patches into smaller one per optimization.

Atish Patra (3):
RISC-V: Do not invoke SBI call if cpumask is empty
RISC-V: Issue a local tlbflush if possible.
RISC-V: Issue a tlb page flush if possible

arch/riscv/mm/tlbflush.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)

--
2.21.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/3] RISC-V: Do not invoke SBI call if cpumask is empty
  2019-08-22  7:51 [PATCH v4 0/3] Optimize tlbflush path Atish Patra
@ 2019-08-22  7:51 ` Atish Patra
  2019-08-22  8:10   ` Christoph Hellwig
  2019-08-22  7:51 ` [PATCH v4 2/3] RISC-V: Issue a local tlbflush if possible Atish Patra
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Atish Patra @ 2019-08-22  7:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Palmer Dabbelt, Atish Patra, Anup Patel,
	Paul Walmsley, linux-riscv, Christoph Hellwig

SBI calls are expensive. If cpumask is empty, there is no need to
trap via SBI as no remote tlb flushing is required.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/mm/tlbflush.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index df93b26f1b9d..1293b8017ee0 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -14,6 +14,9 @@ static void __sbi_tlb_flush_range(struct cpumask *cmask, unsigned long start,
 {
 	struct cpumask hmask;
 
+	if (cpumask_empty(cmask))
+		return;
+
 	riscv_cpuid_to_hartid_mask(cmask, &hmask);
 	sbi_remote_sfence_vma(hmask.bits, start, size);
 }
-- 
2.21.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 2/3] RISC-V: Issue a local tlbflush if possible.
  2019-08-22  7:51 [PATCH v4 0/3] Optimize tlbflush path Atish Patra
  2019-08-22  7:51 ` [PATCH v4 1/3] RISC-V: Do not invoke SBI call if cpumask is empty Atish Patra
@ 2019-08-22  7:51 ` Atish Patra
  2019-08-22  8:10   ` Christoph Hellwig
  2019-08-22  7:51 ` [PATCH v4 3/3] RISC-V: Issue a tlb page flush " Atish Patra
  2019-08-31  2:50 ` [PATCH v4 0/3] Optimize tlbflush path Paul Walmsley
  3 siblings, 1 reply; 10+ messages in thread
From: Atish Patra @ 2019-08-22  7:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Palmer Dabbelt, Atish Patra, Anup Patel,
	Paul Walmsley, linux-riscv, Christoph Hellwig

In RISC-V, tlb flush happens via SBI which is expensive. If the local
cpu is the only cpu in cpumask, there is no need to invoke a SBI call.

Just do a local flush and return.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/mm/tlbflush.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index 1293b8017ee0..8172fbf46123 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -2,6 +2,7 @@
 
 #include <linux/mm.h>
 #include <linux/smp.h>
+#include <linux/sched.h>
 #include <asm/sbi.h>
 
 void flush_tlb_all(void)
@@ -9,16 +10,30 @@ void flush_tlb_all(void)
 	sbi_remote_sfence_vma(NULL, 0, -1);
 }
 
+/*
+ * This function must not be called with cmask being null.
+ * Kernel may panic if cmask is NULL.
+ */
 static void __sbi_tlb_flush_range(struct cpumask *cmask, unsigned long start,
 		unsigned long size)
 {
 	struct cpumask hmask;
+	unsigned int cpuid;
 
 	if (cpumask_empty(cmask))
 		return;
 
-	riscv_cpuid_to_hartid_mask(cmask, &hmask);
-	sbi_remote_sfence_vma(hmask.bits, start, size);
+	cpuid = get_cpu();
+
+	if (cpumask_any_but(cmask, cpuid) >= nr_cpu_ids) {
+		/* local cpu is the only cpu present in cpumask */
+		local_flush_tlb_all();
+	} else {
+		riscv_cpuid_to_hartid_mask(cmask, &hmask);
+		sbi_remote_sfence_vma(cpumask_bits(&hmask), start, size);
+	}
+
+	put_cpu();
 }
 
 void flush_tlb_mm(struct mm_struct *mm)
-- 
2.21.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 3/3] RISC-V: Issue a tlb page flush if possible
  2019-08-22  7:51 [PATCH v4 0/3] Optimize tlbflush path Atish Patra
  2019-08-22  7:51 ` [PATCH v4 1/3] RISC-V: Do not invoke SBI call if cpumask is empty Atish Patra
  2019-08-22  7:51 ` [PATCH v4 2/3] RISC-V: Issue a local tlbflush if possible Atish Patra
@ 2019-08-22  7:51 ` " Atish Patra
  2019-08-22  8:11   ` Christoph Hellwig
  2019-08-31  2:50 ` [PATCH v4 0/3] Optimize tlbflush path Paul Walmsley
  3 siblings, 1 reply; 10+ messages in thread
From: Atish Patra @ 2019-08-22  7:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Palmer Dabbelt, Atish Patra, Anup Patel,
	Paul Walmsley, linux-riscv, Christoph Hellwig

If tlbflush request is for page only, there is no need to do a
complete local tlb shootdown.

Just do a local tlb flush for the given address.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/mm/tlbflush.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
index 8172fbf46123..b1c04751bcf1 100644
--- a/arch/riscv/mm/tlbflush.c
+++ b/arch/riscv/mm/tlbflush.c
@@ -27,7 +27,10 @@ static void __sbi_tlb_flush_range(struct cpumask *cmask, unsigned long start,
 
 	if (cpumask_any_but(cmask, cpuid) >= nr_cpu_ids) {
 		/* local cpu is the only cpu present in cpumask */
-		local_flush_tlb_all();
+		if (size <= PAGE_SIZE)
+			local_flush_tlb_page(start);
+		else
+			local_flush_tlb_all();
 	} else {
 		riscv_cpuid_to_hartid_mask(cmask, &hmask);
 		sbi_remote_sfence_vma(cpumask_bits(&hmask), start, size);
-- 
2.21.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] RISC-V: Do not invoke SBI call if cpumask is empty
  2019-08-22  7:51 ` [PATCH v4 1/3] RISC-V: Do not invoke SBI call if cpumask is empty Atish Patra
@ 2019-08-22  8:10   ` Christoph Hellwig
  0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2019-08-22  8:10 UTC (permalink / raw)
  To: Atish Patra
  Cc: Albert Ou, Palmer Dabbelt, linux-kernel, Anup Patel,
	Paul Walmsley, linux-riscv, Christoph Hellwig

On Thu, Aug 22, 2019 at 12:51:49AM -0700, Atish Patra wrote:
> SBI calls are expensive. If cpumask is empty, there is no need to
> trap via SBI as no remote tlb flushing is required.
> 
> Signed-off-by: Atish Patra <atish.patra@wdc.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/3] RISC-V: Issue a local tlbflush if possible.
  2019-08-22  7:51 ` [PATCH v4 2/3] RISC-V: Issue a local tlbflush if possible Atish Patra
@ 2019-08-22  8:10   ` Christoph Hellwig
  0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2019-08-22  8:10 UTC (permalink / raw)
  To: Atish Patra
  Cc: Albert Ou, Palmer Dabbelt, linux-kernel, Anup Patel,
	Paul Walmsley, linux-riscv, Christoph Hellwig

On Thu, Aug 22, 2019 at 12:51:50AM -0700, Atish Patra wrote:
> In RISC-V, tlb flush happens via SBI which is expensive. If the local
> cpu is the only cpu in cpumask, there is no need to invoke a SBI call.
> 
> Just do a local flush and return.
> 
> Signed-off-by: Atish Patra <atish.patra@wdc.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 3/3] RISC-V: Issue a tlb page flush if possible
  2019-08-22  7:51 ` [PATCH v4 3/3] RISC-V: Issue a tlb page flush " Atish Patra
@ 2019-08-22  8:11   ` Christoph Hellwig
  2019-08-22 18:31     ` Atish Patra
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2019-08-22  8:11 UTC (permalink / raw)
  To: Atish Patra
  Cc: Albert Ou, Palmer Dabbelt, linux-kernel, Anup Patel,
	Paul Walmsley, linux-riscv, Christoph Hellwig

On Thu, Aug 22, 2019 at 12:51:51AM -0700, Atish Patra wrote:
> If tlbflush request is for page only, there is no need to do a
> complete local tlb shootdown.
> 
> Just do a local tlb flush for the given address.

Looks good, although I suspect in many cases even doing multiple
single-page sfence.vma calls might be cheaper than the global one.

But I think that is worth a ѕeparate discussion, preferably with actual
numbers.

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 3/3] RISC-V: Issue a tlb page flush if possible
  2019-08-22  8:11   ` Christoph Hellwig
@ 2019-08-22 18:31     ` Atish Patra
  0 siblings, 0 replies; 10+ messages in thread
From: Atish Patra @ 2019-08-22 18:31 UTC (permalink / raw)
  To: hch; +Cc: palmer, Anup Patel, linux-kernel, aou, paul.walmsley, linux-riscv

On Thu, 2019-08-22 at 10:11 +0200, Christoph Hellwig wrote:
> On Thu, Aug 22, 2019 at 12:51:51AM -0700, Atish Patra wrote:
> > If tlbflush request is for page only, there is no need to do a
> > complete local tlb shootdown.
> > 
> > Just do a local tlb flush for the given address.
> 
> Looks good, although I suspect in many cases even doing multiple
> single-page sfence.vma calls might be cheaper than the global one.
> 
> But I think that is worth a ѕeparate discussion, preferably with
> actual
> numbers.
> 

Yup. Finding a good threashold is always tricky without real
benchmarks.

> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks for the review.

-- 
Regards,
Atish
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/3] Optimize tlbflush path
  2019-08-22  7:51 [PATCH v4 0/3] Optimize tlbflush path Atish Patra
                   ` (2 preceding siblings ...)
  2019-08-22  7:51 ` [PATCH v4 3/3] RISC-V: Issue a tlb page flush " Atish Patra
@ 2019-08-31  2:50 ` Paul Walmsley
  2019-09-28  4:23   ` Atish Patra
  3 siblings, 1 reply; 10+ messages in thread
From: Paul Walmsley @ 2019-08-31  2:50 UTC (permalink / raw)
  To: Atish Patra
  Cc: Albert Ou, Anup Patel, linux-kernel, Palmer Dabbelt, linux-riscv,
	Christoph Hellwig

Hi Atish,

On Thu, 22 Aug 2019, Atish Patra wrote:

> This series adds few optimizations to reduce the trap cost in the tlb
> flush path. We should only make SBI calls to remote tlb flush only if
> absolutely required. 

The patches look great.  My understanding is that these optimization 
patches may actually be a partial workaround for the TLB flushing bug that 
we've been looking at for the last month or so, which can corrupt memory 
or crash the system.

If that's the case, let's first root-cause the underlying bug.  Otherwise 
we'll just be papering over the actual issue, which probably could still 
occur even with this series, correct?  Since it contains no explicit 
fixes?


- Paul

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/3] Optimize tlbflush path
  2019-08-31  2:50 ` [PATCH v4 0/3] Optimize tlbflush path Paul Walmsley
@ 2019-09-28  4:23   ` Atish Patra
  0 siblings, 0 replies; 10+ messages in thread
From: Atish Patra @ 2019-09-28  4:23 UTC (permalink / raw)
  To: paul.walmsley
  Cc: Damien Le Moal, aou, Anup Patel, linux-kernel, palmer, linux-riscv, hch

On Fri, 2019-08-30 at 19:50 -0700, Paul Walmsley wrote:
> Hi Atish,
> 
> On Thu, 22 Aug 2019, Atish Patra wrote:
> 
> > This series adds few optimizations to reduce the trap cost in the
> > tlb
> > flush path. We should only make SBI calls to remote tlb flush only
> > if
> > absolutely required. 
> 
> The patches look great.  My understanding is that these optimization 
> patches may actually be a partial workaround for the TLB flushing bug
> that 
> we've been looking at for the last month or so, which can corrupt
> memory 
> or crash the system.
> 
> If that's the case, let's first root-cause the underlying
> bug.  Otherwise 
> we'll just be papering over the actual issue, which probably could
> still 
> occur even with this series, correct?  Since it contains no explicit 
> fixes?
> 
> 
I have verified the glibc locale install issue both in Qemu and
Unleashed. I don't see any issue with OpenSBI master + Linux v5.3
kernel.

As per our investigation, it looks like a hardware errata with
Unleashed board as the memory corruption issue only happens in case of
tlb range flush. In RISC-V, sfence.vma can only be issued at page
boundary. If the range is larger than that, OpenSBI has to issue
multiple sfence.vma calls back to back leading to possible memory
corruption.

Currently, OpenSBI has a platform feature i.e. "tlb_range_flush_limit"
that allows to configure tlb flush threshold per platform. Any tlb
flush range request greater than this threshold, is converted to a full
flush. Currently, it is set to the default value 4K for every
platform[1]. Glibc locale install memory corruption only happens if
this threshold is changed to a higher value i.e. 1G. This doesn't
change anything in OpenSBI code path except the fact that it will issue
many sfence.vma instructions back to back instead of one. 

If the hardware team at SiFive can look into this as well, it would be
great.

To conclude, we think this issue need to be investigated by hardware
team and the kernel patch can be merged to get the performance benefit.

[1] 
https://github.com/riscv/opensbi/blob/master/include/sbi/sbi_platform.h#L40



> - Paul
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

-- 
Regards,
Atish
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-22  7:51 [PATCH v4 0/3] Optimize tlbflush path Atish Patra
2019-08-22  7:51 ` [PATCH v4 1/3] RISC-V: Do not invoke SBI call if cpumask is empty Atish Patra
2019-08-22  8:10   ` Christoph Hellwig
2019-08-22  7:51 ` [PATCH v4 2/3] RISC-V: Issue a local tlbflush if possible Atish Patra
2019-08-22  8:10   ` Christoph Hellwig
2019-08-22  7:51 ` [PATCH v4 3/3] RISC-V: Issue a tlb page flush " Atish Patra
2019-08-22  8:11   ` Christoph Hellwig
2019-08-22 18:31     ` Atish Patra
2019-08-31  2:50 ` [PATCH v4 0/3] Optimize tlbflush path Paul Walmsley
2019-09-28  4:23   ` Atish Patra

Linux-RISC-V Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-riscv/0 linux-riscv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-riscv linux-riscv/ https://lore.kernel.org/linux-riscv \
		linux-riscv@lists.infradead.org infradead-linux-riscv@archiver.kernel.org
	public-inbox-index linux-riscv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-riscv


AGPL code for this site: git clone https://public-inbox.org/ public-inbox