Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1413642 > unrolled thread

Re: [PATCH 2/2] x86/entry: Inline enter_from_user_mode

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] x86/entry: Inline enter_from_user_mode Andy Lutomirski <luto@amacapital.net> - 2016-06-04 07:10 +0200
    Re: [PATCH 2/2] x86/entry: Inline enter_from_user_mode Paolo Bonzini <pbonzini@redhat.com> - 2016-06-06 18:10 +0200
      Re: [PATCH 2/2] x86/entry: Inline enter_from_user_mode Andy Lutomirski <luto@amacapital.net> - 2016-06-09 19:20 +0200

#1413642 — Re: [PATCH 2/2] x86/entry: Inline enter_from_user_mode

On May 30, 2016 5:30 AM, "Paolo Bonzini" <pbonzini@redhat.com> wrote:
>
> This matches what is already done for prepare_exit_to_usermode,
> and saves about 60 clock cycles (4% speedup) with the benchmark
> in the previous commit message.
>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Ingo Molnar <mingo@kernel.org.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/entry/common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index 946bc1a..582bbc8 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -40,7 +40,7 @@ static struct thread_info *pt_regs_to_thread_info(struct pt_regs *regs)
>
>  #ifdef CONFIG_CONTEXT_TRACKING
>  /* Called on entry from user mode with IRQs off. */
> -__visible void enter_from_user_mode(void)
> +__visible inline void enter_from_user_mode(void)
>  {
>         CT_WARN_ON(ct_state() != CONTEXT_USER);
>         __user_exit();

I wonder if an extern inline *declaration* is needed as well in this C
file.  At least C99 suggests it is.  Maybe __visible is sufficient to
force an external definition to be emitted.

[toc] | [next] | [standalone]

#1415256

On 04/06/2016 07:08, Andy Lutomirski wrote:
> On May 30, 2016 5:30 AM, "Paolo Bonzini" <pbonzini@redhat.com> wrote:
>>
>> This matches what is already done for prepare_exit_to_usermode,
>> and saves about 60 clock cycles (4% speedup) with the benchmark
>> in the previous commit message.
>>
>> Cc: Andy Lutomirski <luto@kernel.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Rik van Riel <riel@redhat.com>
>> Cc: H. Peter Anvin <hpa@zytor.com>
>> Cc: Ingo Molnar <mingo@kernel.org.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  arch/x86/entry/common.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
>> index 946bc1a..582bbc8 100644
>> --- a/arch/x86/entry/common.c
>> +++ b/arch/x86/entry/common.c
>> @@ -40,7 +40,7 @@ static struct thread_info *pt_regs_to_thread_info(struct pt_regs *regs)
>>
>>  #ifdef CONFIG_CONTEXT_TRACKING
>>  /* Called on entry from user mode with IRQs off. */
>> -__visible void enter_from_user_mode(void)
>> +__visible inline void enter_from_user_mode(void)
>>  {
>>         CT_WARN_ON(ct_state() != CONTEXT_USER);
>>         __user_exit();
> 
> I wonder if an extern inline *declaration* is needed as well in this C
> file.  At least C99 suggests it is.  Maybe __visible is sufficient to
> force an external definition to be emitted.

An extern inline declaration is not needed because the kernel uses
-std=gnu89 (or, if you prefer, because prepare_exit_to_usermode didn't
have one :)).

It's awesomely perverted:

  __attribute__((externally_visible)) inline void f(void) {}

  inline void g(void) {}

  extern inline void h(void);
  extern inline void h(void) {}

  inline void i(void);
  inline void i(void) {}

  extern inline void j(void);
  inline void j(void) {}

This patch (and the preexisting prepare_exit_to_usermode code) are
equivalent to "f".

Compile the above file with "--std=gnu89" or "--std=gnu99
-fgnu89-inline" and f/g/i/j are emitted.

Compile it with "--std=gnu99 -fno-gnu89-inline" and h/j is emitted.

Yes, the standard is _almost exactly_ the opposite of the preexisting
GCC implementation.  The only case which achieves the same effect is
when declarations are "extern inline" and definitions must always be
"inline".  Or of course just use "static inline".  At least it's
decently documented in the GCC info documentation.

Thanks,

Paolo

[toc] | [prev] | [next] | [standalone]

#1418504

On Mon, Jun 6, 2016 at 9:01 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 04/06/2016 07:08, Andy Lutomirski wrote:
>> On May 30, 2016 5:30 AM, "Paolo Bonzini" <pbonzini@redhat.com> wrote:
>>>
>>> This matches what is already done for prepare_exit_to_usermode,
>>> and saves about 60 clock cycles (4% speedup) with the benchmark
>>> in the previous commit message.
>>>
>>> Cc: Andy Lutomirski <luto@kernel.org>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Rik van Riel <riel@redhat.com>
>>> Cc: H. Peter Anvin <hpa@zytor.com>
>>> Cc: Ingo Molnar <mingo@kernel.org.com>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  arch/x86/entry/common.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
>>> index 946bc1a..582bbc8 100644
>>> --- a/arch/x86/entry/common.c
>>> +++ b/arch/x86/entry/common.c
>>> @@ -40,7 +40,7 @@ static struct thread_info *pt_regs_to_thread_info(struct pt_regs *regs)
>>>
>>>  #ifdef CONFIG_CONTEXT_TRACKING
>>>  /* Called on entry from user mode with IRQs off. */
>>> -__visible void enter_from_user_mode(void)
>>> +__visible inline void enter_from_user_mode(void)
>>>  {
>>>         CT_WARN_ON(ct_state() != CONTEXT_USER);
>>>         __user_exit();
>>
>> I wonder if an extern inline *declaration* is needed as well in this C
>> file.  At least C99 suggests it is.  Maybe __visible is sufficient to
>> force an external definition to be emitted.
>
> An extern inline declaration is not needed because the kernel uses
> -std=gnu89 (or, if you prefer, because prepare_exit_to_usermode didn't
> have one :)).
>
> It's awesomely perverted:
>
>   __attribute__((externally_visible)) inline void f(void) {}
>
>   inline void g(void) {}
>
>   extern inline void h(void);
>   extern inline void h(void) {}
>
>   inline void i(void);
>   inline void i(void) {}
>
>   extern inline void j(void);
>   inline void j(void) {}
>
> This patch (and the preexisting prepare_exit_to_usermode code) are
> equivalent to "f".
>
> Compile the above file with "--std=gnu89" or "--std=gnu99
> -fgnu89-inline" and f/g/i/j are emitted.
>
> Compile it with "--std=gnu99 -fno-gnu89-inline" and h/j is emitted.
>
> Yes, the standard is _almost exactly_ the opposite of the preexisting
> GCC implementation.  The only case which achieves the same effect is
> when declarations are "extern inline" and definitions must always be
> "inline".  Or of course just use "static inline".  At least it's
> decently documented in the GCC info documentation.

OK.

In any event, if this ever messes up, it'll fail to build and we'll notice.

--Andy

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web