Re: [PATCH v3] printk: Add console owner and waiter logic to load balance console writes
From: Vlastimil Babka
Date: Thu Nov 02 2017 - 18:17:45 EST
On 11/02/2017 06:45 PM, Steven Rostedt wrote:
...> __DEVKMSG_LOG_BIT_ON = 0,
> __DEVKMSG_LOG_BIT_OFF,
> @@ -1753,8 +1760,56 @@ asmlinkage int vprintk_emit(int facility
> * semaphore. The release will print out buffers and wake up
> * /dev/kmsg and syslog() users.
> */
> - if (console_trylock())
> + if (console_trylock()) {
> console_unlock();
> + } else {
> + struct task_struct *owner = NULL;
> + bool waiter;
> + bool spin = false;
> +
> + printk_safe_enter_irqsave(flags);
> +
> + raw_spin_lock(&console_owner_lock);
> + owner = READ_ONCE(console_owner);
> + waiter = READ_ONCE(console_waiter);
> + if (!waiter && owner && owner != current) {
> + WRITE_ONCE(console_waiter, true);
> + spin = true;
> + }
> + raw_spin_unlock(&console_owner_lock);
> +
> + /*
> + * If there is an active printk() writing to the
> + * consoles, instead of having it write our data too,
> + * see if we can offload that load from the active
> + * printer, and do some printing ourselves.
> + * Go into a spin only if there isn't already a waiter
> + * spinning, and there is an active printer, and
> + * that active printer isn't us (recursive printk?).
> + */
> + if (spin) {
> + /* We spin waiting for the owner to release us */
> + spin_acquire(&console_owner_dep_map, 0, 0, _THIS_IP_);
> + /* Owner will clear console_waiter on hand off */
> + while (!READ_ONCE(console_waiter))
This should not be negated, right? We should spin while it's true, not
false.
> + cpu_relax();
> +
> + spin_release(&console_owner_dep_map, 1, _THIS_IP_);
> + printk_safe_exit_irqrestore(flags);
> +
> + /*
> + * The owner passed the console lock to us.
> + * Since we did not spin on console lock, annotate
> + * this as a trylock. Otherwise lockdep will
> + * complain.
> + */
> + mutex_acquire(&console_lock_dep_map, 0, 1, _THIS_IP_);
> + console_unlock();
> + printk_safe_enter_irqsave(flags);
> + }
> + printk_safe_exit_irqrestore(flags);
> +
> + }
> }