Received-SPF: pass (google.com: domain of linux-kernel+bounces-16630-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240102220134.3229156-1-samuel.holland@sifive.com> <20240102220134.3229156-4-samuel.holland@sifive.com>
In-Reply-To: <20240102220134.3229156-4-samuel.holland@sifive.com>
From: Alexandre Ghiti <alexghiti@rivosinc.com>
Date: Thu, 4 Jan 2024 13:15:06 +0100
Message-ID: <CAHVXubjv58osbweO8AtyubLq=Zd7GW5aaeO6qFdb-QLTKrzT4g@mail.gmail.com>
Subject: Re: [PATCH v4 03/12] riscv: mm: Broadcast kernel TLB flushes only
 when needed
To: Samuel Holland <samuel.holland@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>, linux-riscv@lists.infradead.org, 
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jan 2, 2024 at 11:01=E2=80=AFPM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> __flush_tlb_range() avoids broadcasting TLB flushes when an mm context
> is only active on the local CPU. Apply this same optimization to TLB
> flushes of kernel memory when only one CPU is online. This check can be
> constant-folded when SMP is disabled.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
> Changes in v4:
>  - New patch for v4
>
>  arch/riscv/mm/tlbflush.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index 09b03bf71e6a..2f18fe6fc4f3 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -98,27 +98,23 @@ static void __flush_tlb_range(struct mm_struct *mm, u=
nsigned long start,
>  {
>         const struct cpumask *cmask;
>         unsigned long asid =3D FLUSH_TLB_NO_ASID;
> -       bool broadcast;
> +       unsigned int cpu;
>
>         if (mm) {
> -               unsigned int cpuid;
> -
>                 cmask =3D mm_cpumask(mm);
>                 if (cpumask_empty(cmask))
>                         return;
>
> -               cpuid =3D get_cpu();
> -               /* check if the tlbflush needs to be sent to other CPUs *=
/
> -               broadcast =3D cpumask_any_but(cmask, cpuid) < nr_cpu_ids;
> -
>                 if (static_branch_unlikely(&use_asid_allocator))
>                         asid =3D atomic_long_read(&mm->context.id) & asid=
_mask;
>         } else {
>                 cmask =3D cpu_online_mask;
> -               broadcast =3D true;
>         }
>
> -       if (!broadcast) {
> +       cpu =3D get_cpu();
> +
> +       /* Check if the TLB flush needs to be sent to other CPUs. */
> +       if (cpumask_any_but(cmask, cpu) >=3D nr_cpu_ids) {
>                 local_flush_tlb_range_asid(start, size, stride, asid);
>         } else if (riscv_use_sbi_for_rfence()) {
>                 sbi_remote_sfence_vma_asid(cmask, start, size, asid);
> @@ -132,8 +128,7 @@ static void __flush_tlb_range(struct mm_struct *mm, u=
nsigned long start,
>                 on_each_cpu_mask(cmask, __ipi_flush_tlb_range_asid, &ftd,=
 1);
>         }
>
> -       if (mm)
> -               put_cpu();
> +       put_cpu();
>  }
>
>  void flush_tlb_mm(struct mm_struct *mm)
> --
> 2.42.0
>

You can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex