Messages in this thread | | | From | Sumit Garg <> | Date | Mon, 7 Mar 2022 17:02:02 +0530 | Subject | Re: [PATCH v2 1/2] tty/sysrq: Make sysrq handler NMI aware |
| |
Hi Doug,
On Fri, 4 Mar 2022 at 23:36, Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Thu, Mar 3, 2022 at 10:45 PM Hitomi Hasegawa > <hasegawa-hitomi@fujitsu.com> wrote: > > > > void __handle_sysrq(int key, bool check_mask) > > { > > const struct sysrq_key_op *op_p; > > @@ -573,6 +606,10 @@ void __handle_sysrq(int key, bool check_mask) > > int orig_suppress_printk; > > int i; > > > > + /* Skip sysrq handling if one already in progress */ > > + if (sysrq_nmi_key != -1) > > + return; > > Should this give a warning? > > Also, can you remind me why this is safe if two CPUs both call > handle_sysrq() at the same time? Can't both of them make it past this? > That doesn't seem so great. > > > > @@ -596,7 +633,13 @@ void __handle_sysrq(int key, bool check_mask) > > if (!check_mask || sysrq_on_mask(op_p->enable_mask)) { > > pr_info("%s\n", op_p->action_msg); > > console_loglevel = orig_log_level; > > - op_p->handler(key); > > + > > + if (in_nmi() && !op_p->nmi_safe) { > > + sysrq_nmi_key = key; > > + irq_work_queue(&sysrq_irq_work); > > It looks like irq_work_queue() returns false if it fails to queue. > Maybe it's worth checking and setting "sysrq_nmi_key" back to -1 if it > fails?
Thanks for your comments. I hope v4 here [1] addresses all of them. Please have a look again.
[1] https://lkml.org/lkml/2022/3/7/1059
-Sumit
| |