| Message ID | 1460944196-11780-1-git-send-email-shijie.huang@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 18, 2016 at 09:49:56AM +0800, Huang Shijie wrote: > We already re-enable interrupts where necessary in the entry code, so > there is no need to do it again in do_page fault. This patch removes > the redundant code. > > Signed-off-by: Huang Shijie <shijie.huang@arm.com> > --- > arch/arm64/mm/fault.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > index 95df28b..bdc193c 100644 > --- a/arch/arm64/mm/fault.c > +++ b/arch/arm64/mm/fault.c > @@ -212,10 +212,6 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr, > tsk = current; > mm = tsk->mm; > > - /* Enable interrupts if they were enabled in the parent context. */ > - if (interrupts_enabled(regs)) > - local_irq_enable(); We indeed don't have to re-enable interrupts here as they have been enabled by the calling code in entry.S. But have you run this with CONFIG_TRACE_IRQFLAGS enabled? I don't think there is any issue, just a sanity check.
On Mon, Apr 18, 2016 at 02:08:20PM +0100, Catalin Marinas wrote: > > @@ -212,10 +212,6 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr, > > tsk = current; > > mm = tsk->mm; > > > > - /* Enable interrupts if they were enabled in the parent context. */ > > - if (interrupts_enabled(regs)) > > - local_irq_enable(); > > We indeed don't have to re-enable interrupts here as they have been > enabled by the calling code in entry.S. But have you run this with > CONFIG_TRACE_IRQFLAGS enabled? I don't think there is any issue, just a > sanity check. I tested this patch with the CONFIG_TRACE_IRQFLAGS/CONFIG_PROVE_LOCKING/CONFIG_DEBUG_LOCKDEP enabled, in my Juno board, it works fine. Also I find that with this patch, if we want to check the lockdep stat with: #cat /proc/lockdep_stats The "redundant hardirq ons" become 0. Without this patch, the redundant hardirq ons" is a big number, such as 123444. thanks Huang Shijie IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Tue, Apr 19, 2016 at 03:16:29PM +0800, Huang Shijie wrote: > On Mon, Apr 18, 2016 at 02:08:20PM +0100, Catalin Marinas wrote: > > > @@ -212,10 +212,6 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr, > > > tsk = current; > > > mm = tsk->mm; > > > > > > - /* Enable interrupts if they were enabled in the parent context. */ > > > - if (interrupts_enabled(regs)) > > > - local_irq_enable(); > > > > We indeed don't have to re-enable interrupts here as they have been > > enabled by the calling code in entry.S. But have you run this with > > CONFIG_TRACE_IRQFLAGS enabled? I don't think there is any issue, just a > > sanity check. > I tested this patch with the CONFIG_TRACE_IRQFLAGS/CONFIG_PROVE_LOCKING/CONFIG_DEBUG_LOCKDEP > enabled, in my Juno board, it works fine. > > Also I find that with this patch, if we want to check the lockdep stat with: > #cat /proc/lockdep_stats > > The "redundant hardirq ons" become 0. Without this patch, the redundant > hardirq ons" is a big number, such as 123444. Thanks for checking. Acked-by: Catalin Marinas <catalin.marinas@arm.com> Will can pick this up for 4.7.
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index 95df28b..bdc193c 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -212,10 +212,6 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr, tsk = current; mm = tsk->mm; - /* Enable interrupts if they were enabled in the parent context. */ - if (interrupts_enabled(regs)) - local_irq_enable(); - /* * If we're in an interrupt or have no user context, we must not take * the fault.
We already re-enable interrupts where necessary in the entry code, so there is no need to do it again in do_page fault. This patch removes the redundant code. Signed-off-by: Huang Shijie <shijie.huang@arm.com> --- arch/arm64/mm/fault.c | 4 ---- 1 file changed, 4 deletions(-)