2022-08-21 14:58:53

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH v6 RESEND 0/2] use static key to optimize pgtable_l4_enabled

The pgtable_l4|[l5]_enabled check sits at hot code path, performance
is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
boot, so static key can be used to solve the performance issue[1].

An unified way static key was introduced in [2], but it only targets
riscv isa extension. We dunno whether SV48 and SV57 will be considered
as isa extension, so the unified solution isn't used for
pgtable_l4[l5]_enabled now.

patch1 fixes a NULL pointer deference if static key is used a bit earlier.
patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.

[1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
[2] https://lore.kernel.org/linux-riscv/[email protected]/T/#t

Since v5:
- Use DECLARE_STATIC_KEY_FALSE

Since v4:
- rebased on v5.19-rcN
- collect Reviewed-by tags
- Fix kernel panic issue if SPARSEMEM is enabled by moving the
riscv_finalise_pgtable_lx() after sparse_init()

Since v3:
- fix W=1 call to undeclared function 'static_branch_likely' error

Since v2:
- move the W=1 warning fix to a separate patch
- move the unified way to use static key to a new patch series.

Since v1:
- Add a W=1 warning fix
- Fix W=1 error
- Based on v5.18-rcN, since SV57 support is added, so convert
pgtable_l5_enabled as well.


Jisheng Zhang (2):
riscv: move sbi_init() earlier before jump_label_init()
riscv: turn pgtable_l4|[l5]_enabled to static key for RV64

arch/riscv/include/asm/pgalloc.h | 16 ++++----
arch/riscv/include/asm/pgtable-32.h | 3 ++
arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
arch/riscv/include/asm/pgtable.h | 5 +--
arch/riscv/kernel/cpu.c | 4 +-
arch/riscv/kernel/setup.c | 2 +-
arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
arch/riscv/mm/kasan_init.c | 16 ++++----
8 files changed, 104 insertions(+), 66 deletions(-)

--
2.34.1


2022-10-06 01:07:48

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v6 RESEND 0/2] use static key to optimize pgtable_l4_enabled

On Sun, 21 Aug 2022 07:09:16 PDT (-0700), [email protected] wrote:
> The pgtable_l4|[l5]_enabled check sits at hot code path, performance
> is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
> boot, so static key can be used to solve the performance issue[1].
>
> An unified way static key was introduced in [2], but it only targets
> riscv isa extension. We dunno whether SV48 and SV57 will be considered
> as isa extension, so the unified solution isn't used for
> pgtable_l4[l5]_enabled now.
>
> patch1 fixes a NULL pointer deference if static key is used a bit earlier.
> patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.
>
> [1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
> [2] https://lore.kernel.org/linux-riscv/[email protected]/T/#t
>
> Since v5:
> - Use DECLARE_STATIC_KEY_FALSE
>
> Since v4:
> - rebased on v5.19-rcN
> - collect Reviewed-by tags
> - Fix kernel panic issue if SPARSEMEM is enabled by moving the
> riscv_finalise_pgtable_lx() after sparse_init()
>
> Since v3:
> - fix W=1 call to undeclared function 'static_branch_likely' error
>
> Since v2:
> - move the W=1 warning fix to a separate patch
> - move the unified way to use static key to a new patch series.
>
> Since v1:
> - Add a W=1 warning fix
> - Fix W=1 error
> - Based on v5.18-rcN, since SV57 support is added, so convert
> pgtable_l5_enabled as well.
>
>
> Jisheng Zhang (2):
> riscv: move sbi_init() earlier before jump_label_init()
> riscv: turn pgtable_l4|[l5]_enabled to static key for RV64
>
> arch/riscv/include/asm/pgalloc.h | 16 ++++----
> arch/riscv/include/asm/pgtable-32.h | 3 ++
> arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
> arch/riscv/include/asm/pgtable.h | 5 +--
> arch/riscv/kernel/cpu.c | 4 +-
> arch/riscv/kernel/setup.c | 2 +-
> arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
> arch/riscv/mm/kasan_init.c | 16 ++++----
> 8 files changed, 104 insertions(+), 66 deletions(-)

Sorry for being slow here, but it looks like this still causes some
early boot hangs. Specifically kasan+sparsemem is failing. As you can
probably see from the latency I'm still a bit buried right now so I'm
not sure when I'll have a chance to take more of a look.

2022-10-06 09:10:40

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH v6 RESEND 0/2] use static key to optimize pgtable_l4_enabled

