2023-07-27 16:30:49

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH] riscv: enable DEBUG_FORCE_FUNCTION_ALIGN_64B

Allow to force all function address 64B aligned as it is possible for
other architectures. This may be useful when verify if performance
bump is caused by function alignment changes.

Before commit 1bf18da62106 ("lib/Kconfig.debug: add ARCH dependency
for FUNCTION_ALIGN option"), riscv supports enabling the
DEBUG_FORCE_FUNCTION_ALIGN_64B option, but after that commit, each
arch needs to claim the support explicitly. I tried the config file in
[1] for both RV64 and RV32, I can't reproduce the build error as [1],
there is no reason for not allowing to enforce this function alignment.

Link: https://lore.kernel.org/lkml/[email protected]/ [1]
Signed-off-by: Jisheng Zhang <[email protected]>
---
lib/Kconfig.debug | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index fbc89baf7de6..39ffd218e960 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -502,7 +502,7 @@ config SECTION_MISMATCH_WARN_ONLY

config DEBUG_FORCE_FUNCTION_ALIGN_64B
bool "Force all function address 64B aligned"
- depends on EXPERT && (X86_64 || ARM64 || PPC32 || PPC64 || ARC || S390)
+ depends on EXPERT && (X86_64 || ARM64 || PPC32 || PPC64 || ARC || RISCV || S390)
select FUNCTION_ALIGNMENT_64B
help
There are cases that a commit from one domain changes the function
--
2.40.1



2023-07-29 11:50:32

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] riscv: enable DEBUG_FORCE_FUNCTION_ALIGN_64B

Hey,

On Fri, Jul 28, 2023 at 12:03:56AM +0800, Jisheng Zhang wrote:
> Allow to force all function address 64B aligned as it is possible for
> other architectures. This may be useful when verify if performance
> bump is caused by function alignment changes.
>
> Before commit 1bf18da62106 ("lib/Kconfig.debug: add ARCH dependency
> for FUNCTION_ALIGN option"), riscv supports enabling the
> DEBUG_FORCE_FUNCTION_ALIGN_64B option, but after that commit, each
> arch needs to claim the support explicitly. I tried the config file in
> [1] for both RV64 and RV32, I can't reproduce the build error as [1],
> there is no reason for not allowing to enforce this function alignment.
>
> Link: https://lore.kernel.org/lkml/[email protected]/ [1]

This is a CSKY randconfig, is there any particular reason that running
that randconfig (over a year later) and on a different architecture
would trigger whatever the condition was?

The original commit here seems far too penal - why was it not just
disabled on CSKY??? I tried looking a bit on lore, but didn't see
anything explaining the subset of supported archs they picked.
I did see Guo Ren wondering if rv32 would be similarly problematic - but
since this is something likely to just trip up randconfigs, I think we
should go for it and if rv32 becomes a problem, restrict this to 64-bit.

Reviewed-by: Conor Dooley <[email protected]>

Thanks,
Conor.

> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> lib/Kconfig.debug | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index fbc89baf7de6..39ffd218e960 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -502,7 +502,7 @@ config SECTION_MISMATCH_WARN_ONLY
>
> config DEBUG_FORCE_FUNCTION_ALIGN_64B
> bool "Force all function address 64B aligned"
> - depends on EXPERT && (X86_64 || ARM64 || PPC32 || PPC64 || ARC || S390)
> + depends on EXPERT && (X86_64 || ARM64 || PPC32 || PPC64 || ARC || RISCV || S390)
> select FUNCTION_ALIGNMENT_64B
> help
> There are cases that a commit from one domain changes the function
> --
> 2.40.1
>


Attachments:
(No filename) (2.16 kB)
signature.asc (235.00 B)
Download all attachments

2023-07-30 15:17:48

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH] riscv: enable DEBUG_FORCE_FUNCTION_ALIGN_64B

On Sat, Jul 29, 2023 at 11:34:38AM +0100, Conor Dooley wrote:
> Hey,
>
> On Fri, Jul 28, 2023 at 12:03:56AM +0800, Jisheng Zhang wrote:
> > Allow to force all function address 64B aligned as it is possible for
> > other architectures. This may be useful when verify if performance
> > bump is caused by function alignment changes.
> >
> > Before commit 1bf18da62106 ("lib/Kconfig.debug: add ARCH dependency
> > for FUNCTION_ALIGN option"), riscv supports enabling the
> > DEBUG_FORCE_FUNCTION_ALIGN_64B option, but after that commit, each
> > arch needs to claim the support explicitly. I tried the config file in
> > [1] for both RV64 and RV32, I can't reproduce the build error as [1],
> > there is no reason for not allowing to enforce this function alignment.
> >
> > Link: https://lore.kernel.org/lkml/[email protected]/ [1]
>
> This is a CSKY randconfig, is there any particular reason that running
> that randconfig (over a year later) and on a different architecture
> would trigger whatever the condition was?

Just use the randconfig and then s/CSKY/RISCV to check whether RV32
and RV64 can reproduce the compile error ;)

>
> The original commit here seems far too penal - why was it not just
> disabled on CSKY??? I tried looking a bit on lore, but didn't see
> anything explaining the subset of supported archs they picked.
> I did see Guo Ren wondering if rv32 would be similarly problematic - but
> since this is something likely to just trip up randconfigs, I think we
> should go for it and if rv32 becomes a problem, restrict this to 64-bit.
>
> Reviewed-by: Conor Dooley <[email protected]>
>
> Thanks,
> Conor.
>
> > Signed-off-by: Jisheng Zhang <[email protected]>
> > ---
> > lib/Kconfig.debug | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index fbc89baf7de6..39ffd218e960 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -502,7 +502,7 @@ config SECTION_MISMATCH_WARN_ONLY
> >
> > config DEBUG_FORCE_FUNCTION_ALIGN_64B
> > bool "Force all function address 64B aligned"
> > - depends on EXPERT && (X86_64 || ARM64 || PPC32 || PPC64 || ARC || S390)
> > + depends on EXPERT && (X86_64 || ARM64 || PPC32 || PPC64 || ARC || RISCV || S390)
> > select FUNCTION_ALIGNMENT_64B
> > help
> > There are cases that a commit from one domain changes the function
> > --
> > 2.40.1
> >



Subject: Re: [PATCH] riscv: enable DEBUG_FORCE_FUNCTION_ALIGN_64B

Hello:

This patch was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <[email protected]>:

On Fri, 28 Jul 2023 00:03:56 +0800 you wrote:
> Allow to force all function address 64B aligned as it is possible for
> other architectures. This may be useful when verify if performance
> bump is caused by function alignment changes.
>
> Before commit 1bf18da62106 ("lib/Kconfig.debug: add ARCH dependency
> for FUNCTION_ALIGN option"), riscv supports enabling the
> DEBUG_FORCE_FUNCTION_ALIGN_64B option, but after that commit, each
> arch needs to claim the support explicitly. I tried the config file in
> [1] for both RV64 and RV32, I can't reproduce the build error as [1],
> there is no reason for not allowing to enforce this function alignment.
>
> [...]

Here is the summary with links:
- riscv: enable DEBUG_FORCE_FUNCTION_ALIGN_64B
https://git.kernel.org/riscv/c/74f438e75483

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html