* [PATCH] MIPS: cpu-bugs64: Mark inline functions as __always_inline
@ 2019-09-27 5:33 Jiaxun Yang
2019-09-30 22:34 ` Paul Burton
2019-10-01 20:33 ` Paul Burton
0 siblings, 2 replies; 4+ messages in thread
From: Jiaxun Yang @ 2019-09-27 5:33 UTC (permalink / raw)
To: linux-mips; +Cc: Jiaxun Yang, Paul Burton
Commit ac7c3e4ff401 ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")
allows compiler to uninline functions marked as 'inline'. Leading to section
mismatch in this case.
Since we're using const variables to pass assembly flags, 'inline's can't
be dropped. So we simply mark them as __always_inline.
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: Paul Burton <paul.burton@mips.com>
Cc: linux-mips@vger.kernel.org
---
arch/mips/kernel/cpu-bugs64.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/mips/kernel/cpu-bugs64.c b/arch/mips/kernel/cpu-bugs64.c
index fa62cd1dff93..93d23232357c 100644
--- a/arch/mips/kernel/cpu-bugs64.c
+++ b/arch/mips/kernel/cpu-bugs64.c
@@ -24,7 +24,7 @@ static char r4kwar[] __initdata =
static char daddiwar[] __initdata =
"Enable CPU_DADDI_WORKAROUNDS to rectify.";
-static inline void align_mod(const int align, const int mod)
+static __always_inline void align_mod(const int align, const int mod)
{
asm volatile(
".set push\n\t"
@@ -113,7 +113,7 @@ static __always_inline void mult_sh_align_mod(long *v1, long *v2, long *w,
*w = lw;
}
-static inline void check_mult_sh(void)
+static __always_inline void check_mult_sh(void)
{
long v1[8], v2[8], w[8];
int bug, fix, i;
@@ -176,7 +176,7 @@ asmlinkage void __init do_daddi_ov(struct pt_regs *regs)
exception_exit(prev_state);
}
-static inline void check_daddi(void)
+static __always_inline void check_daddi(void)
{
extern asmlinkage void handle_daddi_ov(void);
unsigned long flags;
@@ -242,7 +242,7 @@ static inline void check_daddi(void)
int daddiu_bug = IS_ENABLED(CONFIG_CPU_MIPSR6) ? 0 : -1;
-static inline void check_daddiu(void)
+static __always_inline void check_daddiu(void)
{
long v, w, tmp;
--
2.23.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS: cpu-bugs64: Mark inline functions as __always_inline
2019-09-27 5:33 [PATCH] MIPS: cpu-bugs64: Mark inline functions as __always_inline Jiaxun Yang
@ 2019-09-30 22:34 ` Paul Burton
2019-10-01 12:00 ` Jiaxun Yang
2019-10-01 20:33 ` Paul Burton
1 sibling, 1 reply; 4+ messages in thread
From: Paul Burton @ 2019-09-30 22:34 UTC (permalink / raw)
To: Jiaxun Yang; +Cc: linux-mips
Hi Jiaxun,
On Fri, Sep 27, 2019 at 01:33:39PM +0800, Jiaxun Yang wrote:
> Commit ac7c3e4ff401 ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")
> allows compiler to uninline functions marked as 'inline'. Leading to section
> mismatch in this case.
>
> Since we're using const variables to pass assembly flags, 'inline's can't
> be dropped. So we simply mark them as __always_inline.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: linux-mips@vger.kernel.org
> ---
> arch/mips/kernel/cpu-bugs64.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/mips/kernel/cpu-bugs64.c b/arch/mips/kernel/cpu-bugs64.c
> index fa62cd1dff93..93d23232357c 100644
> --- a/arch/mips/kernel/cpu-bugs64.c
> +++ b/arch/mips/kernel/cpu-bugs64.c
> @@ -24,7 +24,7 @@ static char r4kwar[] __initdata =
> static char daddiwar[] __initdata =
> "Enable CPU_DADDI_WORKAROUNDS to rectify.";
>
> -static inline void align_mod(const int align, const int mod)
> +static __always_inline void align_mod(const int align, const int mod)
> {
> asm volatile(
> ".set push\n\t"
> @@ -113,7 +113,7 @@ static __always_inline void mult_sh_align_mod(long *v1, long *v2, long *w,
> *w = lw;
> }
>
> -static inline void check_mult_sh(void)
> +static __always_inline void check_mult_sh(void)
> {
> long v1[8], v2[8], w[8];
> int bug, fix, i;
So far so good, I see align_mod() uses arguments in inline asm &
check_mult_sh() makes use of it (via mult_sh_align_mod() which is
already __always_inline).
I'd prefer that we add the __init annotation too though, even if all
it's doing is making it clear to human readers when this code can be
used.
> @@ -176,7 +176,7 @@ asmlinkage void __init do_daddi_ov(struct pt_regs *regs)
> exception_exit(prev_state);
> }
>
> -static inline void check_daddi(void)
> +static __always_inline void check_daddi(void)
> {
> extern asmlinkage void handle_daddi_ov(void);
> unsigned long flags;
> @@ -242,7 +242,7 @@ static inline void check_daddi(void)
>
> int daddiu_bug = IS_ENABLED(CONFIG_CPU_MIPSR6) ? 0 : -1;
>
> -static inline void check_daddiu(void)
> +static __always_inline void check_daddiu(void)
> {
> long v, w, tmp;
>
I'm not seeing a reason for always inlining check_daddi() or
check_daddiu() though - neither one appears to use arguments as inline
asm immediates, so I'd prefer that we just mark them with __init & let
the compiler decide whether to inline.
Does that sound good to you?
Thanks,
Paul
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS: cpu-bugs64: Mark inline functions as __always_inline
2019-09-30 22:34 ` Paul Burton
@ 2019-10-01 12:00 ` Jiaxun Yang
0 siblings, 0 replies; 4+ messages in thread
From: Jiaxun Yang @ 2019-10-01 12:00 UTC (permalink / raw)
To: Paul Burton; +Cc: linux-mips
On 2019/10/1 上午6:34, Paul Burton wrote:
> Hi Jiaxun,
>
> On Fri, Sep 27, 2019 at 01:33:39PM +0800, Jiaxun Yang wrote:
>> Commit ac7c3e4ff401 ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")
>> allows compiler to uninline functions marked as 'inline'. Leading to section
>> mismatch in this case.
>>
>> Since we're using const variables to pass assembly flags, 'inline's can't
>> be dropped. So we simply mark them as __always_inline.
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> Cc: Paul Burton <paul.burton@mips.com>
>> Cc: linux-mips@vger.kernel.org
>> ---
>> arch/mips/kernel/cpu-bugs64.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/mips/kernel/cpu-bugs64.c b/arch/mips/kernel/cpu-bugs64.c
>> index fa62cd1dff93..93d23232357c 100644
>> --- a/arch/mips/kernel/cpu-bugs64.c
>> +++ b/arch/mips/kernel/cpu-bugs64.c
>> @@ -24,7 +24,7 @@ static char r4kwar[] __initdata =
>> static char daddiwar[] __initdata =
>> "Enable CPU_DADDI_WORKAROUNDS to rectify.";
>>
>> -static inline void align_mod(const int align, const int mod)
>> +static __always_inline void align_mod(const int align, const int mod)
>> {
>> asm volatile(
>> ".set push\n\t"
>> @@ -113,7 +113,7 @@ static __always_inline void mult_sh_align_mod(long *v1, long *v2, long *w,
>> *w = lw;
>> }
>>
>> -static inline void check_mult_sh(void)
>> +static __always_inline void check_mult_sh(void)
>> {
>> long v1[8], v2[8], w[8];
>> int bug, fix, i;
>
> So far so good, I see align_mod() uses arguments in inline asm &
> check_mult_sh() makes use of it (via mult_sh_align_mod() which is
> already __always_inline).
>
> I'd prefer that we add the __init annotation too though, even if all
> it's doing is making it clear to human readers when this code can be
> used.
Hi Paul,
You're right.
>
>> @@ -176,7 +176,7 @@ asmlinkage void __init do_daddi_ov(struct pt_regs *regs)
>> exception_exit(prev_state);
>> }
>>
>> -static inline void check_daddi(void)
>> +static __always_inline void check_daddi(void)
>> {
>> extern asmlinkage void handle_daddi_ov(void);
>> unsigned long flags;
>> @@ -242,7 +242,7 @@ static inline void check_daddi(void)
>>
>> int daddiu_bug = IS_ENABLED(CONFIG_CPU_MIPSR6) ? 0 : -1;
>>
>> -static inline void check_daddiu(void)
>> +static __always_inline void check_daddiu(void)
>> {
>> long v, w, tmp;
>>
>
> I'm not seeing a reason for always inlining check_daddi() or
> check_daddiu() though - neither one appears to use arguments as inline
> asm immediates, so I'd prefer that we just mark them with __init & let
> the compiler decide whether to inline.
>
> Does that sound good to you?
LGTM. Should I send v2 or you just fix them at apply time?
Thanks.
--
Jiaxun
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS: cpu-bugs64: Mark inline functions as __always_inline
2019-09-27 5:33 [PATCH] MIPS: cpu-bugs64: Mark inline functions as __always_inline Jiaxun Yang
2019-09-30 22:34 ` Paul Burton
@ 2019-10-01 20:33 ` Paul Burton
1 sibling, 0 replies; 4+ messages in thread
From: Paul Burton @ 2019-10-01 20:33 UTC (permalink / raw)
To: Jiaxun Yang; +Cc: linux-mips, Jiaxun Yang, Paul Burton, linux-mips
Hello,
Jiaxun Yang wrote:
> Commit ac7c3e4ff401 ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly")
> allows compiler to uninline functions marked as 'inline'. Leading to section
> mismatch in this case.
>
> Since we're using const variables to pass assembly flags, 'inline's can't
> be dropped. So we simply mark them as __always_inline.
Applied to mips-fixes.
> commit d345d9cad225
> https://git.kernel.org/mips/c/d345d9cad225
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> [paul.burton@mips.com:
> - Annotate these functions with __init, even if it only serves to
> inform human readers when the code can be used.
> - Drop the __always_inline from check_daddi() & check_daddiu() which
> don't use arguments as immediates in inline asm.
> - Rewrap the commit message.]
> Signed-off-by: Paul Burton <paul.burton@mips.com>
Thanks,
Paul
[ This message was auto-generated; if you believe anything is incorrect
then please email paul.burton@mips.com to report it. ]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-01 20:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-27 5:33 [PATCH] MIPS: cpu-bugs64: Mark inline functions as __always_inline Jiaxun Yang
2019-09-30 22:34 ` Paul Burton
2019-10-01 12:00 ` Jiaxun Yang
2019-10-01 20:33 ` Paul Burton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).