RFC: x86/jump_label: Mark arguments as const to satisfy asm constraints
diff mbox series

Message ID 20210211214848.536626-1-jason.gerecke@wacom.com
State Accepted
Commit 864b435514b286c0be2a38a02f487aa28d990ef8
Headers show
Series
  • RFC: x86/jump_label: Mark arguments as const to satisfy asm constraints
Related show

Commit Message

Jason Gerecke Feb. 11, 2021, 9:48 p.m. UTC
When compiling an external kernel module with `-O0` or `-O1`, the following
compile error may be reported:

    ./arch/x86/include/asm/jump_label.h:25:2: error: impossible constraint in ‘asm’
       25 |  asm_volatile_goto("1:"
          |  ^~~~~~~~~~~~~~~~~

It appears that these lower optimization levels prevent GCC from detecting
that the key/branch arguments can be treated as constants and used as
immediate operands. To work around this, explicitly add the `const` label.

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
---
Marked RFC since I'm not familiar with this subsystem or the asm blocks that
are impacted. Extra-close inspection would be appreciated.

 arch/x86/include/asm/jump_label.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Steven Rostedt Feb. 12, 2021, 2:40 p.m. UTC | #1
On Thu, 11 Feb 2021 13:48:48 -0800
Jason Gerecke <killertofu@gmail.com> wrote:

> When compiling an external kernel module with `-O0` or `-O1`, the following
> compile error may be reported:
> 
>     ./arch/x86/include/asm/jump_label.h:25:2: error: impossible constraint in ‘asm’
>        25 |  asm_volatile_goto("1:"
>           |  ^~~~~~~~~~~~~~~~~
> 
> It appears that these lower optimization levels prevent GCC from detecting
> that the key/branch arguments can be treated as constants and used as
> immediate operands. To work around this, explicitly add the `const` label.

Yes this makes sense. The "i" constraint needs to be a constant.

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

> 
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> ---
> Marked RFC since I'm not familiar with this subsystem or the asm blocks that
> are impacted. Extra-close inspection would be appreciated.
> 
>  arch/x86/include/asm/jump_label.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
> index 06c3cc22a058..7f2006645d84 100644
> --- a/arch/x86/include/asm/jump_label.h
> +++ b/arch/x86/include/asm/jump_label.h
> @@ -20,7 +20,7 @@
>  #include <linux/stringify.h>
>  #include <linux/types.h>
>  
> -static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
> +static __always_inline bool arch_static_branch(struct static_key * const key, const bool branch)
>  {
>  	asm_volatile_goto("1:"
>  		".byte " __stringify(STATIC_KEY_INIT_NOP) "\n\t"
> @@ -36,7 +36,7 @@ static __always_inline bool arch_static_branch(struct static_key *key, bool bran
>  	return true;
>  }
>  
> -static __always_inline bool arch_static_branch_jump(struct static_key *key, bool branch)
> +static __always_inline bool arch_static_branch_jump(struct static_key * const key, const bool branch)
>  {
>  	asm_volatile_goto("1:"
>  		".byte 0xe9\n\t .long %l[l_yes] - 2f\n\t"
Peter Zijlstra Feb. 12, 2021, 3:27 p.m. UTC | #2
On Fri, Feb 12, 2021 at 09:40:59AM -0500, Steven Rostedt wrote:
> On Thu, 11 Feb 2021 13:48:48 -0800
> Jason Gerecke <killertofu@gmail.com> wrote:
> 
> > When compiling an external kernel module with `-O0` or `-O1`, the following
> > compile error may be reported:
> > 
> >     ./arch/x86/include/asm/jump_label.h:25:2: error: impossible constraint in ‘asm’
> >        25 |  asm_volatile_goto("1:"
> >           |  ^~~~~~~~~~~~~~~~~
> > 
> > It appears that these lower optimization levels prevent GCC from detecting
> > that the key/branch arguments can be treated as constants and used as
> > immediate operands. To work around this, explicitly add the `const` label.
> 
> Yes this makes sense. The "i" constraint needs to be a constant.

Right, using -O[01] seems a little daft though. But yeah, that patch is
correct and won't cause harm.

I've queued it for after the next merge window.
Jason Gerecke Feb. 12, 2021, 3:55 p.m. UTC | #3
On Fri, Feb 12, 2021 at 7:27 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Feb 12, 2021 at 09:40:59AM -0500, Steven Rostedt wrote:
> > On Thu, 11 Feb 2021 13:48:48 -0800
> > Jason Gerecke <killertofu@gmail.com> wrote:
> >
> > > When compiling an external kernel module with `-O0` or `-O1`, the following
> > > compile error may be reported:
> > >
> > >     ./arch/x86/include/asm/jump_label.h:25:2: error: impossible constraint in ‘asm’
> > >        25 |  asm_volatile_goto("1:"
> > >           |  ^~~~~~~~~~~~~~~~~
> > >
> > > It appears that these lower optimization levels prevent GCC from detecting
> > > that the key/branch arguments can be treated as constants and used as
> > > immediate operands. To work around this, explicitly add the `const` label.
> >
> > Yes this makes sense. The "i" constraint needs to be a constant.
>
> Right, using -O[01] seems a little daft though. But yeah, that patch is
> correct and won't cause harm.
>
> I've queued it for after the next merge window.

Thanks. Only reason I even tried compiling at those levels was to play
around with GCC's new static analyzer options. They seem to be
ineffective at more normal optimization levels.
Josh Poimboeuf Feb. 12, 2021, 4:14 p.m. UTC | #4
On Thu, Feb 11, 2021 at 01:48:48PM -0800, Jason Gerecke wrote:
> When compiling an external kernel module with `-O0` or `-O1`, the following
> compile error may be reported:
> 
>     ./arch/x86/include/asm/jump_label.h:25:2: error: impossible constraint in ‘asm’
>        25 |  asm_volatile_goto("1:"
>           |  ^~~~~~~~~~~~~~~~~
> 
> It appears that these lower optimization levels prevent GCC from detecting
> that the key/branch arguments can be treated as constants and used as
> immediate operands. To work around this, explicitly add the `const` label.
> 
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> ---
> Marked RFC since I'm not familiar with this subsystem or the asm blocks that
> are impacted. Extra-close inspection would be appreciated.

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

Patch
diff mbox series

diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 06c3cc22a058..7f2006645d84 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -20,7 +20,7 @@ 
 #include <linux/stringify.h>
 #include <linux/types.h>
 
-static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
+static __always_inline bool arch_static_branch(struct static_key * const key, const bool branch)
 {
 	asm_volatile_goto("1:"
 		".byte " __stringify(STATIC_KEY_INIT_NOP) "\n\t"
@@ -36,7 +36,7 @@  static __always_inline bool arch_static_branch(struct static_key *key, bool bran
 	return true;
 }
 
-static __always_inline bool arch_static_branch_jump(struct static_key *key, bool branch)
+static __always_inline bool arch_static_branch_jump(struct static_key * const key, const bool branch)
 {
 	asm_volatile_goto("1:"
 		".byte 0xe9\n\t .long %l[l_yes] - 2f\n\t"