On Wed, Oct 05, 2022 at 06:05:28PM -0700, Palmer Dabbelt wrote:
> On Sun, 21 Aug 2022 07:09:16 PDT (-0700), [email protected] wrote:
> > The pgtable_l4|[l5]_enabled check sits at hot code path, performance
> > is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
> > boot, so static key can be used to solve the performance issue[1].
> >
> > An unified way static key was introduced in [2], but it only targets
> > riscv isa extension. We dunno whether SV48 and SV57 will be considered
> > as isa extension, so the unified solution isn't used for
> > pgtable_l4[l5]_enabled now.
> >
> > patch1 fixes a NULL pointer deference if static key is used a bit earlier.
> > patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.
> >
> > [1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
> > [2] https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> >
> > Since v5:
> > - Use DECLARE_STATIC_KEY_FALSE
> >
> > Since v4:
> > - rebased on v5.19-rcN
> > - collect Reviewed-by tags
> > - Fix kernel panic issue if SPARSEMEM is enabled by moving the
> > riscv_finalise_pgtable_lx() after sparse_init()
> >
> > Since v3:
> > - fix W=1 call to undeclared function 'static_branch_likely' error
> >
> > Since v2:
> > - move the W=1 warning fix to a separate patch
> > - move the unified way to use static key to a new patch series.
> >
> > Since v1:
> > - Add a W=1 warning fix
> > - Fix W=1 error
> > - Based on v5.18-rcN, since SV57 support is added, so convert
> > pgtable_l5_enabled as well.
> >
> >
> > Jisheng Zhang (2):
> > riscv: move sbi_init() earlier before jump_label_init()
> > riscv: turn pgtable_l4|[l5]_enabled to static key for RV64
> >
> > arch/riscv/include/asm/pgalloc.h | 16 ++++----
> > arch/riscv/include/asm/pgtable-32.h | 3 ++
> > arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
> > arch/riscv/include/asm/pgtable.h | 5 +--
> > arch/riscv/kernel/cpu.c | 4 +-
> > arch/riscv/kernel/setup.c | 2 +-
> > arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
> > arch/riscv/mm/kasan_init.c | 16 ++++----
> > 8 files changed, 104 insertions(+), 66 deletions(-)
>
> Sorry for being slow here, but it looks like this still causes some early
> boot hangs. Specifically kasan+sparsemem is failing. As you can probably
> see from the latency I'm still a bit buried right now so I'm not sure when
> I'll have a chance to take more of a look.

Hi Palmer,

Before V4, there is a bug which can cause kernel panic when SPARSEMEM
is enabled, V4 have fixed it by moving the riscv_finalise_pgtable_lx()
after sparse_init(). And I just tested the riscv-pgtable_static_key
branch in your tree, enabling KASAN and SPARSEMEM, system booted fine.
I'm not sure what happened. Could you please send me your kernel
config file? I want to fix any issue which can block this series being
merged in 6.1-rc1.

Thanks in advance

2022-10-08 15:05:26

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH v6 RESEND 0/2] use static key to optimize pgtable_l4_enabled

