| Message ID | 20221028231929.347918-1-atishp@rivosinc.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | RISC-V: Do not issue remote fences until smp is available | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/apply | fail | Patch does not apply to for-next |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
On Fri, Oct 28, 2022 at 04:19:29PM -0700, Atish Patra wrote: > It is useless to issue remote fences if there is a single core > available. It becomes a bottleneck for sbi based rfences where > we will be making those ECALLs for no reason. Early code patching > because of static calls end up in this path. > > Signed-off-by: Atish Patra <atishp@rivosinc.com> Hey Atish, This doesn't apply for me to either fixes or for-next. What branch does it apply to? Thanks, Conor. > --- > arch/riscv/mm/cacheflush.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > index f10cb47eac3a..7fafc8c26505 100644 > --- a/arch/riscv/mm/cacheflush.c > +++ b/arch/riscv/mm/cacheflush.c > @@ -19,6 +19,10 @@ void flush_icache_all(void) > { > local_flush_icache_all(); > > + /* No need to issue remote fence if only 1 cpu is online */ > + if (num_online_cpus() == 1) > + return; > + > if (IS_ENABLED(CONFIG_RISCV_SBI) && !riscv_use_ipi_for_rfence()) > sbi_remote_fence_i(NULL); > else > -- > 2.34.1 >
On Mon, Oct 31, 2022 at 12:12 PM Conor Dooley <conor@kernel.org> wrote: > > On Fri, Oct 28, 2022 at 04:19:29PM -0700, Atish Patra wrote: > > It is useless to issue remote fences if there is a single core > > available. It becomes a bottleneck for sbi based rfences where > > we will be making those ECALLs for no reason. Early code patching > > because of static calls end up in this path. > > > > Signed-off-by: Atish Patra <atishp@rivosinc.com> > > Hey Atish, > This doesn't apply for me to either fixes or for-next. What branch does > it apply to? > Thanks, > Conor. > > > --- > > arch/riscv/mm/cacheflush.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > > index f10cb47eac3a..7fafc8c26505 100644 > > --- a/arch/riscv/mm/cacheflush.c > > +++ b/arch/riscv/mm/cacheflush.c > > @@ -19,6 +19,10 @@ void flush_icache_all(void) > > { > > local_flush_icache_all(); > > > > + /* No need to issue remote fence if only 1 cpu is online */ > > + if (num_online_cpus() == 1) > > + return; > > + > > if (IS_ENABLED(CONFIG_RISCV_SBI) && !riscv_use_ipi_for_rfence()) > > sbi_remote_fence_i(NULL); > > else > > -- > > 2.34.1 > > Sorry I forgot to specify the dependencies for this patch. This patch is based on Anup's IPI series [1] as I assumed the IPI series would go first. I can rebase on top of the master if required. However, the issue will manifest only after Jisheng's patch[2] which moved the sbi_init to earlier and introduced the static key in the paging_init path. [1] https://patchwork.kernel.org/project/linux-riscv/patch/20220820065446.389788-8-apatel@ventanamicro.com/ [2] https://lore.kernel.org/lkml/20220716115059.3509-1-jszhang@kernel.org/ -- Regards, Atish
On Mon, 31 Oct 2022 12:26:05 PDT (-0700), atishp@atishpatra.org wrote: > On Mon, Oct 31, 2022 at 12:12 PM Conor Dooley <conor@kernel.org> wrote: >> >> On Fri, Oct 28, 2022 at 04:19:29PM -0700, Atish Patra wrote: >> > It is useless to issue remote fences if there is a single core >> > available. It becomes a bottleneck for sbi based rfences where >> > we will be making those ECALLs for no reason. Early code patching >> > because of static calls end up in this path. >> > >> > Signed-off-by: Atish Patra <atishp@rivosinc.com> >> >> Hey Atish, >> This doesn't apply for me to either fixes or for-next. What branch does >> it apply to? >> Thanks, >> Conor. >> >> > --- >> > arch/riscv/mm/cacheflush.c | 4 ++++ >> > 1 file changed, 4 insertions(+) >> > >> > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c >> > index f10cb47eac3a..7fafc8c26505 100644 >> > --- a/arch/riscv/mm/cacheflush.c >> > +++ b/arch/riscv/mm/cacheflush.c >> > @@ -19,6 +19,10 @@ void flush_icache_all(void) >> > { >> > local_flush_icache_all(); >> > >> > + /* No need to issue remote fence if only 1 cpu is online */ >> > + if (num_online_cpus() == 1) >> > + return; >> > + >> > if (IS_ENABLED(CONFIG_RISCV_SBI) && !riscv_use_ipi_for_rfence()) >> > sbi_remote_fence_i(NULL); >> > else >> > -- >> > 2.34.1 >> > > > Sorry I forgot to specify the dependencies for this patch. This patch > is based on Anup's IPI series [1] as > I assumed the IPI series would go first. I can rebase on top of the > master if required. > However, the issue will manifest only after Jisheng's patch[2] which > moved the sbi_init to earlier and introduced the > static key in the paging_init path. > > [1] https://patchwork.kernel.org/project/linux-riscv/patch/20220820065446.389788-8-apatel@ventanamicro.com/ > [2] https://lore.kernel.org/lkml/20220716115059.3509-1-jszhang@kernel.org/ IMO we should just stop issuing the SBI remote fences at all, with the code to do IPI-based fences we're just adding complexity for the slow case.
On Thu, Nov 10, 2022 at 1:42 PM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Mon, 31 Oct 2022 12:26:05 PDT (-0700), atishp@atishpatra.org wrote: > > On Mon, Oct 31, 2022 at 12:12 PM Conor Dooley <conor@kernel.org> wrote: > >> > >> On Fri, Oct 28, 2022 at 04:19:29PM -0700, Atish Patra wrote: > >> > It is useless to issue remote fences if there is a single core > >> > available. It becomes a bottleneck for sbi based rfences where > >> > we will be making those ECALLs for no reason. Early code patching > >> > because of static calls end up in this path. > >> > > >> > Signed-off-by: Atish Patra <atishp@rivosinc.com> > >> > >> Hey Atish, > >> This doesn't apply for me to either fixes or for-next. What branch does > >> it apply to? > >> Thanks, > >> Conor. > >> > >> > --- > >> > arch/riscv/mm/cacheflush.c | 4 ++++ > >> > 1 file changed, 4 insertions(+) > >> > > >> > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > >> > index f10cb47eac3a..7fafc8c26505 100644 > >> > --- a/arch/riscv/mm/cacheflush.c > >> > +++ b/arch/riscv/mm/cacheflush.c > >> > @@ -19,6 +19,10 @@ void flush_icache_all(void) > >> > { > >> > local_flush_icache_all(); > >> > > >> > + /* No need to issue remote fence if only 1 cpu is online */ > >> > + if (num_online_cpus() == 1) > >> > + return; > >> > + > >> > if (IS_ENABLED(CONFIG_RISCV_SBI) && !riscv_use_ipi_for_rfence()) > >> > sbi_remote_fence_i(NULL); > >> > else > >> > -- > >> > 2.34.1 > >> > > > > > Sorry I forgot to specify the dependencies for this patch. This patch > > is based on Anup's IPI series [1] as > > I assumed the IPI series would go first. I can rebase on top of the > > master if required. > > However, the issue will manifest only after Jisheng's patch[2] which > > moved the sbi_init to earlier and introduced the > > static key in the paging_init path. > > > > [1] https://patchwork.kernel.org/project/linux-riscv/patch/20220820065446.389788-8-apatel@ventanamicro.com/ > > [2] https://lore.kernel.org/lkml/20220716115059.3509-1-jszhang@kernel.org/ > > IMO we should just stop issuing the SBI remote fences at all, with the > code to do IPI-based fences we're just adding complexity for the slow > case. Sure. We can do that too. However, that will have some performance impact for any platform(existing and future ones) without imsic. Is that acceptable ? Maybe it will encourage every vendor to implement AIA instead of PLIC ;) -- Regards, Atish
diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c index f10cb47eac3a..7fafc8c26505 100644 --- a/arch/riscv/mm/cacheflush.c +++ b/arch/riscv/mm/cacheflush.c @@ -19,6 +19,10 @@ void flush_icache_all(void) { local_flush_icache_all(); + /* No need to issue remote fence if only 1 cpu is online */ + if (num_online_cpus() == 1) + return; + if (IS_ENABLED(CONFIG_RISCV_SBI) && !riscv_use_ipi_for_rfence()) sbi_remote_fence_i(NULL); else
It is useless to issue remote fences if there is a single core available. It becomes a bottleneck for sbi based rfences where we will be making those ECALLs for no reason. Early code patching because of static calls end up in this path. Signed-off-by: Atish Patra <atishp@rivosinc.com> --- arch/riscv/mm/cacheflush.c | 4 ++++ 1 file changed, 4 insertions(+)