All of lore.kernel.org
 help / color / mirror / Atom feed
From: james.morse@arm.com (James Morse)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] arm64: Introduce IRQ stack
Date: Fri, 18 Sep 2015 14:44:56 +0100	[thread overview]
Message-ID: <55FC1558.7030303@arm.com> (raw)
In-Reply-To: <6A6F7BF2-D75E-4D8D-B0F7-45294C4C4426@gmail.com>

On 18/09/15 13:57, Jungseok Lee wrote:
> On Sep 18, 2015, at 1:21 AM, Catalin Marinas wrote:
>> in more detail. BTW, I don't think we need the any count for the irq
>> stack as we don't re-enter the same IRQ stack.
> 
> Another interrupt could come in since IRQ is enabled when handling softirq
> according to the following information which are self-evident.
> 
> (Am I missing something?)
> 
> 1) kernel/softirq.c
> 
> asmlinkage __visible void __do_softirq(void)
> {
>         unsigned long end = jiffies + MAX_SOFTIRQ_TIME;
>         unsigned long old_flags = current->flags;
>         int max_restart = MAX_SOFTIRQ_RESTART;
>         struct softirq_action *h;
>         bool in_hardirq;
>         __u32 pending;
>         int softirq_bit;
> 
>         /*  
>          * Mask out PF_MEMALLOC s current task context is borrowed for the
>          * softirq. A softirq handled such as network RX might set PF_MEMALLOC
>          * again if the socket is related to swap
>          */
>         current->flags &= ~PF_MEMALLOC;
> 
>         pending = local_softirq_pending();
>         account_irq_enter_time(current);
> 
>         __local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET);
>         in_hardirq = lockdep_softirq_start();
> 
> restart:
>         /* Reset the pending bitmask before enabling irqs */
>         set_softirq_pending(0);
> 
>         local_irq_enable();

This call into __do_softirq() should be prevented by
kernel/softirq.c:irq_exit():
> preempt_count_sub(HARDIRQ_OFFSET);
> if (!in_interrupt() && local_softirq_pending())
>      invoke_softirq();

in_interrupt(), pulls preempt_count out of thread_info, and masks it with
(SOFTIRQ_MASK | HARDIRQ_MASK | NMI_MASK). This value is zero due to the
preempt_count_sub() immediately before.

preempt_count_add(HARDIRQ_OFFSET) is called in __irq_enter(), so its not
unreasonable that it decrements it here - but it is falling to zero,
causing softirqs to be handled during interrupt handling.


Despite the '!in_interrupt()', it looks like this is entirely intentional,
from invoke_softirq():
/*
 * We can safely execute softirq on the current stack if
 * it is the irq stack, because it should be near empty
 * at this stage.
 */


x86 has an additional preempt_count_{add,sub}(HARDIRQ_OFFSET) in
ist_{enter,exit}(), which would prevent this. They call this for
double_faults and debug-exceptions.

It looks like we need to 'preempt_count_add(HARDIRQ_OFFSET)' in el1_irq()
to prevent the fall-to-zero, to prevent recursive use of the irq stack -
alternatively I have a smaller set of asm for irq_stack_entry() which keeps
the status quo.



James

WARNING: multiple messages have this Message-ID (diff)
From: James Morse <james.morse@arm.com>
To: Jungseok Lee <jungseoklee85@gmail.com>,
	Catalin Marinas <Catalin.Marinas@arm.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"takahiro.akashi@linaro.org" <takahiro.akashi@linaro.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2] arm64: Introduce IRQ stack
Date: Fri, 18 Sep 2015 14:44:56 +0100	[thread overview]
Message-ID: <55FC1558.7030303@arm.com> (raw)
In-Reply-To: <6A6F7BF2-D75E-4D8D-B0F7-45294C4C4426@gmail.com>

