diff mbox series

[v3,1/4] Revert "Documentation/features: mark BATCHED_UNMAP_TLB_FLUSH doesn't apply to ARM64"

Message ID 20220822082120.8347-2-yangyicong@huawei.com (mailing list archive)
State New
Headers show
Series mm: arm64: bring up BATCHED_UNMAP_TLB_FLUSH | expand

Commit Message

Yicong Yang Aug. 22, 2022, 8:21 a.m. UTC
From: Barry Song <v-songbaohua@oppo.com>

This reverts commit 6bfef171d0d74cb050112e0e49feb20bfddf7f42.

I was wrong. Though ARM64 has hardware TLB flush, but it is not free
and it is still expensive.
We still have a good chance to enable batched and deferred TLB flush
on ARM64 for memory reclamation. A possible way is that we only queue
tlbi instructions in hardware's queue. When we have to broadcast TLB,
we broadcast it by dsb. We just need to get adapted the existing
BATCHED_UNMAP_TLB_FLUSH.

Tested-by: Xin Hao <xhao@linux.alibaba.com>
Signed-off-by: Barry Song <v-songbaohua@oppo.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 Documentation/features/arch-support.txt        | 1 -
 Documentation/features/vm/TLB/arch-support.txt | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

Comments

Anshuman Khandual Sept. 9, 2022, 4:26 a.m. UTC | #1
On 8/22/22 13:51, Yicong Yang wrote:
> From: Barry Song <v-songbaohua@oppo.com>
> 
> This reverts commit 6bfef171d0d74cb050112e0e49feb20bfddf7f42.
> 
> I was wrong. Though ARM64 has hardware TLB flush, but it is not free
> and it is still expensive.
> We still have a good chance to enable batched and deferred TLB flush
> on ARM64 for memory reclamation. A possible way is that we only queue
> tlbi instructions in hardware's queue. When we have to broadcast TLB,
> we broadcast it by dsb. We just need to get adapted the existing
> BATCHED_UNMAP_TLB_FLUSH.
> 
> Tested-by: Xin Hao <xhao@linux.alibaba.com>
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
>  Documentation/features/arch-support.txt        | 1 -
>  Documentation/features/vm/TLB/arch-support.txt | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/Documentation/features/arch-support.txt b/Documentation/features/arch-support.txt
> index 118ae031840b..d22a1095e661 100644
> --- a/Documentation/features/arch-support.txt
> +++ b/Documentation/features/arch-support.txt
> @@ -8,5 +8,4 @@ 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 039e4e91ada3..1c009312b9c1 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: | N/A  |
> +    |       arm64: | TODO |
>      |        csky: | TODO |
>      |     hexagon: | TODO |
>      |        ia64: | TODO |

I believe this patch is not needed, which explicitly reverts an
older commit. Instead when ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
framework gets enabled on arm64, the same patch could just drop
'NA' as possible values for arch support for a give feature in
file Documentation/features/arch-support.txt.
Barry Song Sept. 9, 2022, 4:40 a.m. UTC | #2
On Fri, Sep 9, 2022 at 4:26 PM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 8/22/22 13:51, Yicong Yang wrote:
> > From: Barry Song <v-songbaohua@oppo.com>
> >
> > This reverts commit 6bfef171d0d74cb050112e0e49feb20bfddf7f42.
> >
> > I was wrong. Though ARM64 has hardware TLB flush, but it is not free
> > and it is still expensive.
> > We still have a good chance to enable batched and deferred TLB flush
> > on ARM64 for memory reclamation. A possible way is that we only queue
> > tlbi instructions in hardware's queue. When we have to broadcast TLB,
> > we broadcast it by dsb. We just need to get adapted the existing
> > BATCHED_UNMAP_TLB_FLUSH.
> >
> > Tested-by: Xin Hao <xhao@linux.alibaba.com>
> > Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> > ---
> >  Documentation/features/arch-support.txt        | 1 -
> >  Documentation/features/vm/TLB/arch-support.txt | 2 +-
> >  2 files changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/Documentation/features/arch-support.txt b/Documentation/features/arch-support.txt
> > index 118ae031840b..d22a1095e661 100644
> > --- a/Documentation/features/arch-support.txt
> > +++ b/Documentation/features/arch-support.txt
> > @@ -8,5 +8,4 @@ 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 039e4e91ada3..1c009312b9c1 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: | N/A  |
> > +    |       arm64: | TODO |
> >      |        csky: | TODO |
> >      |     hexagon: | TODO |
> >      |        ia64: | TODO |
>
> I believe this patch is not needed, which explicitly reverts an
> older commit. Instead when ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
> framework gets enabled on arm64, the same patch could just drop
> 'NA' as possible values for arch support for a give feature in
> file Documentation/features/arch-support.txt.

Sure. it is certainly ok to fix this in
arm64: support batched/deferred tlb shootdown during page reclamation

By a separate patch, I was trying to highlight that my previous patch was
wrong. but, yes. it is not fundamentally necessary.

Thanks
Barry
diff mbox series

Patch

diff --git a/Documentation/features/arch-support.txt b/Documentation/features/arch-support.txt
index 118ae031840b..d22a1095e661 100644
--- a/Documentation/features/arch-support.txt
+++ b/Documentation/features/arch-support.txt
@@ -8,5 +8,4 @@  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 039e4e91ada3..1c009312b9c1 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: | N/A  |
+    |       arm64: | TODO |
     |        csky: | TODO |
     |     hexagon: | TODO |
     |        ia64: | TODO |