Messages in this thread | | | From | Andrew Bresticker <> | Date | Mon, 14 Nov 2022 09:31:08 -0500 | Subject | Re: [PATCH] RISC-V: Fix unannoted hardirqs-on in return to userspace slow-path |
| |
On Sat, Nov 12, 2022 at 6:52 PM Conor Dooley <conor@kernel.org> wrote: > > On Fri, Nov 11, 2022 at 05:31:08PM -0500, Andrew Bresticker wrote: > > The return to userspace path in entry.S may enable interrupts without the > > corresponding lockdep annotation, producing a splat[0] when DEBUG_LOCKDEP > > is enabled. Simply calling __trace_hardirqs_on() here gets a bit messy > > due to the use of RA to point back to ret_from_exception, so just move > > the whole slow-path loop into C. It's more readable and it lets us use > > local_irq_{enable,disable}(), avoiding the need for manual annotations > > altogether. > > > > [0]: > > ------------[ cut here ]------------ > > DEBUG_LOCKS_WARN_ON(!lockdep_hardirqs_enabled()) > > WARNING: CPU: 2 PID: 1 at kernel/locking/lockdep.c:5512 check_flags+0x10a/0x1e0 > > Modules linked in: > > CPU: 2 PID: 1 Comm: init Not tainted 6.1.0-rc4-00160-gb56b6e2b4f31 #53 > > Hardware name: riscv-virtio,qemu (DT) > > epc : check_flags+0x10a/0x1e0 > > ra : check_flags+0x10a/0x1e0 > > <snip> > > status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003 > > [<ffffffff808edb90>] lock_is_held_type+0x78/0x14e > > [<ffffffff8003dae2>] __might_resched+0x26/0x22c > > [<ffffffff8003dd24>] __might_sleep+0x3c/0x66 > > [<ffffffff80022c60>] get_signal+0x9e/0xa70 > > [<ffffffff800054a2>] do_notify_resume+0x6e/0x422 > > [<ffffffff80003c68>] ret_from_exception+0x0/0x10 > > irq event stamp: 44512 > > hardirqs last enabled at (44511): [<ffffffff808f901c>] _raw_spin_unlock_irqrestore+0x54/0x62 > > hardirqs last disabled at (44512): [<ffffffff80008200>] __trace_hardirqs_off+0xc/0x14 > > softirqs last enabled at (44472): [<ffffffff808f9fbe>] __do_softirq+0x3de/0x51e > > softirqs last disabled at (44467): [<ffffffff80017760>] irq_exit+0xd6/0x104 > > ---[ end trace 0000000000000000 ]--- > > possible reason: unannotated irqs-on. > > > > Signed-off-by: Andrew Bresticker <abrestic@rivosinc.com> > > --- > > This should also theoretically be fixed by the conversion to generic entry, > > but it's not clear how far away that series is from landing. > > Heh.. In case this makes the cut instead - what commit does this fix?
Ah, right --
Fixes: 3c4697982982 ("riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT")
-Andrew
> > > --- > > arch/riscv/kernel/entry.S | 18 +++++------------- > > arch/riscv/kernel/signal.c | 34 +++++++++++++++++++++------------- > > 2 files changed, 26 insertions(+), 26 deletions(-) > > > > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S > > index b9eda3fcbd6d..58dfa8595e19 100644 > > --- a/arch/riscv/kernel/entry.S > > +++ b/arch/riscv/kernel/entry.S > > @@ -263,12 +263,11 @@ ret_from_exception: > > #endif > > bnez s0, resume_kernel > > > > -resume_userspace: > > /* Interrupts must be disabled here so flags are checked atomically */ > > REG_L s0, TASK_TI_FLAGS(tp) /* current_thread_info->flags */ > > andi s1, s0, _TIF_WORK_MASK > > - bnez s1, work_pending > > - > > + bnez s1, resume_userspace_slow > > +resume_userspace: > > #ifdef CONFIG_CONTEXT_TRACKING_USER > > call user_enter_callable > > #endif > > @@ -368,19 +367,12 @@ resume_kernel: > > j restore_all > > #endif > > > > -work_pending: > > +resume_userspace_slow: > > /* Enter slow path for supplementary processing */ > > - la ra, ret_from_exception > > - andi s1, s0, _TIF_NEED_RESCHED > > - bnez s1, work_resched > > -work_notifysig: > > - /* Handle pending signals and notify-resume requests */ > > - csrs CSR_STATUS, SR_IE /* Enable interrupts for do_notify_resume() */ > > move a0, sp /* pt_regs */ > > move a1, s0 /* current_thread_info->flags */ > > - tail do_notify_resume > > -work_resched: > > - tail schedule > > + call do_work_pending > > + j resume_userspace > > > > /* Slow paths for ptrace. */ > > handle_syscall_trace_enter: > > diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c > > index 5c591123c440..bfb2afa4135f 100644 > > --- a/arch/riscv/kernel/signal.c > > +++ b/arch/riscv/kernel/signal.c > > @@ -313,19 +313,27 @@ static void do_signal(struct pt_regs *regs) > > } > > > > /* > > - * notification of userspace execution resumption > > - * - triggered by the _TIF_WORK_MASK flags > > + * Handle any pending work on the resume-to-userspace path, as indicated by > > + * _TIF_WORK_MASK. Entered from assembly with IRQs off. > > */ > > -asmlinkage __visible void do_notify_resume(struct pt_regs *regs, > > - unsigned long thread_info_flags) > > +asmlinkage __visible void do_work_pending(struct pt_regs *regs, > > + unsigned long thread_info_flags) > > { > > - if (thread_info_flags & _TIF_UPROBE) > > - uprobe_notify_resume(regs); > > - > > - /* Handle pending signal delivery */ > > - if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) > > - do_signal(regs); > > - > > - if (thread_info_flags & _TIF_NOTIFY_RESUME) > > - resume_user_mode_work(regs); > > + do { > > + if (thread_info_flags & _TIF_NEED_RESCHED) { > > + schedule(); > > + } else { > > + local_irq_enable(); > > + if (thread_info_flags & _TIF_UPROBE) > > + uprobe_notify_resume(regs); > > + /* Handle pending signal delivery */ > > + if (thread_info_flags & (_TIF_SIGPENDING | > > + _TIF_NOTIFY_SIGNAL)) > > + do_signal(regs); > > + if (thread_info_flags & _TIF_NOTIFY_RESUME) > > + resume_user_mode_work(regs); > > + } > > + local_irq_disable(); > > + thread_info_flags = read_thread_flags(); > > + } while (thread_info_flags & _TIF_WORK_MASK); > > } > > -- > > 2.25.1 > >
| |