From: takahiro.akashi@linaro.org (AKASHI Takahiro)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3] arm64: Introduce IRQ stack
Date: Mon, 5 Oct 2015 15:37:11 +0900 [thread overview]
Message-ID: <56121A97.7000600@linaro.org> (raw)
In-Reply-To: <67E35231-C136-49B9-B0EF-CD0A2B21752A@gmail.com>
On 10/04/2015 11:32 PM, Jungseok Lee wrote:
> On Oct 3, 2015, at 1:23 AM, James Morse wrote:
>
>> Hi,
>
> Hi James,
>
>>
>> On 22/09/15 13:11, Jungseok Lee wrote:
>>> Currently, kernel context and interrupts are handled using a single
>>> kernel stack navigated by sp_el1. This forces a system to use 16KB
>>> stack, not 8KB one. This restriction makes low memory platforms suffer
>>> from memory pressure accompanied by performance degradation.
>>>
>>> This patch addresses the issue as introducing a separate percpu IRQ
>>> stack to handle both hard and soft interrupts with two ground rules:
>>>
>>> - Utilize sp_el0 in EL1 context, which is not used currently
>>> - Do not complicate current_thread_info calculation
>>>
>>> It is a core concept to directly retrieve struct thread_info from
>>> sp_el0. This approach helps to prevent text section size from being
>>> increased largely as removing masking operation using THREAD_SIZE
>>> in tons of places.
>>
>> One observed change in behaviour:
>> Any stack-unwinding now stops at el1_irq(), which is the bottom of the irq
>> stack. This shows up with perf (using incantation [0]), and with any calls
>> to dump_stack() (which actually stops the frame before el1_irq()).
>>
>> I don't know if this will break something, (perf still seems to work) - but
>> it makes the panic() output less useful, as all the 'other' cpus print:
>
> Agreed. A process stack should be walked to deliver useful information.
>
> There are two approaches I've tried as experimental.
>
> 1) Link IRQ stack to a process one via frame pointer
> As saving x29 and elr_el1 into IRQ stack and then updating x29, IRQ stack
> could be linked to a process one. It is similar to your patch except some
> points. However, it might complicate "stack tracer on ftrace" issue.
Well, as far as object_is_on_stack() works correctly, stack tracer will not
follow an interrupt stack even if unwind_frame() might traverse from
an interrupt stack to a process stack. See check_stack().
Under this assumption, I'm going to simplify my "stack tracer" bugfix
by removing interrupt-related nasty hacks that I described in RFC.
Thanks,
-Takahiro AKASHI
> 2) Walk a process stack followed by IRQ one
> This idea, which is straightforward, comes from x86 implementation [1]. The
> approach might be orthogonal to "stack tracer on ftrace" issue. In this case,
> unfortunately, a top bit comparison of stack pointer cannot be adopted due to
> a necessity of a final snapshot of a process stack pointer, which is struct
> irq_stack::thread_sp in v2 patch.
>
> Which one is your favorite? or any ideas?
>
> BTW, I have another question. Is it reasonable to introduce THREAD_SIZE as a
> kernel configuration option like page size for the sake of convenience because
> a combination of ARM64 and a small ram is not unusual in real practice? Needless
> to say, a patch, reducing the size, can be managed as out of mainline tree one.
>
> [1] arch/x86/kernel/dumpstack_64.c
>
> Best Regards
> Jungseok Lee
>
WARNING: multiple messages have this Message-ID (diff)
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: Jungseok Lee <jungseoklee85@gmail.com>,
James Morse <james.morse@arm.com>
Cc: Catalin Marinas <Catalin.Marinas@arm.com>,
Will Deacon <Will.Deacon@arm.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Mark Rutland <Mark.Rutland@arm.com>,
"barami97@gmail.com" <barami97@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3] arm64: Introduce IRQ stack
Date: Mon, 5 Oct 2015 15:37:11 +0900 [thread overview]
Message-ID: <56121A97.7000600@linaro.org> (raw)
In-Reply-To: <67E35231-C136-49B9-B0EF-CD0A2B21752A@gmail.com>
On 10/04/2015 11:32 PM, Jungseok Lee wrote:
> On Oct 3, 2015, at 1:23 AM, James Morse wrote:
>
>> Hi,
>
> Hi James,
>
>>
>> On 22/09/15 13:11, Jungseok Lee wrote:
>>> Currently, kernel context and interrupts are handled using a single
>>> kernel stack navigated by sp_el1. This forces a system to use 16KB
>>> stack, not 8KB one. This restriction makes low memory platforms suffer
>>> from memory pressure accompanied by performance degradation.
>>>
>>> This patch addresses the issue as introducing a separate percpu IRQ
>>> stack to handle both hard and soft interrupts with two ground rules:
>>>
>>> - Utilize sp_el0 in EL1 context, which is not used currently
>>> - Do not complicate current_thread_info calculation
>>>
>>> It is a core concept to directly retrieve struct thread_info from
>>> sp_el0. This approach helps to prevent text section size from being
>>> increased largely as removing masking operation using THREAD_SIZE
>>> in tons of places.
>>
>> One observed change in behaviour:
>> Any stack-unwinding now stops at el1_irq(), which is the bottom of the irq
>> stack. This shows up with perf (using incantation [0]), and with any calls
>> to dump_stack() (which actually stops the frame before el1_irq()).
>>
>> I don't know if this will break something, (perf still seems to work) - but
>> it makes the panic() output less useful, as all the 'other' cpus print:
>
> Agreed. A process stack should be walked to deliver useful information.
>
> There are two approaches I've tried as experimental.
>
> 1) Link IRQ stack to a process one via frame pointer
> As saving x29 and elr_el1 into IRQ stack and then updating x29, IRQ stack
> could be linked to a process one. It is similar to your patch except some
> points. However, it might complicate "stack tracer on ftrace" issue.
Well, as far as object_is_on_stack() works correctly, stack tracer will not
follow an interrupt stack even if unwind_frame() might traverse from
an interrupt stack to a process stack. See check_stack().
Under this assumption, I'm going to simplify my "stack tracer" bugfix
by removing interrupt-related nasty hacks that I described in RFC.
Thanks,
-Takahiro AKASHI
> 2) Walk a process stack followed by IRQ one
> This idea, which is straightforward, comes from x86 implementation [1]. The
> approach might be orthogonal to "stack tracer on ftrace" issue. In this case,
> unfortunately, a top bit comparison of stack pointer cannot be adopted due to
> a necessity of a final snapshot of a process stack pointer, which is struct
> irq_stack::thread_sp in v2 patch.
>
> Which one is your favorite? or any ideas?
>
> BTW, I have another question. Is it reasonable to introduce THREAD_SIZE as a
> kernel configuration option like page size for the sake of convenience because
> a combination of ARM64 and a small ram is not unusual in real practice? Needless
> to say, a patch, reducing the size, can be managed as out of mainline tree one.
>
> [1] arch/x86/kernel/dumpstack_64.c
>
> Best Regards
> Jungseok Lee
>
next prev parent reply other threads:[~2015-10-05 6:37 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-22 12:11 [PATCH v3] arm64: Introduce IRQ stack Jungseok Lee
2015-09-22 12:11 ` Jungseok Lee
2015-09-23 19:59 ` Jungseok Lee
2015-09-23 19:59 ` Jungseok Lee
2015-10-02 16:23 ` James Morse
2015-10-02 16:23 ` James Morse
2015-10-04 14:32 ` Jungseok Lee
2015-10-04 14:32 ` Jungseok Lee
2015-10-05 6:37 ` AKASHI Takahiro [this message]
2015-10-05 6:37 ` AKASHI Takahiro
2015-10-05 17:24 ` James Morse
2015-10-05 17:24 ` James Morse
2015-10-05 20:03 ` Jungseok Lee
2015-10-05 20:03 ` Jungseok Lee
-- strict thread matches above, loose matches on Subject: below --
2015-09-21 12:19 Jungseok Lee
2015-09-21 12:19 ` Jungseok Lee
2015-09-21 13:38 ` Jungseok Lee
2015-09-21 13:38 ` Jungseok Lee
[not found] ` <CA+tMzQE58f9OGt5ucJvOg1wFb-0KruUxmTKjT7Zd9cFEpMCDAg@mail.gmail.com>
2015-10-07 15:33 ` Jungseok Lee
2015-10-07 15:33 ` Jungseok Lee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56121A97.7000600@linaro.org \
--to=takahiro.akashi@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.