On Thu, Oct 06, 2022 at 04:37:57PM +0800, Jisheng Zhang wrote:
> On Wed, Oct 05, 2022 at 06:05:28PM -0700, Palmer Dabbelt wrote:
> > On Sun, 21 Aug 2022 07:09:16 PDT (-0700), [email protected] wrote:
> > > The pgtable_l4|[l5]_enabled check sits at hot code path, performance
> > > is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
> > > boot, so static key can be used to solve the performance issue[1].
> > >
> > > An unified way static key was introduced in [2], but it only targets
> > > riscv isa extension. We dunno whether SV48 and SV57 will be considered
> > > as isa extension, so the unified solution isn't used for
> > > pgtable_l4[l5]_enabled now.
> > >
> > > patch1 fixes a NULL pointer deference if static key is used a bit earlier.
> > > patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.
> > >
> > > [1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
> > > [2] https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> > >
> > > Since v5:
> > > - Use DECLARE_STATIC_KEY_FALSE
> > >
> > > Since v4:
> > > - rebased on v5.19-rcN
> > > - collect Reviewed-by tags
> > > - Fix kernel panic issue if SPARSEMEM is enabled by moving the
> > > riscv_finalise_pgtable_lx() after sparse_init()
> > >
> > > Since v3:
> > > - fix W=1 call to undeclared function 'static_branch_likely' error
> > >
> > > Since v2:
> > > - move the W=1 warning fix to a separate patch
> > > - move the unified way to use static key to a new patch series.
> > >
> > > Since v1:
> > > - Add a W=1 warning fix
> > > - Fix W=1 error
> > > - Based on v5.18-rcN, since SV57 support is added, so convert
> > > pgtable_l5_enabled as well.
> > >
> > >
> > > Jisheng Zhang (2):
> > > riscv: move sbi_init() earlier before jump_label_init()
> > > riscv: turn pgtable_l4|[l5]_enabled to static key for RV64
> > >
> > > arch/riscv/include/asm/pgalloc.h | 16 ++++----
> > > arch/riscv/include/asm/pgtable-32.h | 3 ++
> > > arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
> > > arch/riscv/include/asm/pgtable.h | 5 +--
> > > arch/riscv/kernel/cpu.c | 4 +-
> > > arch/riscv/kernel/setup.c | 2 +-
> > > arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
> > > arch/riscv/mm/kasan_init.c | 16 ++++----
> > > 8 files changed, 104 insertions(+), 66 deletions(-)
> >
> > Sorry for being slow here, but it looks like this still causes some early
> > boot hangs. Specifically kasan+sparsemem is failing. As you can probably
> > see from the latency I'm still a bit buried right now so I'm not sure when
> > I'll have a chance to take more of a look.
>
> Hi Palmer,
>
> Before V4, there is a bug which can cause kernel panic when SPARSEMEM
> is enabled, V4 have fixed it by moving the riscv_finalise_pgtable_lx()
> after sparse_init(). And I just tested the riscv-pgtable_static_key
> branch in your tree, enabling KASAN and SPARSEMEM, system booted fine.
> I'm not sure what happened. Could you please send me your kernel
> config file? I want to fix any issue which can block this series being
> merged in 6.1-rc1.

Hi Palmer,

I know you are busy ;) Do you have time to send me your test kernel
config file so that I can reproduce the "early boot hang"?

Thanks

2022-12-11 11:30:03

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH v6 RESEND 0/2] use static key to optimize pgtable_l4_enabled

