| Message ID | 20190927053339.11866-1-jiaxun.yang@flygoat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | MIPS: cpu-bugs64: Mark inline functions as __always_inline | expand |
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
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
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. ]
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;
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(-)