Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1176619 > unrolled thread
| Started by | Denys Vlasenko <vda.linux@googlemail.com> |
|---|---|
| First post | 2015-07-03 16:40 +0200 |
| Last post | 2015-07-05 10:50 +0200 |
| Articles | 3 — 3 participants |
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.
Re: [PATCH v4 14/17] x86/asm/entry/64: Migrate error and interrupt exit work to C Denys Vlasenko <vda.linux@googlemail.com> - 2015-07-03 16:40 +0200
Re: [PATCH v4 14/17] x86/asm/entry/64: Migrate error and interrupt exit work to C Andy Lutomirski <luto@amacapital.net> - 2015-07-03 18:30 +0200
Re: [PATCH v4 14/17] x86/asm/entry/64: Migrate error and interrupt exit work to C Ingo Molnar <mingo@kernel.org> - 2015-07-05 10:50 +0200
| From | Denys Vlasenko <vda.linux@googlemail.com> |
|---|---|
| Date | 2015-07-03 16:40 +0200 |
| Subject | Re: [PATCH v4 14/17] x86/asm/entry/64: Migrate error and interrupt exit work to C |
| Message-ID | <pIaie-1dJ-25@gated-at.bofh.it> |
On Thu, Jul 2, 2015 at 6:09 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Jul 2, 2015 at 5:09 AM, Borislav Petkov <bp@alien8.de> wrote:
>> On Mon, Jun 29, 2015 at 12:33:46PM -0700, Andy Lutomirski wrote:
>>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>>> ---
>>> arch/x86/entry/entry_64.S | 63 +++++++++++-----------------------------
>>> arch/x86/entry/entry_64_compat.S | 5 ++++
>>> 2 files changed, 22 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
>>> index ce2c9049abef..08a37ec049f0 100644
>>> --- a/arch/x86/entry/entry_64.S
>>> +++ b/arch/x86/entry/entry_64.S
>>> @@ -508,7 +508,16 @@ END(irq_entries_start)
>>>
>>> testb $3, CS(%rsp)
>>> jz 1f
>>> +
>>> + /*
>>> + * IRQ from user mode. Switch to kernel gsbase and inform context
>>> + * tracking that we're in kernel mode.
>>> + */
>>> SWAPGS
>>> +#ifdef CONFIG_CONTEXT_TRACKING
>>> + call enter_from_user_mode
>>> +#endif
>>
>> I think you can make this much cleaner by getting rid of the ifdeffery
>> and pushing it into the enter_from_user_mode() function:
>>
>> __visible void enter_from_user_mode(void)
>> {
>> #ifdef CONFIG_CONTEXT_TRACKING
>>
>> ...
>>
>> #endif
>> }
>>
>> The disadvantage of all that cleanliness is that we get one dumb
>>
>> call enter_from_user_mode
>>
>> there to this abomination:
>>
>> ffffffff810014f0 <enter_from_user_mode>:
>> ffffffff810014f0: e8 db 97 67 00 callq ffffffff8167acd0 <__fentry__>
>> ffffffff810014f5: 55 push %rbp
>> ffffffff810014f6: 48 89 e5 mov %rsp,%rbp
>> ffffffff810014f9: 5d pop %rbp
>> ffffffff810014fa: c3 retq
>> ffffffff810014fb: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
>>
>> which sux.
>>
>> We sure could use LTO here.
>>
>
> You mean link-time asm optimizations? Turning off frame pointers in
> leaf functions as long as rbp is still preserved might not be so
> terrible either.
>
> I'm torn on this one. In principle, you're right, or we could have a
> macro CALL_ENTER_FROM_USER_MODE that does nothing if context tracking
> is off. OTOH, that's also kind of messy.
busybox has IF() macros:
IF_CONTEXT_TRACKING(call enter_from_user_mode)
Unlike #if, this needs one line, not three.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2015-07-03 18:30 +0200 |
| Message-ID | <pIc0H-2mW-59@gated-at.bofh.it> |
| In reply to | #1176619 |
On Fri, Jul 3, 2015 at 7:37 AM, Denys Vlasenko <vda.linux@googlemail.com> wrote:
> On Thu, Jul 2, 2015 at 6:09 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Thu, Jul 2, 2015 at 5:09 AM, Borislav Petkov <bp@alien8.de> wrote:
>>> On Mon, Jun 29, 2015 at 12:33:46PM -0700, Andy Lutomirski wrote:
>>>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>>>> ---
>>>> arch/x86/entry/entry_64.S | 63 +++++++++++-----------------------------
>>>> arch/x86/entry/entry_64_compat.S | 5 ++++
>>>> 2 files changed, 22 insertions(+), 46 deletions(-)
>>>>
>>>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
>>>> index ce2c9049abef..08a37ec049f0 100644
>>>> --- a/arch/x86/entry/entry_64.S
>>>> +++ b/arch/x86/entry/entry_64.S
>>>> @@ -508,7 +508,16 @@ END(irq_entries_start)
>>>>
>>>> testb $3, CS(%rsp)
>>>> jz 1f
>>>> +
>>>> + /*
>>>> + * IRQ from user mode. Switch to kernel gsbase and inform context
>>>> + * tracking that we're in kernel mode.
>>>> + */
>>>> SWAPGS
>>>> +#ifdef CONFIG_CONTEXT_TRACKING
>>>> + call enter_from_user_mode
>>>> +#endif
>>>
>>> I think you can make this much cleaner by getting rid of the ifdeffery
>>> and pushing it into the enter_from_user_mode() function:
>>>
>>> __visible void enter_from_user_mode(void)
>>> {
>>> #ifdef CONFIG_CONTEXT_TRACKING
>>>
>>> ...
>>>
>>> #endif
>>> }
>>>
>>> The disadvantage of all that cleanliness is that we get one dumb
>>>
>>> call enter_from_user_mode
>>>
>>> there to this abomination:
>>>
>>> ffffffff810014f0 <enter_from_user_mode>:
>>> ffffffff810014f0: e8 db 97 67 00 callq ffffffff8167acd0 <__fentry__>
>>> ffffffff810014f5: 55 push %rbp
>>> ffffffff810014f6: 48 89 e5 mov %rsp,%rbp
>>> ffffffff810014f9: 5d pop %rbp
>>> ffffffff810014fa: c3 retq
>>> ffffffff810014fb: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
>>>
>>> which sux.
>>>
>>> We sure could use LTO here.
>>>
>>
>> You mean link-time asm optimizations? Turning off frame pointers in
>> leaf functions as long as rbp is still preserved might not be so
>> terrible either.
>>
>> I'm torn on this one. In principle, you're right, or we could have a
>> macro CALL_ENTER_FROM_USER_MODE that does nothing if context tracking
>> is off. OTOH, that's also kind of messy.
>
> busybox has IF() macros:
>
> IF_CONTEXT_TRACKING(call enter_from_user_mode)
>
> Unlike #if, this needs one line, not three.
I predict all of these call sites will get moved to C before 4.3. If
not, then I'd be glad to clean this part up.
--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2015-07-05 10:50 +0200 |
| Message-ID | <pINMC-Ad-31@gated-at.bofh.it> |
| In reply to | #1176713 |
* Andy Lutomirski <luto@amacapital.net> wrote: > >> I'm torn on this one. In principle, you're right, or we could have a macro > >> CALL_ENTER_FROM_USER_MODE that does nothing if context tracking is off. > >> OTOH, that's also kind of messy. > > > > busybox has IF() macros: > > > > IF_CONTEXT_TRACKING(call enter_from_user_mode) > > > > Unlike #if, this needs one line, not three. > > I predict all of these call sites will get moved to C before 4.3. If not, then > I'd be glad to clean this part up. Ok, let's see how it works out. I think in the initial phase we should be conservative and should attempt to introduce as few unrelated changes as possible, and get the conversion done. The current x86 entry code is a reasonable base to start with. Once most of the 'mechanic' conversion is done we can do all the other changes. The conversion itself is risky enough as-is, we want to offload as much of any other risk to after the conversion is done. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web