On Sat, Oct 08, 2022 at 10:28:35PM +0800, Jisheng Zhang wrote:
> On Thu, Oct 06, 2022 at 04:37:57PM +0800, Jisheng Zhang wrote:
> > On Wed, Oct 05, 2022 at 06:05:28PM -0700, Palmer Dabbelt wrote:
> > > On Sun, 21 Aug 2022 07:09:16 PDT (-0700), [email protected] wrote:
> > > > The pgtable_l4|[l5]_enabled check sits at hot code path, performance
> > > > is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
> > > > boot, so static key can be used to solve the performance issue[1].
> > > >
> > > > An unified way static key was introduced in [2], but it only targets
> > > > riscv isa extension. We dunno whether SV48 and SV57 will be considered
> > > > as isa extension, so the unified solution isn't used for
> > > > pgtable_l4[l5]_enabled now.
> > > >
> > > > patch1 fixes a NULL pointer deference if static key is used a bit earlier.
> > > > patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.
> > > >
> > > > [1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
> > > > [2] https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> > > >
> > > > Since v5:
> > > > - Use DECLARE_STATIC_KEY_FALSE
> > > >
> > > > Since v4:
> > > > - rebased on v5.19-rcN
> > > > - collect Reviewed-by tags
> > > > - Fix kernel panic issue if SPARSEMEM is enabled by moving the
> > > > riscv_finalise_pgtable_lx() after sparse_init()
> > > >
> > > > Since v3:
> > > > - fix W=1 call to undeclared function 'static_branch_likely' error
> > > >
> > > > Since v2:
> > > > - move the W=1 warning fix to a separate patch
> > > > - move the unified way to use static key to a new patch series.
> > > >
> > > > Since v1:
> > > > - Add a W=1 warning fix
> > > > - Fix W=1 error
> > > > - Based on v5.18-rcN, since SV57 support is added, so convert
> > > > pgtable_l5_enabled as well.
> > > >
> > > >
> > > > Jisheng Zhang (2):
> > > > riscv: move sbi_init() earlier before jump_label_init()
> > > > riscv: turn pgtable_l4|[l5]_enabled to static key for RV64
> > > >
> > > > arch/riscv/include/asm/pgalloc.h | 16 ++++----
> > > > arch/riscv/include/asm/pgtable-32.h | 3 ++
> > > > arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
> > > > arch/riscv/include/asm/pgtable.h | 5 +--
> > > > arch/riscv/kernel/cpu.c | 4 +-
> > > > arch/riscv/kernel/setup.c | 2 +-
> > > > arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
> > > > arch/riscv/mm/kasan_init.c | 16 ++++----
> > > > 8 files changed, 104 insertions(+), 66 deletions(-)
> > >
> > > Sorry for being slow here, but it looks like this still causes some early
> > > boot hangs. Specifically kasan+sparsemem is failing. As you can probably
> > > see from the latency I'm still a bit buried right now so I'm not sure when
> > > I'll have a chance to take more of a look.
> >
> > Hi Palmer,
> >
> > Before V4, there is a bug which can cause kernel panic when SPARSEMEM
> > is enabled, V4 have fixed it by moving the riscv_finalise_pgtable_lx()
> > after sparse_init(). And I just tested the riscv-pgtable_static_key
> > branch in your tree, enabling KASAN and SPARSEMEM, system booted fine.
> > I'm not sure what happened. Could you please send me your kernel
> > config file? I want to fix any issue which can block this series being
> > merged in 6.1-rc1.
>
> Hi Palmer,
>
> I know you are busy ;) Do you have time to send me your test kernel
> config file so that I can reproduce the "early boot hang"?
>
> Thanks

Hi Palmer,

I think the early boot hangs maybe the same as the one which has been
fixed by commit 9f2ac64d6ca6 ("riscv: mm: add missing memcpy in
kasan_init"). Will you give this series another try for v6.2-rc1? If
the boot hang can still be reproduced, could you please send me your
.config file?

Thanks in advance

2023-01-11 18:31:49

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH v6 RESEND 0/2] use static key to optimize pgtable_l4_enabled

