Re: Perf: WARNING: arch/x86/entry/common.c:624 idtentry_exit_cond_rcu+0x92/0xc0

On Thu, Jul 2, 2020 at 8:03 AM Peter Zijlstra peterz@infradead.org wrote:

On Thu, Jul 02, 2020 at 07:57:54PM +0530, Naresh Kamboju wrote:

...

I have reported this warning on linux-next and now it is happening on linux mainline tree. May I know , are we missing a fix patch on linus 's tree ?

• Naresh

---

While running selftest x86 single_step_syscall_32 on i386 kernel linux 5.8.0-rc3 kernel warning noticed.

steps to reproduce:

perf test and cd /opt/kselftests/default-in-kernel/x86 ./single_step_syscall_32

crash dump, [ 1324.774385] kselftest: Running tests in x86 [ 1324.830187] ------------[ cut here ]------------ [ 1324.834820] IRQs not disabled as expected [ 1324.838838] WARNING: CPU: 2 PID: 5365 at /usr/src/kernel/arch/x86/entry/common.c:645 idtentry_exit_cond_rcu+0x92/0xc0

How's this?

DEFINE_IDTENTRY_DEBUG() is DEFINE_IDTENTRY_RAW on x86_64 DEFINE_IDTENTRY on i386

calling exc_debug_*() from DEFINE_IDTENTRY() does a double layer of idtentry_{enter,exit}*() functions.

Also, handle_debug() is still a trainwreck, we should never get to cond_local_irq_enable() when !user.

Completely untested...

---

arch/x86/kernel/traps.c | 49 +++++++++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index b0195771c932..47d6b46e1564 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -806,8 +806,17 @@ static void handle_debug(struct pt_regs *regs, unsigned long dr6, bool user) * If DR6 is zero, no point in trying to handle it. The kernel is * not using INT1. */

•   if (!user && !dr6)
  

•           return;
  

•   if (!user) {
  

•           /*
  

•            * Catch SYSENTER with TF set and clear DR_STEP. If this hit a
  

•            * watchpoint at the same time then that will still be handled.
  

•            */
  

•           if ((dr6 & DR_STEP) && is_sysenter_singlestep(regs))
  

•                   dr6 &= ~DR_STEP;
  

•           if (!dr6)
  

•                   return;
  

•   }
  
    /*
     * If dr6 has no reason to give us about the origin of this trap,
  

@@ -859,25 +868,29 @@ static void handle_debug(struct pt_regs *regs, unsigned long dr6, bool user) cond_local_irq_disable(regs); }

+#ifdef CONFIG_X86_64 static __always_inline void exc_debug_kernel(struct pt_regs *regs, unsigned long dr6) { bool irq_state = idtentry_enter_nmi(regs); instrumentation_begin();

•   /*
  

•    * Catch SYSENTER with TF set and clear DR_STEP. If this hit a
  

•    * watchpoint at the same time then that will still be handled.
  

•    */
  

•   if ((dr6 & DR_STEP) && is_sysenter_singlestep(regs))
  

•           dr6 &= ~DR_STEP;
  

•   handle_debug(regs, dr6, false);
  
    instrumentation_end();
    idtentry_exit_nmi(regs, irq_state);
  

}

+/* IST stack entry */ +DEFINE_IDTENTRY_DEBUG(exc_debug) +{

•   unsigned long dr6, dr7;
  

•   debug_enter(&dr6, &dr7);
  

•   exc_debug_kernel(regs, dr6);
  

•   debug_exit(dr7);
  

+}

static __always_inline void exc_debug_user(struct pt_regs *regs, unsigned long dr6) { @@ -890,17 +903,6 @@ static __always_inline void exc_debug_user(struct pt_regs *regs, idtentry_exit_user(regs); }

-#ifdef CONFIG_X86_64 -/* IST stack entry */ -DEFINE_IDTENTRY_DEBUG(exc_debug) -{

•   unsigned long dr6, dr7;
  

•   debug_enter(&dr6, &dr7);
  

•   exc_debug_kernel(regs, dr6);
  

•   debug_exit(dr7);
  

-}

/* User entry, runs on regular task stack */ DEFINE_IDTENTRY_DEBUG_USER(exc_debug) { @@ -917,12 +919,7 @@ DEFINE_IDTENTRY_DEBUG(exc_debug) unsigned long dr6, dr7;

    debug_enter(&dr6, &dr7);

•   if (user_mode(regs))
  

•           exc_debug_user(regs, dr6);
  

•   else
  

•           exc_debug_kernel(regs, dr6);
  

•   handle_debug(regs, dr6, user_mode(regs));
    debug_exit(dr7);
  

} #endif

I'll fold something like this into my pile of pending fixes.