Path: csiph.com!eternal-september.org!feeder.eternal-september.org!aioe.org!bofh.it!news.nic.it!robomod From: Sergey Senozhatsky Newsgroups: linux.kernel Subject: Re: [PATCH 1/7] printk: Hand over printing to console if printing too long Date: Thu, 31 Dec 2015 06:00:01 +0100 Message-ID: References: Dkim-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=QodR48QnkoOSSAykCdrqPAwpCy2GVy2qEzPsK0ztCKo=; b=QeLFu94x9Uzr6cBSf0/IDry/IQK1C1ateQOqD/TUd5DQTTSFvl05E7cDLC6rvtPRiG Oa7cmLbpqS6vs4Zc2qeHDZSXcvTNbId1FJBRmF61TEgdLH8tn2ZAZSXgkpeEcQW/b7L1 R1uCvtcfvm5TgvBe2rBG0ZOKU6KjDK3gD47NNI1INRd5B9JbLCxEnjs1QNDyVmJ4QcKy RAuyCWOTKBWEmgO9jC2ngzNVz+iwdSucXiVE6riDsH7bgjy1OX7Jt5ubKMf6D55S5IgS aMu3VEn6W7Z+5X7JefHhgEuqxXdXd2imHHDy0/OY41H/Scf/KqrwCY50vbDxMYXrUvl7 jtsw== X-Received: by 10.98.32.74 with SMTP id g71mr67228202pfg.46.1451537872119; Wed, 30 Dec 2015 20:57:52 -0800 (PST) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="4Ckj6UjgE2iN1+kY" Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) Sender: robomod@news.nic.it List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Approved: robomod@news.nic.it Lines: 200 Organization: linux.* mail to news gateway X-Original-Cc: Andrew Morton , Petr Mladek , KY Sri nivasan , Steven Rostedt , linux-kernel@vger.kernel.org, Sergey Senozhatsky , Sergey Senozhatsky X-Original-Date: Thu, 31 Dec 2015 13:58:59 +0900 X-Original-Message-ID: <20151231045859.GC479@swordfish> X-Original-References: <20151210145251.GA540@swordfish> <20151222134730.GD7266@quack.suse.cz> <20151231024459.GA479@swordfish> <20151231031337.GB479@swordfish> X-Original-Sender: linux-kernel-owner@vger.kernel.org Xref: csiph.com linux.kernel:1299575 --4Ckj6UjgE2iN1+kY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On (12/31/15 12:13), Sergey Senozhatsky wrote: [..] > cond_resched() does its job there, of course. well, a user process still can > do a lot of call_console_drivers() calls. may be we can check who is calling > console_unlock() and if we have "!printk_sync && !oops_in_progress" (or just printk_sync > test) AND a user process then return from console_unlock() doing irq_work_queue() > and set PRINTK_PENDING_OUTPUT pending bit, the way vprintk_emit() does it. attached two patches, I ended up having on top of yours. just in case. printk: factor out can_printk_async() console_unlock() can be called directly or indirectly by a user space process, so it can end up doing call_console_drivers() loop, which will hold it from returning back to user-space from a syscall for unpredictable amount of time. Factor out can_printk_async() function, which queues an irq work and sets a PRINTK_PENDING_OUTPUT pending bit (if we can do async printk). vprintk_emit() already does it, add can_printk_async() call to console_unlock() for !PF_KTHREAD processes. and printk: introduce console_sync_mode console_sync_mode() should be called early in panic() to switch printk from async mode to sync. Otherwise, STOP IPIs can arrive to other CPUs too late and those CPUs will see oops_in_progress being 0 again. -ss --4Ckj6UjgE2iN1+kY Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-printk-introduce-console_sync_mode.patch" From c3fc955809adab8f497cdc7581e67e1fa29d6517 Mon Sep 17 00:00:00 2001 From: Sergey Senozhatsky Date: Wed, 30 Dec 2015 20:39:12 +0900 Subject: [PATCH 1/2] printk: introduce console_sync_mode console_sync_mode() should be called early in panic() to switch printk from async mode to sync. Otherwise, STOP IPIs can arrive to other CPUs too late and those CPUs will see oops_in_progress being 0 again. --- include/linux/console.h | 1 + kernel/panic.c | 1 + kernel/printk/printk.c | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/include/linux/console.h b/include/linux/console.h index bd19434..f068985 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -150,6 +150,7 @@ extern int console_trylock(void); extern void console_unlock(void); extern void console_conditional_schedule(void); extern void console_unblank(void); +extern void console_sync_mode(void); extern struct tty_driver *console_device(int *); extern void console_stop(struct console *); extern void console_start(struct console *); diff --git a/kernel/panic.c b/kernel/panic.c index b333380..04c8ff4 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -117,6 +117,7 @@ void panic(const char *fmt, ...) if (old_cpu != PANIC_CPU_INVALID && old_cpu != this_cpu) panic_smp_self_stop(); + console_sync_mode(); console_verbose(); bust_spinlocks(1); va_start(args, fmt); diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index de9d31b..47a70a2 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2501,6 +2501,11 @@ void console_unblank(void) console_unlock(); } +void console_sync_mode(void) +{ + printk_sync = true; +} + /* * Return the console tty driver structure and its associated index */ -- 2.6.4 --4Ckj6UjgE2iN1+kY Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0002-printk-factor-out-can_printk_async.patch" From 92f2c0f2a5ed015caa2757dcfec4407d708f8628 Mon Sep 17 00:00:00 2001 From: Sergey Senozhatsky Date: Thu, 31 Dec 2015 13:39:58 +0900 Subject: [PATCH 2/2] printk: factor out can_printk_async() console_unlock() can be called directly or indirectly by a user space process, so it can end up doing call_console_drivers() loop, which will hold it from returning back to user-space from a syscall for unpredictable amount of time. Factor out can_printk_async() function, which queues an irq work and sets a PRINTK_PENDING_OUTPUT pending bit (if we can do async printk). vprintk_emit() already does it, add can_printk_async() call to console_unlock() for !PF_KTHREAD processes. --- kernel/printk/printk.c | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 47a70a2..7d3a8e1 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -355,6 +355,26 @@ int printk_deferred(const char *fmt, ...) return r; } +static bool can_printk_async(bool sync) +{ + /* + * By default we print message to console asynchronously so that kernel + * doesn't get stalled due to slow serial console. That can lead to + * softlockups, lost interrupts, or userspace timing out under heavy + * printing load. + * + * However we resort to synchronous printing of messages during early + * boot, when oops is in progress, or when synchronous printing was + * explicitely requested by kernel parameter. + */ + if (keventd_up() && !oops_in_progress && !sync) { + __this_cpu_or(printk_pending, PRINTK_PENDING_OUTPUT); + irq_work_queue(this_cpu_ptr(&wake_up_klogd_work)); + return true; + } + return false; +} + /* Return log buffer address */ char *log_buf_addr_get(void) { @@ -1889,20 +1909,7 @@ asmlinkage int vprintk_emit(int facility, int level, logbuf_cpu = UINT_MAX; raw_spin_unlock(&logbuf_lock); lockdep_on(); - /* - * By default we print message to console asynchronously so that kernel - * doesn't get stalled due to slow serial console. That can lead to - * softlockups, lost interrupts, or userspace timing out under heavy - * printing load. - * - * However we resort to synchronous printing of messages during early - * boot, when oops is in progress, or when synchronous printing was - * explicitely requested by kernel parameter. - */ - if (keventd_up() && !oops_in_progress && !sync_print) { - __this_cpu_or(printk_pending, PRINTK_PENDING_OUTPUT); - irq_work_queue(this_cpu_ptr(&wake_up_klogd_work)); - } else + if (!can_printk_async(sync_print)) sync_print = true; local_irq_restore(flags); @@ -2328,6 +2335,13 @@ void console_unlock(void) return; } + if (!(current->flags & PF_KTHREAD) && + can_printk_async(printk_sync)) { + console_locked = 0; + up_console_sem(); + return; + } + /* * Console drivers are called under logbuf_lock, so * @console_may_schedule should be cleared before; however, we may -- 2.6.4 --4Ckj6UjgE2iN1+kY-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/