Subject: [PATCH] Documentation/features: mark BATCHED_UNMAP_TLB_FLUSH doesn't apply to ARM64

BATCHED_UNMAP_TLB_FLUSH is used on x86 to do batched tlb shootdown by
sending one IPI to TLB flush all entries after unmapping pages rather
than sending an IPI to flush each individual entry.
On arm64, tlb shootdown is done by hardware. Flush instructions are
innershareable. The local flushes are limited to the boot (1 per CPU)
and when a task is getting a new ASID.
So marking this feature as "TODO" is not proper. ".." isn't good as
well. So this patch adds a "N/A" for this kind of features which are
not needed on some architectures.

Cc: Mel Gorman <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Barry Song <[email protected]>
---
Documentation/features/arch-support.txt | 1 +
Documentation/features/vm/TLB/arch-support.txt | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/features/arch-support.txt b/Documentation/features/arch-support.txt
index d22a1095e661..118ae031840b 100644
--- a/Documentation/features/arch-support.txt
+++ b/Documentation/features/arch-support.txt
@@ -8,4 +8,5 @@ The meaning of entries in the tables is:
| ok | # feature supported by the architecture
|TODO| # feature not yet supported by the architecture
| .. | # feature cannot be supported by the hardware
+ | N/A| # feature doesn't apply to the architecture

diff --git a/Documentation/features/vm/TLB/arch-support.txt b/Documentation/features/vm/TLB/arch-support.txt
index 30f75a79ce01..0d070f9f98d8 100644
--- a/Documentation/features/vm/TLB/arch-support.txt
+++ b/Documentation/features/vm/TLB/arch-support.txt
@@ -9,7 +9,7 @@
| alpha: | TODO |
| arc: | TODO |
| arm: | TODO |
- | arm64: | TODO |
+ | arm64: | N/A |
| c6x: | .. |
| csky: | TODO |
| h8300: | .. |
--
2.25.1


2021-02-23 06:26:36

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH] Documentation/features: mark BATCHED_UNMAP_TLB_FLUSH doesn't apply to ARM64



On 2/23/21 6:02 AM, Barry Song wrote:
> BATCHED_UNMAP_TLB_FLUSH is used on x86 to do batched tlb shootdown by
> sending one IPI to TLB flush all entries after unmapping pages rather
> than sending an IPI to flush each individual entry.
> On arm64, tlb shootdown is done by hardware. Flush instructions are
> innershareable. The local flushes are limited to the boot (1 per CPU)
> and when a task is getting a new ASID.

Is there any previous discussion around this ?

> So marking this feature as "TODO" is not proper. ".." isn't good as
> well. So this patch adds a "N/A" for this kind of features which are
> not needed on some architectures.
>
> Cc: Mel Gorman <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Barry Song <[email protected]>
> ---
> Documentation/features/arch-support.txt | 1 +
> Documentation/features/vm/TLB/arch-support.txt | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/features/arch-support.txt b/Documentation/features/arch-support.txt
> index d22a1095e661..118ae031840b 100644
> --- a/Documentation/features/arch-support.txt
> +++ b/Documentation/features/arch-support.txt
> @@ -8,4 +8,5 @@ The meaning of entries in the tables is:
> | ok | # feature supported by the architecture
> |TODO| # feature not yet supported by the architecture
> | .. | # feature cannot be supported by the hardware
> + | N/A| # feature doesn't apply to the architecture

NA might be better here. s/doesn't apply/not applicable/ in order to match NA.
Still wondering if NA is really needed when there is already ".." ? Regardless
either way should be fine.

>
> diff --git a/Documentation/features/vm/TLB/arch-support.txt b/Documentation/features/vm/TLB/arch-support.txt
> index 30f75a79ce01..0d070f9f98d8 100644
> --- a/Documentation/features/vm/TLB/arch-support.txt
> +++ b/Documentation/features/vm/TLB/arch-support.txt
> @@ -9,7 +9,7 @@
> | alpha: | TODO |
> | arc: | TODO |
> | arm: | TODO |
> - | arm64: | TODO |
> + | arm64: | N/A |
> | c6x: | .. |
> | csky: | TODO |
> | h8300: | .. |
>

Subject: RE: [Linuxarm] Re: [PATCH] Documentation/features: mark BATCHED_UNMAP_TLB_FLUSH doesn't apply to ARM64



> -----Original Message-----
> From: Anshuman Khandual [mailto:[email protected]]
> Sent: Tuesday, February 23, 2021 7:10 PM
> To: Song Bao Hua (Barry Song) <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; Mel Gorman <[email protected]>; Andy Lutomirski
> <[email protected]>; Catalin Marinas <[email protected]>; Will Deacon
> <[email protected]>
> Subject: [Linuxarm] Re: [PATCH] Documentation/features: mark
> BATCHED_UNMAP_TLB_FLUSH doesn't apply to ARM64
>
>
>
> On 2/23/21 6:02 AM, Barry Song wrote:
> > BATCHED_UNMAP_TLB_FLUSH is used on x86 to do batched tlb shootdown by
> > sending one IPI to TLB flush all entries after unmapping pages rather
> > than sending an IPI to flush each individual entry.
> > On arm64, tlb shootdown is done by hardware. Flush instructions are
> > innershareable. The local flushes are limited to the boot (1 per CPU)
> > and when a task is getting a new ASID.
>
> Is there any previous discussion around this ?

I copied the declaration of local flushes from:

"ARM64 Linux kernel is SMP-aware (no possibility to build only for UP).
Most of the flush instructions are innershareable. The local flushes are
limited to the boot (1 per CPU) and when a task is getting a new ASIC."

