From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>, Jan Kara <jack@suse.com>,
Petr Mladek <pmladek@suse.com>, Kyle McMartin <kyle@kernel.org>,
Dave Jones <davej@codemonkey.org.uk>,
Calvin Owens <calvinowens@fb.com>,
linux-kernel@vger.kernel.org,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: [RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for_printk
Date: Thu, 14 Jan 2016 13:57:22 +0900 [thread overview]
Message-ID: <1452747443-9927-2-git-send-email-sergey.senozhatsky@gmail.com> (raw)
In-Reply-To: <1452747443-9927-1-git-send-email-sergey.senozhatsky@gmail.com>
vprintk_emit() disables preemption around console_trylock_for_printk()
and console_unlock() calls for a strong reason -- can_use_console()
check. The thing is that vprintl_emit() can be called on a CPU that
is not fully brought up yet (!cpu_online()), which potentially can
cause problems if console driver accesses per-cpu data. A console
driver can explicitly state that it's safe to call it from !online
cpu by setting CON_ANYTIME bit in console ->flags. That's why for
!cpu_online() can_use_console() iterates all the console to find out
if there is a CON_ANYTIME console, otherwise console_unlock() must be
avoided.
call_console_drivers(), called from console_cont_flush() and
console_unlock(), does the same test during for_each_console() loop.
However, we can have the following corner case. Assume that we have 2
cpus -- CPU0 is online, CPU1 is !online; and no CON_ANYTIME consoles
available.
CPU0 online CPU1 !online
console_trylock()
...
console_unlock()
console_cont_flush
spin_lock logbuf_lock
if (!cont.len) {
spin_unlock logbuf_lock
return
}
for (;;) {
vprintk_emit
spin_lock logbuf_lock
log_store
spin_unlock logbuf_lock
spin_lock logbuf_lock
!console_trylock_for_printk msg_print_text
return console_idx = log_next()
console_seq++
console_prev = msg->flags
spin_unlock logbuf_lock
call_console_drivers()
for_each_console(con) {
if (!cpu_online() &&
!(con->flags & CON_ANYTIME))
continue;
}
/*
* no message printed, we lost it
*/
vprintk_emit
spin_lock logbuf_lock
log_store
spin_unlock logbuf_lock
!console_trylock_for_printk
return
/*
* go to the beginning of the loop,
* find out there are new messages,
* lose it
*/
}
This patch moves can_use_console() check out of
console_trylock_for_printk(). Instead it calls it in two
non-preemptible sections:
-- console_cont_flush()
can_use_console() call happens with spin_lock logbuf_lock
taken and local IRQs disabled, local IRQs are not enabled
until call_console_drivers() returns.
-- console_unlock()
can_use_console() call happens with spin_lock logbuf_lock
taken and local IRQs disabled, local IRQs are not enabled
until call_console_drivers() returns.
In both cases, we need to know that call_console_drivers() call
was aborted because of !can_use_console() and we must exit
console_unlock() function.
This also lets to drop '!cpu_online() && !(con->flags & CON_ANYTIME)'
test from call_console_drivers().
At the same time this patch serves as a preparation to enable
cond_resched for 'in-direct' console_unlock() callers -- the ones
that come from vprintk_emit(). The 'direct' ones -- the ones that
do console_lock()/console_unlock() -- already can cond_resched.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
kernel/printk/printk.c | 71 +++++++++++++++++++++++++-------------------------
1 file changed, 36 insertions(+), 35 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 7ebcfea..5cc0ce6 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1452,9 +1452,6 @@ static void call_console_drivers(int level,
continue;
if (!con->write)
continue;
- if (!cpu_online(smp_processor_id()) &&
- !(con->flags & CON_ANYTIME))
- continue;
if (con->flags & CON_EXTENDED)
con->write(con, ext_text, ext_len);
else
@@ -1500,18 +1497,6 @@ static int have_callable_console(void)
}
/*
- * Can we actually use the console at this time on this cpu?
- *
- * Console drivers may assume that per-cpu resources have been allocated. So
- * unless they're explicitly marked as being able to cope (CON_ANYTIME) don't
- * call them until this CPU is officially up.
- */
-static inline int can_use_console(unsigned int cpu)
-{
- return cpu_online(cpu) || have_callable_console();
-}
-
-/*
* Try to get console ownership to actually show the kernel
* messages from a 'printk'. Return true (and with the
* console_lock held, and 'console_locked' set) if it
@@ -1519,21 +1504,7 @@ static inline int can_use_console(unsigned int cpu)
*/
static int console_trylock_for_printk(void)
{
- unsigned int cpu = smp_processor_id();
-
- if (!console_trylock())
- return 0;
- /*
- * If we can't use the console, we need to release the console
- * semaphore by hand to avoid flushing the buffer. We need to hold the
- * console semaphore in order to do this test safely.
- */
- if (!can_use_console(cpu)) {
- console_locked = 0;
- up_console_sem();
- return 0;
- }
- return 1;
+ return console_trylock();
}
int printk_delay_msec __read_mostly;
@@ -2177,10 +2148,23 @@ int is_console_locked(void)
return console_locked;
}
-static void console_cont_flush(char *text, size_t size)
+/*
+ * Can we actually use the console at this time on this cpu?
+ *
+ * Console drivers may assume that per-cpu resources have been allocated. So
+ * unless they're explicitly marked as being able to cope (CON_ANYTIME) don't
+ * call them until this CPU is officially up.
+ */
+static inline int can_use_console(void)
+{
+ return cpu_online(smp_processor_id()) || have_callable_console();
+}
+
+static bool console_cont_flush(char *text, size_t size)
{
unsigned long flags;
size_t len;
+ bool ret = true;
raw_spin_lock_irqsave(&logbuf_lock, flags);
@@ -2195,15 +2179,21 @@ static void console_cont_flush(char *text, size_t size)
if (console_seq < log_next_seq && !cont.cons)
goto out;
+ if (!can_use_console()) {
+ ret = false;
+ goto out;
+ }
+
len = cont_print_text(text, size);
raw_spin_unlock(&logbuf_lock);
stop_critical_timings();
call_console_drivers(cont.level, NULL, 0, text, len);
start_critical_timings();
local_irq_restore(flags);
- return;
+ return ret;
out:
raw_spin_unlock_irqrestore(&logbuf_lock, flags);
+ return ret;
}
/**
@@ -2227,7 +2217,7 @@ void console_unlock(void)
static u64 seen_seq;
unsigned long flags;
bool wake_klogd = false;
- bool do_cond_resched, retry;
+ bool do_cond_resched, retry, aborted = false;
if (console_suspended) {
up_console_sem();
@@ -2248,7 +2238,12 @@ void console_unlock(void)
console_may_schedule = 0;
/* flush buffered message fragment immediately to console */
- console_cont_flush(text, sizeof(text));
+ if (!console_cont_flush(text, sizeof(text))) {
+ console_locked = 0;
+ up_console_sem();
+ return;
+ }
+
again:
for (;;) {
struct printk_log *msg;
@@ -2257,6 +2252,12 @@ again:
int level;
raw_spin_lock_irqsave(&logbuf_lock, flags);
+
+ if (!can_use_console()) {
+ aborted = true;
+ break;
+ }
+
if (seen_seq != log_next_seq) {
wake_klogd = true;
seen_seq = log_next_seq;
@@ -2337,7 +2338,7 @@ skip:
* flush, no worries.
*/
raw_spin_lock(&logbuf_lock);
- retry = console_seq != log_next_seq;
+ retry = !aborted && (console_seq != log_next_seq);
raw_spin_unlock_irqrestore(&logbuf_lock, flags);
if (retry && console_trylock())
--
2.7.0
next prev parent reply other threads:[~2016-01-14 4:57 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-14 4:57 [RFC][PATCH -next 0/2] cond_resched() some of console_trylock callers Sergey Senozhatsky
2016-01-14 4:57 ` Sergey Senozhatsky [this message]
2016-01-14 5:59 ` [PATCH 1/2] printk: move can_use_console out of console_trylock_for_printk Sergey Senozhatsky
2016-01-18 15:42 ` [RFC][PATCH -next " Petr Mladek
2016-01-19 0:42 ` Sergey Senozhatsky
2016-01-19 13:31 ` Petr Mladek
2016-01-19 15:00 ` Sergey Senozhatsky
2016-01-19 16:16 ` Petr Mladek
2016-01-20 4:18 ` Sergey Senozhatsky
2016-01-20 10:09 ` Petr Mladek
2016-01-14 4:57 ` [RFC][PATCH -next 2/2] printk: set may_schedule for some of console_trylock callers Sergey Senozhatsky
2016-01-17 14:11 ` Sergey Senozhatsky
2016-01-17 14:23 ` Sergey Senozhatsky
2016-01-18 16:17 ` Petr Mladek
2016-01-19 1:15 ` Sergey Senozhatsky
2016-01-19 15:18 ` Petr Mladek
2016-01-20 3:50 ` Sergey Senozhatsky
2016-01-20 11:51 ` Sergey Senozhatsky
2016-01-20 12:38 ` Petr Mladek
2016-01-20 12:31 ` Petr Mladek
2016-01-21 1:25 ` Sergey Senozhatsky
2016-01-21 5:51 ` Sergey Senozhatsky
2016-01-22 9:48 ` Petr Mladek
2016-01-23 4:46 ` Sergey Senozhatsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1452747443-9927-2-git-send-email-sergey.senozhatsky@gmail.com \
--to=sergey.senozhatsky.work@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=calvinowens@fb.com \
--cc=davej@codemonkey.org.uk \
--cc=jack@suse.com \
--cc=kyle@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pmladek@suse.com \
--cc=sergey.senozhatsky@gmail.com \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®