* [PATCH 1/4] printk: Re-add irqsave/restore in printk_sched
2014-05-02 22:09 [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2) John Stultz
@ 2014-05-02 22:09 ` John Stultz
2014-05-04 14:58 ` Jan Kara
2014-05-02 22:09 ` [PATCH 2/4] printk: Rename printk_sched to printk_deferred John Stultz
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: John Stultz @ 2014-05-02 22:09 UTC (permalink / raw)
To: LKML
Cc: John Stultz, Jan Kara, Peter Zijlstra, Jiri Bohac,
Thomas Gleixner, Ingo Molnar, Andrew Morton, Steven Rostedt
A commit in akpm's tree (printk: remove separate printk_sched
buffers...), removed the printk_sched irqsave/restore lines
since it was safe for current users. Since we may be expanding
usage of printk_sched(), re-add the irqsave/restore logic
to make the functionality more generally safe.
Cc: Jan Kara <jack@suse.cz>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
kernel/printk/printk.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 82d19e6..bf62f2b 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2586,15 +2586,18 @@ void wake_up_klogd(void)
int printk_sched(const char *fmt, ...)
{
+ unsigned long flags;
va_list args;
int r;
+ local_irq_save(flags);
va_start(args, fmt);
r = vprintk_emit(0, SCHED_MESSAGE_LOGLEVEL, NULL, 0, fmt, args);
va_end(args);
__this_cpu_or(printk_pending, PRINTK_PENDING_OUTPUT);
irq_work_queue(&__get_cpu_var(wake_up_klogd_work));
+ local_irq_restore(flags);
return r;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] printk: Re-add irqsave/restore in printk_sched
2014-05-02 22:09 ` [PATCH 1/4] printk: Re-add irqsave/restore in printk_sched John Stultz
@ 2014-05-04 14:58 ` Jan Kara
2014-05-04 16:13 ` Peter Zijlstra
0 siblings, 1 reply; 13+ messages in thread
From: Jan Kara @ 2014-05-04 14:58 UTC (permalink / raw)
To: John Stultz
Cc: LKML, Jan Kara, Peter Zijlstra, Jiri Bohac, Thomas Gleixner,
Ingo Molnar, Andrew Morton, Steven Rostedt
On Fri 02-05-14 15:09:15, John Stultz wrote:
> A commit in akpm's tree (printk: remove separate printk_sched
> buffers...), removed the printk_sched irqsave/restore lines
> since it was safe for current users. Since we may be expanding
> usage of printk_sched(), re-add the irqsave/restore logic
> to make the functionality more generally safe.
So I'm just wondering: Do you have anything particular for which you need
interrupts disabled? Won't e.g. disabling preemption be enough?
Honza
> Cc: Jan Kara <jack@suse.cz>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Jiri Bohac <jbohac@suse.cz>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> kernel/printk/printk.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 82d19e6..bf62f2b 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2586,15 +2586,18 @@ void wake_up_klogd(void)
>
> int printk_sched(const char *fmt, ...)
> {
> + unsigned long flags;
> va_list args;
> int r;
>
> + local_irq_save(flags);
> va_start(args, fmt);
> r = vprintk_emit(0, SCHED_MESSAGE_LOGLEVEL, NULL, 0, fmt, args);
> va_end(args);
>
> __this_cpu_or(printk_pending, PRINTK_PENDING_OUTPUT);
> irq_work_queue(&__get_cpu_var(wake_up_klogd_work));
> + local_irq_restore(flags);
>
> return r;
> }
> --
> 1.9.1
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] printk: Re-add irqsave/restore in printk_sched
2014-05-04 14:58 ` Jan Kara
@ 2014-05-04 16:13 ` Peter Zijlstra
2014-05-05 20:16 ` John Stultz
0 siblings, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2014-05-04 16:13 UTC (permalink / raw)
To: Jan Kara
Cc: John Stultz, LKML, Jiri Bohac, Thomas Gleixner, Ingo Molnar,
Andrew Morton, Steven Rostedt
On Sun, May 04, 2014 at 04:58:47PM +0200, Jan Kara wrote:
> On Fri 02-05-14 15:09:15, John Stultz wrote:
> > A commit in akpm's tree (printk: remove separate printk_sched
> > buffers...), removed the printk_sched irqsave/restore lines
> > since it was safe for current users. Since we may be expanding
> > usage of printk_sched(), re-add the irqsave/restore logic
> > to make the functionality more generally safe.
> So I'm just wondering: Do you have anything particular for which you need
> interrupts disabled? Won't e.g. disabling preemption be enough?
Yeah, I think preemption disabled is sufficient.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] printk: Re-add irqsave/restore in printk_sched
2014-05-04 16:13 ` Peter Zijlstra
@ 2014-05-05 20:16 ` John Stultz
0 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2014-05-05 20:16 UTC (permalink / raw)
To: Peter Zijlstra, Jan Kara
Cc: LKML, Jiri Bohac, Thomas Gleixner, Ingo Molnar, Andrew Morton,
Steven Rostedt
On 05/04/2014 09:13 AM, Peter Zijlstra wrote:
> On Sun, May 04, 2014 at 04:58:47PM +0200, Jan Kara wrote:
>> On Fri 02-05-14 15:09:15, John Stultz wrote:
>>> A commit in akpm's tree (printk: remove separate printk_sched
>>> buffers...), removed the printk_sched irqsave/restore lines
>>> since it was safe for current users. Since we may be expanding
>>> usage of printk_sched(), re-add the irqsave/restore logic
>>> to make the functionality more generally safe.
>> So I'm just wondering: Do you have anything particular for which you need
>> interrupts disabled? Won't e.g. disabling preemption be enough?
> Yeah, I think preemption disabled is sufficient.
Ok, will change this and resend.
thanks
-john
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] printk: Rename printk_sched to printk_deferred
2014-05-02 22:09 [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2) John Stultz
2014-05-02 22:09 ` [PATCH 1/4] printk: Re-add irqsave/restore in printk_sched John Stultz
@ 2014-05-02 22:09 ` John Stultz
2014-05-02 22:09 ` [PATCH 3/4] printk: Add printk_once_deferred John Stultz
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2014-05-02 22:09 UTC (permalink / raw)
To: LKML
Cc: John Stultz, Jan Kara, Peter Zijlstra, Jiri Bohac,
Thomas Gleixner, Ingo Molnar, Andrew Morton, Steven Rostedt
After learning we'll need some sort of deferred printk functionality
in the timekeeping core, Peter suggested we rename the printk_sched
function so it can be reused by needed subsystems.
This only changes the function name and name of the associated buffer.
No logic changes.
Cc: Jan Kara <jack@suse.cz>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
include/linux/printk.h | 6 +++---
kernel/printk/printk.c | 2 +-
kernel/sched/core.c | 2 +-
kernel/sched/deadline.c | 2 +-
kernel/sched/rt.c | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 8752f75..7847301 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -128,9 +128,9 @@ asmlinkage __printf(1, 2) __cold
int printk(const char *fmt, ...);
/*
- * Special printk facility for scheduler use only, _DO_NOT_USE_ !
+ * Special printk facility for scheduler/timekeeping use only, _DO_NOT_USE_ !
*/
-__printf(1, 2) __cold int printk_sched(const char *fmt, ...);
+__printf(1, 2) __cold int printk_deferred(const char *fmt, ...);
/*
* Please don't use printk_ratelimit(), because it shares ratelimiting state
@@ -165,7 +165,7 @@ int printk(const char *s, ...)
return 0;
}
static inline __printf(1, 2) __cold
-int printk_sched(const char *s, ...)
+int printk_deferred(const char *s, ...)
{
return 0;
}
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index bf62f2b..ffcb487 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2584,7 +2584,7 @@ void wake_up_klogd(void)
preempt_enable();
}
-int printk_sched(const char *fmt, ...)
+int printk_deferred(const char *fmt, ...)
{
unsigned long flags;
va_list args;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 805b8a9..263c790 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1336,7 +1336,7 @@ out:
* leave kernel.
*/
if (p->mm && printk_ratelimit()) {
- printk_sched("process %d (%s) no longer affine to cpu%d\n",
+ printk_deferred("process %d (%s) no longer affine to cpu%d\n",
task_pid_nr(p), p->comm, cpu);
}
}
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index b080957..657ed68 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -352,7 +352,7 @@ static void replenish_dl_entity(struct sched_dl_entity *dl_se,
if (!lag_once) {
lag_once = true;
- printk_sched("sched: DL replenish lagged to much\n");
+ printk_deferred("sched: DL replenish lagged to much\n");
}
dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
dl_se->runtime = pi_se->dl_runtime;
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 7795e29..e7dc728 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -896,7 +896,7 @@ static int sched_rt_runtime_exceeded(struct rt_rq *rt_rq)
if (!once) {
once = true;
- printk_sched("sched: RT throttling activated\n");
+ printk_deferred("sched: RT throttling activated\n");
}
} else {
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] printk: Add printk_once_deferred
2014-05-02 22:09 [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2) John Stultz
2014-05-02 22:09 ` [PATCH 1/4] printk: Re-add irqsave/restore in printk_sched John Stultz
2014-05-02 22:09 ` [PATCH 2/4] printk: Rename printk_sched to printk_deferred John Stultz
@ 2014-05-02 22:09 ` John Stultz
2014-05-02 22:09 ` [PATCH 4/4] timekeeping: Use printk_deferred when holding timekeeping seqlock John Stultz
2014-05-02 23:05 ` [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2) Andrew Morton
4 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2014-05-02 22:09 UTC (permalink / raw)
To: LKML
Cc: John Stultz, Jan Kara, Peter Zijlstra, Jiri Bohac,
Thomas Gleixner, Ingo Molnar, Andrew Morton, Steven Rostedt
Two of the three prink_deferred uses are really printk_once style
uses, so add a printk_once_deferred macro to simplify those call
sites.
Cc: Jan Kara <jack@suse.cz>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
include/linux/printk.h | 11 +++++++++++
kernel/sched/deadline.c | 7 +------
kernel/sched/rt.c | 8 +-------
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 7847301..bd21234 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -266,9 +266,20 @@ extern asmlinkage void dump_stack(void) __cold;
printk(fmt, ##__VA_ARGS__); \
} \
})
+#define printk_once_deferred(fmt, ...) \
+({ \
+ static bool __print_once __read_mostly; \
+ \
+ if (!__print_once) { \
+ __print_once = true; \
+ printk_deferred(fmt, ##__VA_ARGS__); \
+ } \
+})
#else
#define printk_once(fmt, ...) \
no_printk(fmt, ##__VA_ARGS__)
+#define printk_once_deferred(fmt, ...) \
+ no_printk(fmt, ##__VA_ARGS__)
#endif
#define pr_emerg_once(fmt, ...) \
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 657ed68..3ec96bd 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -348,12 +348,7 @@ static void replenish_dl_entity(struct sched_dl_entity *dl_se,
* entity.
*/
if (dl_time_before(dl_se->deadline, rq_clock(rq))) {
- static bool lag_once = false;
-
- if (!lag_once) {
- lag_once = true;
- printk_deferred("sched: DL replenish lagged to much\n");
- }
+ printk_once_deferred("sched: DL replenish lagged to much\n");
dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
dl_se->runtime = pi_se->dl_runtime;
}
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index e7dc728..fd6e9ca 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -890,14 +890,8 @@ static int sched_rt_runtime_exceeded(struct rt_rq *rt_rq)
* but accrue some time due to boosting.
*/
if (likely(rt_b->rt_runtime)) {
- static bool once = false;
-
rt_rq->rt_throttled = 1;
-
- if (!once) {
- once = true;
- printk_deferred("sched: RT throttling activated\n");
- }
+ printk_once_deferred("sched: RT throttling activated\n");
} else {
/*
* In case we did anyway, make it go away,
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] timekeeping: Use printk_deferred when holding timekeeping seqlock
2014-05-02 22:09 [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2) John Stultz
` (2 preceding siblings ...)
2014-05-02 22:09 ` [PATCH 3/4] printk: Add printk_once_deferred John Stultz
@ 2014-05-02 22:09 ` John Stultz
2014-05-02 23:05 ` [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2) Andrew Morton
4 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2014-05-02 22:09 UTC (permalink / raw)
To: LKML
Cc: John Stultz, Jan Kara, Peter Zijlstra, Jiri Bohac,
Thomas Gleixner, Ingo Molnar, Andrew Morton, Steven Rostedt
Jiri Bohac pointed out that there are rare but potential deadlock
possibilities when calling printk while holding the timekeeping
seqlock.
This is due to printk() triggering console sem wakeup, which can
cause scheduling code to trigger hrtimers which may try to read
the time.
Specifically, as Jiri pointed out, that path is:
printk
vprintk_emit
console_unlock
up(&console_sem)
__up
wake_up_process
try_to_wake_up
ttwu_do_activate
ttwu_activate
activate_task
enqueue_task
enqueue_task_fair
hrtick_update
hrtick_start_fair
hrtick_start_fair
get_time
ktime_get
--> endless loop on
read_seqcount_retry(&timekeeper_seq, ...)
This patch tries to avoid this issue by using printk_deferred (previously
named printk_sched) which should defer printing via a irq_work_queue.
Cc: Jan Kara <jack@suse.cz>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Reported-by: Jiri Bohac <jbohac@suse.cz>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
kernel/time/ntp.c | 15 +++++++++------
kernel/time/timekeeping.c | 7 ++++---
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 419a52c..5b0ac4d 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -786,8 +786,9 @@ static long hardpps_update_freq(struct pps_normtime freq_norm)
time_status |= STA_PPSERROR;
pps_errcnt++;
pps_dec_freq_interval();
- pr_err("hardpps: PPSERROR: interval too long - %ld s\n",
- freq_norm.sec);
+ printk_deferred(KERN_ERR
+ "hardpps: PPSERROR: interval too long - %ld s\n",
+ freq_norm.sec);
return 0;
}
@@ -800,7 +801,8 @@ static long hardpps_update_freq(struct pps_normtime freq_norm)
delta = shift_right(ftemp - pps_freq, NTP_SCALE_SHIFT);
pps_freq = ftemp;
if (delta > PPS_MAXWANDER || delta < -PPS_MAXWANDER) {
- pr_warning("hardpps: PPSWANDER: change=%ld\n", delta);
+ printk_deferred(KERN_WARNING
+ "hardpps: PPSWANDER: change=%ld\n", delta);
time_status |= STA_PPSWANDER;
pps_stbcnt++;
pps_dec_freq_interval();
@@ -844,8 +846,9 @@ static void hardpps_update_phase(long error)
* the time offset is updated.
*/
if (jitter > (pps_jitter << PPS_POPCORN)) {
- pr_warning("hardpps: PPSJITTER: jitter=%ld, limit=%ld\n",
- jitter, (pps_jitter << PPS_POPCORN));
+ printk_deferred(KERN_WARNING
+ "hardpps: PPSJITTER: jitter=%ld, limit=%ld\n",
+ jitter, (pps_jitter << PPS_POPCORN));
time_status |= STA_PPSJITTER;
pps_jitcnt++;
} else if (time_status & STA_PPSTIME) {
@@ -902,7 +905,7 @@ void __hardpps(const struct timespec *phase_ts, const struct timespec *raw_ts)
time_status |= STA_PPSJITTER;
/* restart the frequency calibration interval */
pps_fbase = *raw_ts;
- pr_err("hardpps: PPSJITTER: bad pulse\n");
+ printk_deferred(KERN_ERR "hardpps: PPSJITTER: bad pulse\n");
return;
}
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index f7df8ea..ffd3113 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -852,8 +852,9 @@ static void __timekeeping_inject_sleeptime(struct timekeeper *tk,
struct timespec *delta)
{
if (!timespec_valid_strict(delta)) {
- printk(KERN_WARNING "__timekeeping_inject_sleeptime: Invalid "
- "sleep delta value!\n");
+ printk_deferred(KERN_WARNING
+ "__timekeeping_inject_sleeptime: Invalid "
+ "sleep delta value!\n");
return;
}
tk_xtime_add(tk, delta);
@@ -1157,7 +1158,7 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
if (unlikely(tk->clock->maxadj &&
(tk->mult + adj > tk->clock->mult + tk->clock->maxadj))) {
- printk_once(KERN_WARNING
+ printk_once_deferred(KERN_WARNING
"Adjusting %s more than 11%% (%ld vs %ld)\n",
tk->clock->name, (long)tk->mult + adj,
(long)tk->clock->mult + tk->clock->maxadj);
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2)
2014-05-02 22:09 [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2) John Stultz
` (3 preceding siblings ...)
2014-05-02 22:09 ` [PATCH 4/4] timekeeping: Use printk_deferred when holding timekeeping seqlock John Stultz
@ 2014-05-02 23:05 ` Andrew Morton
2014-05-05 15:30 ` Steven Rostedt
` (2 more replies)
4 siblings, 3 replies; 13+ messages in thread
From: Andrew Morton @ 2014-05-02 23:05 UTC (permalink / raw)
To: John Stultz
Cc: LKML, Jan Kara, Peter Zijlstra, Jiri Bohac, Thomas Gleixner,
Ingo Molnar, Steven Rostedt
On Fri, 2 May 2014 15:09:14 -0700 John Stultz <john.stultz@linaro.org> wrote:
> Recently, Jiri pointed out a potential deadlock when calling printk
> while holding the timekeeping seqlock.
>
> Annoyingly, the seqlock lockdep enablement doesn't catch this, as
> printk disables lockdep.
>
> When looking for possible solutions, one idea was to use a local buffer
> and defer the printk to later. Ends up there is already similar
> functionality in printk_sched() to avoid similar style deadlocks w/
> the scheduler.
>
> Thus this patchset (based on next/akpm) renames printk_sched to
> printk_deferred and then moves the affected timekeeping printks to make
> use of it.
>
> There were some points in the discussion between Jan and Peter that
> made it seem that there may still be problems lurking in the console
> layer, and I'm not sure I fully understand their point, so this solution
> may be incomplete.
>
> Additionally, the same issue likely affects any WARN_ONs as well, but
> I wanted to get some thoughts on this approach before trying to remove
> or convert affected WARN_ONS.
>
> Your thoughts and feedback are greatly appreciated!
All look pretty simple and sane to me. printk is a crazy hotspot
lately but this patchset looks like it won't get singed.
Would "printk_deferred_once" be more logical than
"printk_once_deferred"? Think so. It's (((printk(deferred(once))),
not (((printk(once(deferred))).
Why do I see a pr_emerg_once_deferred() in my future?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2)
2014-05-02 23:05 ` [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2) Andrew Morton
@ 2014-05-05 15:30 ` Steven Rostedt
2014-05-05 18:42 ` Steven Rostedt
2014-05-05 20:15 ` John Stultz
2 siblings, 0 replies; 13+ messages in thread
From: Steven Rostedt @ 2014-05-05 15:30 UTC (permalink / raw)
To: Andrew Morton
Cc: John Stultz, LKML, Jan Kara, Peter Zijlstra, Jiri Bohac,
Thomas Gleixner, Ingo Molnar
On Fri, 2 May 2014 16:05:36 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> Would "printk_deferred_once" be more logical than
> "printk_once_deferred"? Think so. It's (((printk(deferred(once))),
> not (((printk(once(deferred))).
Or printk_once_removed()? Or does that only deal with cousins?
-- Steve
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2)
2014-05-02 23:05 ` [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2) Andrew Morton
2014-05-05 15:30 ` Steven Rostedt
@ 2014-05-05 18:42 ` Steven Rostedt
2014-05-05 20:25 ` John Stultz
2014-05-05 20:15 ` John Stultz
2 siblings, 1 reply; 13+ messages in thread
From: Steven Rostedt @ 2014-05-05 18:42 UTC (permalink / raw)
To: Andrew Morton
Cc: John Stultz, LKML, Jan Kara, Peter Zijlstra, Jiri Bohac,
Thomas Gleixner, Ingo Molnar
On Fri, 2 May 2014 16:05:36 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> Would "printk_deferred_once" be more logical than
> "printk_once_deferred"? Think so. It's (((printk(deferred(once))),
> not (((printk(once(deferred))).
>
I agree with the above, but other than that you can add my:
Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
also, perhaps this series should include my patch:
https://lkml.org/lkml/2014/3/25/336
-- Steve
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2)
2014-05-05 18:42 ` Steven Rostedt
@ 2014-05-05 20:25 ` John Stultz
0 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2014-05-05 20:25 UTC (permalink / raw)
To: Steven Rostedt, Andrew Morton
Cc: LKML, Jan Kara, Peter Zijlstra, Jiri Bohac, Thomas Gleixner, Ingo Molnar
On 05/05/2014 11:42 AM, Steven Rostedt wrote:
> On Fri, 2 May 2014 16:05:36 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
>
>
>> Would "printk_deferred_once" be more logical than
>> "printk_once_deferred"? Think so. It's (((printk(deferred(once))),
>> not (((printk(once(deferred))).
>>
> I agree with the above, but other than that you can add my:
>
> Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
Thanks, will do!
>
> also, perhaps this series should include my patch:
>
> https://lkml.org/lkml/2014/3/25/336
So this set applies ontop of -mm, which I believe includes your change.
If I try to cherry-pick your patch to include in my set, I think I have
to cherry-pick Jan's changes as well, so its probably just easier for
folks to grab -mm. :)
thanks
-john
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2)
2014-05-02 23:05 ` [PATCH 0/4] Convert timekeeping core to use printk_deferred (v2) Andrew Morton
2014-05-05 15:30 ` Steven Rostedt
2014-05-05 18:42 ` Steven Rostedt
@ 2014-05-05 20:15 ` John Stultz
2 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2014-05-05 20:15 UTC (permalink / raw)
To: Andrew Morton
Cc: LKML, Jan Kara, Peter Zijlstra, Jiri Bohac, Thomas Gleixner,
Ingo Molnar, Steven Rostedt
On 05/02/2014 04:05 PM, Andrew Morton wrote:
> On Fri, 2 May 2014 15:09:14 -0700 John Stultz <john.stultz@linaro.org> wrote:
>
>> Recently, Jiri pointed out a potential deadlock when calling printk
>> while holding the timekeeping seqlock.
>>
>> Annoyingly, the seqlock lockdep enablement doesn't catch this, as
>> printk disables lockdep.
>>
>> When looking for possible solutions, one idea was to use a local buffer
>> and defer the printk to later. Ends up there is already similar
>> functionality in printk_sched() to avoid similar style deadlocks w/
>> the scheduler.
>>
>> Thus this patchset (based on next/akpm) renames printk_sched to
>> printk_deferred and then moves the affected timekeeping printks to make
>> use of it.
>>
>> There were some points in the discussion between Jan and Peter that
>> made it seem that there may still be problems lurking in the console
>> layer, and I'm not sure I fully understand their point, so this solution
>> may be incomplete.
>>
>> Additionally, the same issue likely affects any WARN_ONs as well, but
>> I wanted to get some thoughts on this approach before trying to remove
>> or convert affected WARN_ONS.
>>
>> Your thoughts and feedback are greatly appreciated!
> All look pretty simple and sane to me. printk is a crazy hotspot
> lately but this patchset looks like it won't get singed.
>
> Would "printk_deferred_once" be more logical than
> "printk_once_deferred"? Think so. It's (((printk(deferred(once))),
> not (((printk(once(deferred))).
Sounds good. Will update the set with this.
thanks!
-john
^ permalink raw reply [flat|nested] 13+ messages in thread