https://patchwork.kernel.org/project/xen-devel/patch/[email protected]/

I am not sure if getting a new asid and the boot are the only two
cases of local flushes while I think this is probably true.

But even we find more corner cases, hardly the trend arm64 doesn't
need BATCHED_UNMAP_TLB_FLUSH will be changed.

>
> > So marking this feature as "TODO" is not proper. ".." isn't good as
> > well. So this patch adds a "N/A" for this kind of features which are
> > not needed on some architectures.
> >
> > Cc: Mel Gorman <[email protected]>
> > Cc: Andy Lutomirski <[email protected]>
> > Cc: Catalin Marinas <[email protected]>
> > Cc: Will Deacon <[email protected]>
> > Signed-off-by: Barry Song <[email protected]>
> > ---
> > Documentation/features/arch-support.txt | 1 +
> > Documentation/features/vm/TLB/arch-support.txt | 2 +-
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/features/arch-support.txt
> b/Documentation/features/arch-support.txt
> > index d22a1095e661..118ae031840b 100644
> > --- a/Documentation/features/arch-support.txt
> > +++ b/Documentation/features/arch-support.txt
> > @@ -8,4 +8,5 @@ The meaning of entries in the tables is:
> > | ok | # feature supported by the architecture
> > |TODO| # feature not yet supported by the architecture
> > | .. | # feature cannot be supported by the hardware
> > + | N/A| # feature doesn't apply to the architecture
>
> NA might be better here. s/doesn't apply/not applicable/ in order to match NA.
> Still wondering if NA is really needed when there is already ".." ? Regardless
> either way should be fine.

I don't think ".." is proper here. ".." means hardware doesn't support
the feature. But here it is just opposite, arm64 has the hardware
support of tlb shootdown rather than depending on a software IPI.

>
> >
> > diff --git a/Documentation/features/vm/TLB/arch-support.txt
> b/Documentation/features/vm/TLB/arch-support.txt
> > index 30f75a79ce01..0d070f9f98d8 100644
> > --- a/Documentation/features/vm/TLB/arch-support.txt
> > +++ b/Documentation/features/vm/TLB/arch-support.txt
> > @@ -9,7 +9,7 @@
> > | alpha: | TODO |
> > | arc: | TODO |
> > | arm: | TODO |
> > - | arm64: | TODO |
> > + | arm64: | N/A |
> > | c6x: | .. |
> > | csky: | TODO |
> > | h8300: | .. |
> >
Thanks
Barry

2021-03-08 13:01:35

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] Documentation/features: mark BATCHED_UNMAP_TLB_FLUSH doesn't apply to ARM64

On Tue, Feb 23, 2021 at 01:32:30PM +1300, Barry Song wrote:
> BATCHED_UNMAP_TLB_FLUSH is used on x86 to do batched tlb shootdown by
> sending one IPI to TLB flush all entries after unmapping pages rather
> than sending an IPI to flush each individual entry.
> On arm64, tlb shootdown is done by hardware. Flush instructions are
> innershareable. The local flushes are limited to the boot (1 per CPU)
> and when a task is getting a new ASID.
> So marking this feature as "TODO" is not proper. ".." isn't good as
> well. So this patch adds a "N/A" for this kind of features which are
> not needed on some architectures.
>
> Cc: Mel Gorman <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Barry Song <[email protected]>
> ---
> Documentation/features/arch-support.txt | 1 +
> Documentation/features/vm/TLB/arch-support.txt | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/features/arch-support.txt b/Documentation/features/arch-support.txt
> index d22a1095e661..118ae031840b 100644
> --- a/Documentation/features/arch-support.txt
> +++ b/Documentation/features/arch-support.txt
> @@ -8,4 +8,5 @@ The meaning of entries in the tables is:
> | ok | # feature supported by the architecture
> |TODO| # feature not yet supported by the architecture
> | .. | # feature cannot be supported by the hardware
> + | N/A| # feature doesn't apply to the architecture
>
> diff --git a/Documentation/features/vm/TLB/arch-support.txt b/Documentation/features/vm/TLB/arch-support.txt
> index 30f75a79ce01..0d070f9f98d8 100644
> --- a/Documentation/features/vm/TLB/arch-support.txt
> +++ b/Documentation/features/vm/TLB/arch-support.txt
> @@ -9,7 +9,7 @@
> | alpha: | TODO |
> | arc: | TODO |
> | arm: | TODO |
> - | arm64: | TODO |
> + | arm64: | N/A |
> | c6x: | .. |
> | csky: | TODO |
> | h8300: | .. |

Acked-by: Will Deacon <[email protected]>

Will

2021-03-16 02:31:48

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH] Documentation/features: mark BATCHED_UNMAP_TLB_FLUSH doesn't apply to ARM64

Barry Song <[email protected]> writes:

> BATCHED_UNMAP_TLB_FLUSH is used on x86 to do batched tlb shootdown by
> sending one IPI to TLB flush all entries after unmapping pages rather
> than sending an IPI to flush each individual entry.
> On arm64, tlb shootdown is done by hardware. Flush instructions are
> innershareable. The local flushes are limited to the boot (1 per CPU)
> and when a task is getting a new ASID.
> So marking this feature as "TODO" is not proper. ".." isn't good as
> well. So this patch adds a "N/A" for this kind of features which are
> not needed on some architectures.
>
> Cc: Mel Gorman <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Barry Song <[email protected]>
> ---
> Documentation/features/arch-support.txt | 1 +
> Documentation/features/vm/TLB/arch-support.txt | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)

This had a conflict with the c6x removal, but I fixed that up and
(finally) applied it, thanks.

jon