On 18/09/15 13:57, Jungseok Lee wrote:
> On Sep 18, 2015, at 1:21 AM, Catalin Marinas wrote:
>> in more detail. BTW, I don't think we need the any count for the irq
>> stack as we don't re-enter the same IRQ stack.
> 
> Another interrupt could come in since IRQ is enabled when handling softirq
> according to the following information which are self-evident.
> 
> (Am I missing something?)
> 
> 1) kernel/softirq.c
> 
> asmlinkage __visible void __do_softirq(void)
> {
>         unsigned long end = jiffies + MAX_SOFTIRQ_TIME;
>         unsigned long old_flags = current->flags;
>         int max_restart = MAX_SOFTIRQ_RESTART;
>         struct softirq_action *h;
>         bool in_hardirq;
>         __u32 pending;
>         int softirq_bit;
> 
>         /*  
>          * Mask out PF_MEMALLOC s current task context is borrowed for the
>          * softirq. A softirq handled such as network RX might set PF_MEMALLOC
>          * again if the socket is related to swap
>          */
>         current->flags &= ~PF_MEMALLOC;
> 
>         pending = local_softirq_pending();
>         account_irq_enter_time(current);
> 
>         __local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET);
>         in_hardirq = lockdep_softirq_start();
> 
> restart:
>         /* Reset the pending bitmask before enabling irqs */
>         set_softirq_pending(0);
> 
>         local_irq_enable();

This call into __do_softirq() should be prevented by
kernel/softirq.c:irq_exit():
> preempt_count_sub(HARDIRQ_OFFSET);
> if (!in_interrupt() && local_softirq_pending())
>      invoke_softirq();

in_interrupt(), pulls preempt_count out of thread_info, and masks it with
(SOFTIRQ_MASK | HARDIRQ_MASK | NMI_MASK). This value is zero due to the
preempt_count_sub() immediately before.

preempt_count_add(HARDIRQ_OFFSET) is called in __irq_enter(), so its not
unreasonable that it decrements it here - but it is falling to zero,
causing softirqs to be handled during interrupt handling.


Despite the '!in_interrupt()', it looks like this is entirely intentional,
from invoke_softirq():
/*
 * We can safely execute softirq on the current stack if
 * it is the irq stack, because it should be near empty
 * at this stage.
 */


x86 has an additional preempt_count_{add,sub}(HARDIRQ_OFFSET) in
ist_{enter,exit}(), which would prevent this. They call this for
double_faults and debug-exceptions.

It looks like we need to 'preempt_count_add(HARDIRQ_OFFSET)' in el1_irq()
to prevent the fall-to-zero, to prevent recursive use of the irq stack -
alternatively I have a smaller set of asm for irq_stack_entry() which keeps
the status quo.



James




  reply	other threads:[~2015-09-18 13:44 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-13 14:42 [PATCH v2] arm64: Introduce IRQ stack Jungseok Lee
2015-09-13 14:42 ` Jungseok Lee
2015-09-16 11:25 ` Will Deacon
2015-09-16 11:25   ` Will Deacon
2015-09-17 10:33   ` James Morse
2015-09-17 10:33     ` James Morse
2015-09-17 10:48     ` Catalin Marinas
2015-09-17 10:48       ` Catalin Marinas
2015-09-17 12:36     ` Jungseok Lee
2015-09-17 12:36       ` Jungseok Lee
2015-09-17 17:07       ` Catalin Marinas
2015-09-17 17:07         ` Catalin Marinas
2015-09-18 13:02         ` Jungseok Lee
2015-09-18 13:02           ` Jungseok Lee
2015-09-17 11:17 ` Catalin Marinas
2015-09-17 11:17   ` Catalin Marinas
2015-09-17 13:17   ` Jungseok Lee
2015-09-17 13:17     ` Jungseok Lee
2015-09-17 13:22     ` Jungseok Lee
2015-09-17 13:22       ` Jungseok Lee
2015-09-17 16:21       ` Catalin Marinas
2015-09-17 16:21         ` Catalin Marinas
2015-09-18 12:57         ` Jungseok Lee
2015-09-18 12:57           ` Jungseok Lee
2015-09-18 13:44           ` James Morse [this message]
2015-09-18 13:44             ` James Morse
2015-09-18 15:03           ` Catalin Marinas
2015-09-18 15:03             ` Catalin Marinas
2015-09-18 15:31             ` Catalin Marinas
2015-09-18 15:31               ` Catalin Marinas
2015-09-19  8:44               ` Jungseok Lee
2015-09-19  8:44                 ` Jungseok Lee
2015-09-21  9:25                 ` Catalin Marinas
2015-09-21  9:25                   ` Catalin Marinas
2015-09-21 12:14                   ` Jungseok Lee
2015-09-21 12:14                     ` Jungseok Lee
2015-09-18 13:46 ` James Morse
2015-09-18 13:46   ` James Morse
2015-09-19  8:20   ` Jungseok Lee
2015-09-19  8:20     ` 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=55FC1558.7030303@arm.com \
    --to=james.morse@arm.com \
    --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.