Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup

[Steven Rostedt]

On Wed, 17 Jan 2018 12:12:51 -0500
Steven Rostedt <rost...@goodmis.org> wrote:

> @@ -2393,15 +2451,20 @@ void console_unlock(void)
>                * waiter waiting to take over.
>                */
>               console_lock_spinning_enable();
> +             offload = recursion_check_start();
>  
>               stop_critical_timings();        /* don't trace print latency */
>               call_console_drivers(ext_text, ext_len, text, len);
>               start_critical_timings();
>  
> +             recursion_check_finish(offload);
> +
>               if (console_lock_spinning_disable_and_check()) {
>                       printk_safe_exit_irqrestore(flags);
>                       return;
>               }
> +             if (offload)
> +                     kick_offload_thread();
>  

Ah, major flaw in this code. The recursion check needs to be in
printk() itself around the trylock.

-- Steve

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 9cb943c90d98..31df145cc4d7 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1826,6 +1826,63 @@ static size_t log_output(int facility, int level, enum 
log_flags lflags, const c
        /* Store it in the record log */
        return log_store(facility, level, lflags, 0, dict, dictlen, text, 
text_len);
 }
+/*
+ * Used for which context the printk is in.
+ *  NMI     = 0
+ *  IRQ     = 1
+ *  SOFTIRQ = 2
+ *  NORMAL  = 3
+ *
+ * Stack ordered, where the lower number can preempt
+ * the higher number: mask &= mask - 1, will only clear
+ * the lowerest set bit.
+ */
+enum {
+       CTX_NMI,
+       CTX_IRQ,
+       CTX_SOFTIRQ,
+       CTX_NORMAL,
+};
+
+static DEFINE_PER_CPU(int, recursion_bits);
+
+static bool recursion_check_start(void)
+{
+       unsigned long pc = preempt_count();
+       int val = this_cpu_read(recursion_bits);
+
+       if (!(pc & (NMI_MASK | HARDIRQ_MASK | SOFTIRQ_OFFSET)))
+               bit = CTX_NORMAL;
+       else
+               bit = pc & NMI_MASK ? CTX_NMI :
+                       pc & HARDIRQ_MASK ? CTX_IRQ : CTX_SOFTIRQ;
+
+       if (unlikely(val & (1 << bit)))
+               return true;
+
+       val |= (1 << bit);
+       this_cpu_write(recursion_bits, val);
+       return false;
+}
+
+static void recursion_check_finish(bool offload)
+{
+       int val = this_cpu_read(recursion_bits);
+
+       if (offload)
+               return;
+
+       val &= val - 1;
+       this_cpu_write(recursion_bits, val);
+}
+
+static void kick_offload_thread(void)
+{
+       /*
+        * Consoles are triggering printks, offload the printks
+        * to another CPU to hopefully avoid a lockup.
+        */
+}
 
 asmlinkage int vprintk_emit(int facility, int level,
                            const char *dict, size_t dictlen,
@@ -1895,12 +1952,14 @@ asmlinkage int vprintk_emit(int facility, int level,
 
        /* If called from the scheduler, we can not call up(). */
        if (!in_sched) {
+               bool offload;
                /*
                 * Disable preemption to avoid being preempted while holding
                 * console_sem which would prevent anyone from printing to
                 * console
                 */
                preempt_disable();
+               offload = recursion_check_start();
                /*
                 * Try to acquire and then immediately release the console
                 * semaphore.  The release will print out buffers and wake up
@@ -1908,7 +1967,12 @@ asmlinkage int vprintk_emit(int facility, int level,
                 */
                if (console_trylock_spinning())
                        console_unlock();
+
+               recursion_check_finish(offload);
                preempt_enable();
+
+               if (offload)
+                       kick_offload_thread();
        }
 
        return printed_len;