Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1308993 > unrolled thread
| Started by | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| First post | 2016-01-14 06:00 +0100 |
| Last post | 2016-01-14 06:00 +0100 |
| Articles | 4 — 1 participant |
Back to article view | Back to linux.kernel
[RFC][PATCH -next 0/2] cond_resched() some of console_trylock callers Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-14 06:00 +0100
[RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for_printk Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-14 06:00 +0100
[PATCH 1/2] printk: move can_use_console out of console_trylock_for_printk Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-14 07:00 +0100
[RFC][PATCH -next 2/2] printk: set may_schedule for some of console_trylock callers Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-14 06:00 +0100
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2016-01-14 06:00 +0100 |
| Subject | [RFC][PATCH -next 0/2] cond_resched() some of console_trylock callers |
| Message-ID | <qQIaS-1Pl-1@gated-at.bofh.it> |
Hello, RFC (apologies for sending it during the merge window. no rush) console_unlock() allows to cond_resched() if its caller has set `console_may_schedule' to 1 (this functionality present since commit 'printk: do cond_resched() between lines while outputting to consoles'). The rules are: -- console_lock() always sets `console_may_schedule' to 1 -- console_trylock() always sets `console_may_schedule' to 0 However, console_trylock() callers (among them is printk()) are not necessarily executing in atomic contexts, and some of them can cond_resched() in console_unlock(). So console_trylock() can set `console_may_schedule' to 0 only if cond_resched() is invalid in the current context, and set it to 1 otherwise. Patch 0001 addresses a theoretical loss of printk messages and, at the same time, serves as a preparation work for 0002. Sergey Senozhatsky (2): printk: move can_use_console out of console_trylock_for_printk printk: set may_schedule for some of console_trylock callers kernel/printk/printk.c | 84 ++++++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 44 deletions(-) -- 2.7.0
[toc] | [next] | [standalone]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2016-01-14 06:00 +0100 |
| Subject | [RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for_printk |
| Message-ID | <qQIaT-1Pl-17@gated-at.bofh.it> |
| In reply to | #1308993 |
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
[toc] | [prev] | [next] | [standalone]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2016-01-14 07:00 +0100 |
| Subject | [PATCH 1/2] printk: move can_use_console out of console_trylock_for_printk |
| Message-ID | <qQJ6W-2CR-7@gated-at.bofh.it> |
| In reply to | #1308997 |
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:
1) 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.
2) 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>
---
V2:
make have_callable_console() available for !PRINTK configs (lkp@intel.com)
kernel/printk/printk.c | 101 +++++++++++++++++++++++++------------------------
1 file changed, 51 insertions(+), 50 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 7ebcfea..ae641d7 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
@@ -1485,33 +1482,6 @@ static void zap_locks(void)
}
/*
- * Check if we have any console that is capable of printing while cpu is
- * booting or shutting down. Requires console_sem.
- */
-static int have_callable_console(void)
-{
- struct console *con;
-
- for_each_console(con)
- if (con->flags & CON_ANYTIME)
- return 1;
-
- return 0;
-}
-
-/*
- * 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 +1489,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 +2133,38 @@ int is_console_locked(void)
return console_locked;
}
-static void console_cont_flush(char *text, size_t size)
+/*
+ * Check if we have any console that is capable of printing while cpu is
+ * booting or shutting down. Requires console_sem.
+ */
+static int have_callable_console(void)
+{
+ struct console *con;
+
+ for_each_console(con)
+ if (con->flags & CON_ANYTIME)
+ return 1;
+
+ return 0;
+}
+
+/*
+ * 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
[toc] | [prev] | [next] | [standalone]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2016-01-14 06:00 +0100 |
| Subject | [RFC][PATCH -next 2/2] printk: set may_schedule for some of console_trylock callers |
| Message-ID | <qQIaT-1Pl-19@gated-at.bofh.it> |
| In reply to | #1308993 |
console_unlock() allows to cond_resched() if its caller has
set `console_may_schedule' to 1 (this functionality present
since commit 'printk: do cond_resched() between lines while
outputting to consoles').
The rules are:
-- console_lock() always sets `console_may_schedule' to 1
-- console_trylock() always sets `console_may_schedule' to 0
However, console_trylock() callers (among them is printk()) are
not necessarily executing in atomic contexts, and some of them
can cond_resched() in console_unlock(). So console_trylock()
can set `console_may_schedule' to 0 only if cond_resched() is
invalid in the current context, and set it to 1 otherwise.
The patch also drops explicit preempt_disable()/preempt_enable()
calls in vprintk_emit().
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
kernel/printk/printk.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 5cc0ce6..d0f5a73 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1792,20 +1792,12 @@ asmlinkage int vprintk_emit(int facility, int level,
if (!in_sched) {
lockdep_off();
/*
- * Disable preemption to avoid being preempted while holding
- * console_sem which would prevent anyone from printing to
- * console
- */
- preempt_disable();
-
- /*
* Try to acquire and then immediately release the console
* semaphore. The release will print out buffers and wake up
* /dev/kmsg and syslog() users.
*/
if (console_trylock_for_printk())
console_unlock();
- preempt_enable();
lockdep_on();
}
@@ -2138,7 +2130,10 @@ int console_trylock(void)
return 0;
}
console_locked = 1;
- console_may_schedule = 0;
+ console_may_schedule = !(oops_in_progress ||
+ in_interrupt() ||
+ irqs_disabled() ||
+ in_atomic());
return 1;
}
EXPORT_SYMBOL(console_trylock);
--
2.7.0
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web