On Sun, Dec 11, 2022 at 06:44:04PM +0800, Jisheng Zhang wrote:
> On Sat, Oct 08, 2022 at 10:28:35PM +0800, Jisheng Zhang wrote:
> > On Thu, Oct 06, 2022 at 04:37:57PM +0800, Jisheng Zhang wrote:
> > > On Wed, Oct 05, 2022 at 06:05:28PM -0700, Palmer Dabbelt wrote:
> > > > On Sun, 21 Aug 2022 07:09:16 PDT (-0700), [email protected] wrote:
> > > > > The pgtable_l4|[l5]_enabled check sits at hot code path, performance
> > > > > is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
> > > > > boot, so static key can be used to solve the performance issue[1].
> > > > >
> > > > > An unified way static key was introduced in [2], but it only targets
> > > > > riscv isa extension. We dunno whether SV48 and SV57 will be considered
> > > > > as isa extension, so the unified solution isn't used for
> > > > > pgtable_l4[l5]_enabled now.
> > > > >
> > > > > patch1 fixes a NULL pointer deference if static key is used a bit earlier.
> > > > > patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.
> > > > >
> > > > > [1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
> > > > > [2] https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> > > > >
> > > > > Since v5:
> > > > > - Use DECLARE_STATIC_KEY_FALSE
> > > > >
> > > > > Since v4:
> > > > > - rebased on v5.19-rcN
> > > > > - collect Reviewed-by tags
> > > > > - Fix kernel panic issue if SPARSEMEM is enabled by moving the
> > > > > riscv_finalise_pgtable_lx() after sparse_init()
> > > > >
> > > > > Since v3:
> > > > > - fix W=1 call to undeclared function 'static_branch_likely' error
> > > > >
> > > > > Since v2:
> > > > > - move the W=1 warning fix to a separate patch
> > > > > - move the unified way to use static key to a new patch series.
> > > > >
> > > > > Since v1:
> > > > > - Add a W=1 warning fix
> > > > > - Fix W=1 error
> > > > > - Based on v5.18-rcN, since SV57 support is added, so convert
> > > > > pgtable_l5_enabled as well.
> > > > >
> > > > >
> > > > > Jisheng Zhang (2):
> > > > > riscv: move sbi_init() earlier before jump_label_init()
> > > > > riscv: turn pgtable_l4|[l5]_enabled to static key for RV64
> > > > >
> > > > > arch/riscv/include/asm/pgalloc.h | 16 ++++----
> > > > > arch/riscv/include/asm/pgtable-32.h | 3 ++
> > > > > arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
> > > > > arch/riscv/include/asm/pgtable.h | 5 +--
> > > > > arch/riscv/kernel/cpu.c | 4 +-
> > > > > arch/riscv/kernel/setup.c | 2 +-
> > > > > arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
> > > > > arch/riscv/mm/kasan_init.c | 16 ++++----
> > > > > 8 files changed, 104 insertions(+), 66 deletions(-)
> > > >
> > > > Sorry for being slow here, but it looks like this still causes some early
> > > > boot hangs. Specifically kasan+sparsemem is failing. As you can probably
> > > > see from the latency I'm still a bit buried right now so I'm not sure when
> > > > I'll have a chance to take more of a look.
> > >
> > > Hi Palmer,
> > >
> > > Before V4, there is a bug which can cause kernel panic when SPARSEMEM
> > > is enabled, V4 have fixed it by moving the riscv_finalise_pgtable_lx()
> > > after sparse_init(). And I just tested the riscv-pgtable_static_key
> > > branch in your tree, enabling KASAN and SPARSEMEM, system booted fine.
> > > I'm not sure what happened. Could you please send me your kernel
> > > config file? I want to fix any issue which can block this series being
> > > merged in 6.1-rc1.
> >
> > Hi Palmer,
> >
> > I know you are busy ;) Do you have time to send me your test kernel
> > config file so that I can reproduce the "early boot hang"?
> >
> > Thanks
>
> Hi Palmer,
>
> I think the early boot hangs maybe the same as the one which has been
> fixed by commit 9f2ac64d6ca6 ("riscv: mm: add missing memcpy in
> kasan_init"). Will you give this series another try for v6.2-rc1? If
> the boot hang can still be reproduced, could you please send me your
> .config file?
>
> Thanks in advance
Hi all,

Just request to comment what to do with this patch, I think there
are two independent points to consult:

1. IIRC, Palmer gave this patch two chances to merge in early versions
but he found boot hangs if enable KASAN and SPARSEMEM, while I can't
reproduce the boot hang. And I also expect the hang should be fixed by
commit 9f2ac64d6ca6 ("riscv: mm: add missing memcpy in kasan_init")

2. Now we know alternative is preferred than static branch for ISA
extensions dynamic code patching. So we also need to switch static
branch usage here to alternative mechanism, but the problem is
SV48 and SV57 are not ISA extensions, so we can't directly make use
of the recently introduced riscv_has_extension_likely|unlikely()[1]
which is based on alternative mechanism.

Any comments are appreciated.

Thanks in advance

[1] https://lore.kernel.org/linux-riscv/[email protected]/T/#t

2023-01-11 19:18:18

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v6 RESEND 0/2] use static key to optimize pgtable_l4_enabled

