On Mon, Feb 28, 2022 at 01:23:51PM +0530, Sumit Garg wrote:
> Allow a magic sysrq to be triggered from an NMI context. This is done

*why* though?


> +#define SYSRQ_NMI_FIFO_SIZE  2
> +static DEFINE_KFIFO(sysrq_nmi_fifo, int, SYSRQ_NMI_FIFO_SIZE);
> +
> +static void sysrq_do_nmi_work(struct irq_work *work)

That naming don't make sense, it does the !NMI work, from IRQ context.

> +{
> +     const struct sysrq_key_op *op_p;
> +     int orig_suppress_printk;
> +     int key;
> +
> +     orig_suppress_printk = suppress_printk;
> +     suppress_printk = 0;
> +
> +     rcu_sysrq_start();
> +     rcu_read_lock();
> +
> +     if (kfifo_peek(&sysrq_nmi_fifo, &key)) {
> +             op_p = __sysrq_get_key_op(key);
> +             if (op_p)
> +                     op_p->handler(key);
> +     }
> +
> +     rcu_read_unlock();
> +     rcu_sysrq_end();
> +
> +     suppress_printk = orig_suppress_printk;
> +
> +     kfifo_reset_out(&sysrq_nmi_fifo);
> +}
> +
> +static DEFINE_IRQ_WORK(sysrq_nmi_work, sysrq_do_nmi_work);
> +
>  void __handle_sysrq(int key, bool check_mask)
>  {
>       const struct sysrq_key_op *op_p;
> @@ -573,6 +612,10 @@ void __handle_sysrq(int key, bool check_mask)
>       int orig_suppress_printk;
>       int i;
>  
> +     /* Skip sysrq handling if one already in progress */
> +     if (!kfifo_is_empty(&sysrq_nmi_fifo))
> +             return;
> +
>       orig_suppress_printk = suppress_printk;
>       suppress_printk = 0;
>  
> @@ -596,7 +639,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) {
> +                             kfifo_put(&sysrq_nmi_fifo, key);
> +                             irq_work_queue(&sysrq_nmi_work);
> +                     } else {
> +                             op_p->handler(key);
> +                     }
>               } else {
>                       pr_info("This sysrq operation is disabled.\n");
>                       console_loglevel = orig_log_level;

I'm missing the point of that kfifo stuff; afaict it only ever buffers
_1_ key, might as well use a simple variable, no?


_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Reply via email to