lkml.org 
[lkml]   [2017]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [RFC][PATCHv2 1/8] printk: move printk_pending out of per-cpu
    On Wed 2017-03-29 18:25:04, Sergey Senozhatsky wrote:
    > Do not keep `printk_pending' in per-CPU area. We set the following bits
    > of printk_pending:
    > a) PRINTK_PENDING_WAKEUP
    > when we need to wakeup klogd
    > b) PRINTK_PENDING_OUTPUT
    > when there is a pending output from deferred printk and we need
    > to call console_unlock().
    >
    > So none of the bits control/represent a state of a particular CPU and,
    > basically, they should be global instead.
    >
    > Besides we will use `printk_pending' to control printk kthread, so this
    > patch is also a preparation work.
    >
    > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
    > Suggested-by: Petr Mladek <pmladek@suse.com>
    > ---
    > kernel/printk/printk.c | 26 ++++++++++++--------------
    > 1 file changed, 12 insertions(+), 14 deletions(-)
    >
    > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
    > index 2984fb0f0257..2d07678e9ff9 100644
    > --- a/kernel/printk/printk.c
    > +++ b/kernel/printk/printk.c
    > @@ -401,6 +401,14 @@ DEFINE_RAW_SPINLOCK(logbuf_lock);
    > printk_safe_exit_irqrestore(flags); \
    > } while (0)
    >
    > +/*
    > + * Delayed printk version, for scheduler-internal messages:
    > + */
    > +#define PRINTK_PENDING_WAKEUP 0x01
    > +#define PRINTK_PENDING_OUTPUT 0x02
    > +
    > +static unsigned long printk_pending;
    > +
    > #ifdef CONFIG_PRINTK
    > DECLARE_WAIT_QUEUE_HEAD(log_wait);
    > /* the next printk record to read by syslog(READ) or /proc/kmsg */
    > @@ -2654,25 +2662,15 @@ static int __init printk_late_init(void)
    > late_initcall(printk_late_init);
    >
    > #if defined CONFIG_PRINTK
    > -/*
    > - * Delayed printk version, for scheduler-internal messages:
    > - */
    > -#define PRINTK_PENDING_WAKEUP 0x01
    > -#define PRINTK_PENDING_OUTPUT 0x02
    > -
    > -static DEFINE_PER_CPU(int, printk_pending);
    > -
    > static void wake_up_klogd_work_func(struct irq_work *irq_work)
    > {
    > - int pending = __this_cpu_xchg(printk_pending, 0);
    > -
    > - if (pending & PRINTK_PENDING_OUTPUT) {
    > + if (test_and_clear_bit(PRINTK_PENDING_OUTPUT, &printk_pending)) {
    > /* If trylock fails, someone else is doing the printing */
    > if (console_trylock())
    > console_unlock();
    > }
    >
    > - if (pending & PRINTK_PENDING_WAKEUP)
    > + if (test_and_clear_bit(PRINTK_PENDING_WAKEUP, &printk_pending))
    > wake_up_interruptible(&log_wait);
    > }
    >
    > @@ -2685,7 +2683,7 @@ void wake_up_klogd(void)
    > {
    > preempt_disable();
    > if (waitqueue_active(&log_wait)) {
    > - this_cpu_or(printk_pending, PRINTK_PENDING_WAKEUP);
    > + set_bit(PRINTK_PENDING_WAKEUP, &printk_pending);

    We should add here a write barrier:

    /*
    * irq_work_queue() uses cmpxchg() and implies the memory
    * barrier only when the work is queued. An explicit barrier
    * is needed here to make sure that wake_up_klogd_work_func()
    * sees printk_pending set even when the work was already queued
    * because of an other pending event.
    */
    smp_wmb();

    > irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
    > }
    > preempt_enable();
    > @@ -2701,7 +2699,7 @@ int printk_deferred(const char *fmt, ...)
    > r = vprintk_emit(0, LOGLEVEL_SCHED, NULL, 0, fmt, args);
    > va_end(args);
    >
    > - __this_cpu_or(printk_pending, PRINTK_PENDING_OUTPUT);
    > + set_bit(PRINTK_PENDING_OUTPUT, &printk_pending);

    Same here.

    We are going to use printk_pending even more in the other patches.
    I still have to check the final state. It might be useful
    to define a helper if the barrier is needed on more locations.

    Otherwise the change looks fine to me. It should reduce redundant
    wakeups.

    Best Regards,
    Petr

    \
     
     \ /
      Last update: 2017-03-31 15:14    [W:4.182 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site