On Thu, Jan 12, 2023 at 01:28:40AM +0800, Jisheng Zhang wrote:
> On Sun, Dec 11, 2022 at 06:44:04PM +0800, Jisheng Zhang wrote:
> > On Sat, Oct 08, 2022 at 10:28:35PM +0800, Jisheng Zhang wrote:
> > > On Thu, Oct 06, 2022 at 04:37:57PM +0800, Jisheng Zhang wrote:
> > > > On Wed, Oct 05, 2022 at 06:05:28PM -0700, Palmer Dabbelt wrote:
> > > > > On Sun, 21 Aug 2022 07:09:16 PDT (-0700), [email protected] wrote:
> > > > > > The pgtable_l4|[l5]_enabled check sits at hot code path, performance
> > > > > > is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
> > > > > > boot, so static key can be used to solve the performance issue[1].
> > > > > >
> > > > > > An unified way static key was introduced in [2], but it only targets
> > > > > > riscv isa extension. We dunno whether SV48 and SV57 will be considered
> > > > > > as isa extension, so the unified solution isn't used for
> > > > > > pgtable_l4[l5]_enabled now.
> > > > > >
> > > > > > patch1 fixes a NULL pointer deference if static key is used a bit earlier.
> > > > > > patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.
> > > > > >
> > > > > > [1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
> > > > > > [2] https://lore.kernel.org/linux-riscv/[email protected]/T/#t
> > > > > >
> > > > > > Since v5:
> > > > > > - Use DECLARE_STATIC_KEY_FALSE
> > > > > >
> > > > > > Since v4:
> > > > > > - rebased on v5.19-rcN
> > > > > > - collect Reviewed-by tags
> > > > > > - Fix kernel panic issue if SPARSEMEM is enabled by moving the
> > > > > > riscv_finalise_pgtable_lx() after sparse_init()
> > > > > >
> > > > > > Since v3:
> > > > > > - fix W=1 call to undeclared function 'static_branch_likely' error
> > > > > >
> > > > > > Since v2:
> > > > > > - move the W=1 warning fix to a separate patch
> > > > > > - move the unified way to use static key to a new patch series.
> > > > > >
> > > > > > Since v1:
> > > > > > - Add a W=1 warning fix
> > > > > > - Fix W=1 error
> > > > > > - Based on v5.18-rcN, since SV57 support is added, so convert
> > > > > > pgtable_l5_enabled as well.
> > > > > >
> > > > > >
> > > > > > Jisheng Zhang (2):
> > > > > > riscv: move sbi_init() earlier before jump_label_init()
> > > > > > riscv: turn pgtable_l4|[l5]_enabled to static key for RV64
> > > > > >
> > > > > > arch/riscv/include/asm/pgalloc.h | 16 ++++----
> > > > > > arch/riscv/include/asm/pgtable-32.h | 3 ++
> > > > > > arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
> > > > > > arch/riscv/include/asm/pgtable.h | 5 +--
> > > > > > arch/riscv/kernel/cpu.c | 4 +-
> > > > > > arch/riscv/kernel/setup.c | 2 +-
> > > > > > arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
> > > > > > arch/riscv/mm/kasan_init.c | 16 ++++----
> > > > > > 8 files changed, 104 insertions(+), 66 deletions(-)
> > > > >
> > > > > Sorry for being slow here, but it looks like this still causes some early
> > > > > boot hangs. Specifically kasan+sparsemem is failing. As you can probably
> > > > > see from the latency I'm still a bit buried right now so I'm not sure when
> > > > > I'll have a chance to take more of a look.
> > > >
> > > > Hi Palmer,
> > > >
> > > > Before V4, there is a bug which can cause kernel panic when SPARSEMEM
> > > > is enabled, V4 have fixed it by moving the riscv_finalise_pgtable_lx()
> > > > after sparse_init(). And I just tested the riscv-pgtable_static_key
> > > > branch in your tree, enabling KASAN and SPARSEMEM, system booted fine.
> > > > I'm not sure what happened. Could you please send me your kernel
> > > > config file? I want to fix any issue which can block this series being
> > > > merged in 6.1-rc1.
> > >
> > > Hi Palmer,
> > >
> > > I know you are busy ;) Do you have time to send me your test kernel
> > > config file so that I can reproduce the "early boot hang"?
> > >
> > > Thanks
> >
> > Hi Palmer,
> >
> > I think the early boot hangs maybe the same as the one which has been
> > fixed by commit 9f2ac64d6ca6 ("riscv: mm: add missing memcpy in
> > kasan_init"). Will you give this series another try for v6.2-rc1? If
> > the boot hang can still be reproduced, could you please send me your
> > .config file?
> >
> > Thanks in advance
> Hi all,
>
> Just request to comment what to do with this patch, I think there
> are two independent points to consult:
>
> 1. IIRC, Palmer gave this patch two chances to merge in early versions
> but he found boot hangs if enable KASAN and SPARSEMEM, while I can't
> reproduce the boot hang. And I also expect the hang should be fixed by
> commit 9f2ac64d6ca6 ("riscv: mm: add missing memcpy in kasan_init")
>
> 2. Now we know alternative is preferred than static branch for ISA
> extensions dynamic code patching. So we also need to switch static
> branch usage here to alternative mechanism, but the problem is
> SV48 and SV57 are not ISA extensions, so we can't directly make use
> of the recently introduced riscv_has_extension_likely|unlikely()[1]
> which is based on alternative mechanism.

We could rename the "has_extension" framework to "has_cpufeature" and
then lump extensions and features such as sv48 and sv57 together. Or,
if it's best to keep extensions separate, then duplicate the framework
to create a "has_non_extension_feature" version where features like
sv48 and sv57 live.

Thanks,
drew

>
> Any comments are appreciated.
>
> Thanks in advance
>
> [1] https://lore.kernel.org/linux-riscv/[email protected]/T/#t
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-01-11 21:08:12

by Jessica Clarke

[permalink] [raw]
Subject: Re: [PATCH v6 RESEND 0/2] use static key to optimize pgtable_l4_enabled

On 11 Jan 2023, at 19:00, Andrew Jones <[email protected]> wrote:
> On Thu, Jan 12, 2023 at 01:28:40AM +0800, Jisheng Zhang wrote:
>> On Sun, Dec 11, 2022 at 06:44:04PM +0800, Jisheng Zhang wrote:
>>> On Sat, Oct 08, 2022 at 10:28:35PM +0800, Jisheng Zhang wrote:
>>>> On Thu, Oct 06, 2022 at 04:37:57PM +0800, Jisheng Zhang wrote:
>>>>> On Wed, Oct 05, 2022 at 06:05:28PM -0700, Palmer Dabbelt wrote:
>>>>>> On Sun, 21 Aug 2022 07:09:16 PDT (-0700), [email protected] wrote:
>>>>>>> The pgtable_l4|[l5]_enabled check sits at hot code path, performance
>>>>>>> is impacted a lot. Since pgtable_l4|[l5]_enabled isn't changed after
>>>>>>> boot, so static key can be used to solve the performance issue[1].
>>>>>>>
>>>>>>> An unified way static key was introduced in [2], but it only targets
>>>>>>> riscv isa extension. We dunno whether SV48 and SV57 will be considered
>>>>>>> as isa extension, so the unified solution isn't used for
>>>>>>> pgtable_l4[l5]_enabled now.
>>>>>>>
>>>>>>> patch1 fixes a NULL pointer deference if static key is used a bit earlier.
>>>>>>> patch2 uses the static key to optimize pgtable_l4|[l5]_enabled.
>>>>>>>
>>>>>>> [1] http://lists.infradead.org/pipermail/linux-riscv/2021-December/011164.html
>>>>>>> [2] https://lore.kernel.org/linux-riscv/[email protected]/T/#t
>>>>>>>
>>>>>>> Since v5:
>>>>>>> - Use DECLARE_STATIC_KEY_FALSE
>>>>>>>
>>>>>>> Since v4:
>>>>>>> - rebased on v5.19-rcN
>>>>>>> - collect Reviewed-by tags
>>>>>>> - Fix kernel panic issue if SPARSEMEM is enabled by moving the
>>>>>>> riscv_finalise_pgtable_lx() after sparse_init()
>>>>>>>
>>>>>>> Since v3:
>>>>>>> - fix W=1 call to undeclared function 'static_branch_likely' error
>>>>>>>
>>>>>>> Since v2:
>>>>>>> - move the W=1 warning fix to a separate patch
>>>>>>> - move the unified way to use static key to a new patch series.
>>>>>>>
>>>>>>> Since v1:
>>>>>>> - Add a W=1 warning fix
>>>>>>> - Fix W=1 error
>>>>>>> - Based on v5.18-rcN, since SV57 support is added, so convert
>>>>>>> pgtable_l5_enabled as well.
>>>>>>>
>>>>>>>
>>>>>>> Jisheng Zhang (2):
>>>>>>> riscv: move sbi_init() earlier before jump_label_init()
>>>>>>> riscv: turn pgtable_l4|[l5]_enabled to static key for RV64
>>>>>>>
>>>>>>> arch/riscv/include/asm/pgalloc.h | 16 ++++----
>>>>>>> arch/riscv/include/asm/pgtable-32.h | 3 ++
>>>>>>> arch/riscv/include/asm/pgtable-64.h | 60 ++++++++++++++++++---------
>>>>>>> arch/riscv/include/asm/pgtable.h | 5 +--
>>>>>>> arch/riscv/kernel/cpu.c | 4 +-
>>>>>>> arch/riscv/kernel/setup.c | 2 +-
>>>>>>> arch/riscv/mm/init.c | 64 ++++++++++++++++++-----------
>>>>>>> arch/riscv/mm/kasan_init.c | 16 ++++----
>>>>>>> 8 files changed, 104 insertions(+), 66 deletions(-)
>>>>>>
>>>>>> Sorry for being slow here, but it looks like this still causes some early
>>>>>> boot hangs. Specifically kasan+sparsemem is failing. As you can probably
>>>>>> see from the latency I'm still a bit buried right now so I'm not sure when
>>>>>> I'll have a chance to take more of a look.
>>>>>
>>>>> Hi Palmer,
>>>>>
>>>>> Before V4, there is a bug which can cause kernel panic when SPARSEMEM
>>>>> is enabled, V4 have fixed it by moving the riscv_finalise_pgtable_lx()
>>>>> after sparse_init(). And I just tested the riscv-pgtable_static_key
>>>>> branch in your tree, enabling KASAN and SPARSEMEM, system booted fine.
>>>>> I'm not sure what happened. Could you please send me your kernel
>>>>> config file? I want to fix any issue which can block this series being
>>>>> merged in 6.1-rc1.
>>>>
>>>> Hi Palmer,
>>>>
>>>> I know you are busy ;) Do you have time to send me your test kernel
>>>> config file so that I can reproduce the "early boot hang"?
>>>>
>>>> Thanks
>>>
>>> Hi Palmer,
>>>
>>> I think the early boot hangs maybe the same as the one which has been
>>> fixed by commit 9f2ac64d6ca6 ("riscv: mm: add missing memcpy in
>>> kasan_init"). Will you give this series another try for v6.2-rc1? If
>>> the boot hang can still be reproduced, could you please send me your
>>> .config file?
>>>
>>> Thanks in advance
>> Hi all,
>>
>> Just request to comment what to do with this patch, I think there
>> are two independent points to consult:
>>
>> 1. IIRC, Palmer gave this patch two chances to merge in early versions
>> but he found boot hangs if enable KASAN and SPARSEMEM, while I can't
>> reproduce the boot hang. And I also expect the hang should be fixed by
>> commit 9f2ac64d6ca6 ("riscv: mm: add missing memcpy in kasan_init")
>>
>> 2. Now we know alternative is preferred than static branch for ISA
>> extensions dynamic code patching. So we also need to switch static
>> branch usage here to alternative mechanism, but the problem is
>> SV48 and SV57 are not ISA extensions, so we can't directly make use
>> of the recently introduced riscv_has_extension_likely|unlikely()[1]
>> which is based on alternative mechanism.
>
> We could rename the "has_extension" framework to "has_cpufeature" and
> then lump extensions and features such as sv48 and sv57 together. Or,
> if it's best to keep extensions separate, then duplicate the framework
> to create a "has_non_extension_feature" version where features like
> sv48 and sv57 live.

Sv39, Sv48 and Sv57 are extensions these days (see the draft profiles
spec[1] and [2]).

Jess

[1] https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc#522-rva20s64-mandatory-extensions
[2] https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions

> Thanks,
> drew
>
>>
>> Any comments are appreciated.
>>
>> Thanks in advance
>>
>> [1] https://lore.kernel.org/linux-riscv/[email protected]/T/#t
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv