On Tue, 11 Mar 2014, John Stultz wrote:
> On 03/06/2014 09:45 AM, Jiri Bohac wrote:
> > Hi,
> >
> > I'm looking at the printk call in
> > __timekeeping_inject_sleeptime(), introduced in cb5de2f8
> > (time: Catch invalid timespec sleep values in
> > __timekeeping_inject_sleeptime)
> >
> > Is it safe to call printk() while timekeeper_seq is held for
> > writing?
> >
> > What about this call chain?
> > 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, ...)
> >
> >
> > It looks like an unlikely but possible deadlock.
> > Or did I overlook something?
>
> So I don't think I've seen anything like the above in my testing, but it
> may just be very hard to get that path to trigger.
It's hard, but possible:
CPU0 CPU1
T1 down(&console_sem);
T2 down(&console_sem);
--> preemption or interrupt
write_seqcount_begin(&timekeeper_seq);
T1 up(&console_sem);
down(&console_sem);
....
up(&console_sem);
wakeup(T2);
....
hrtick_update();
> I was also surprised the seqlock lockdep enablement changes wouldn't
> catch this, but Jiri pointed out printk calls lockdep_off in
> vprintk_emit() - which makes sense as you don't want lockdep splats
> calling printk and recursing - but is frustrating to have that hole in
> the checking.
>
> There's a few spots where we do printks with the timekeeping seqlock
> held, and they're annoyingly nested far enough to make deferring the
> printk awkward. So I'm half thinking we could have some sort of buffer
> something like time_printk() could fill and then flush it after the lock
> is dropped. Then we just need something to warn if any new printks' are
> added to timekeeping seqlock sequences.
>
> The whole thing makes my head spin a bit, since even if we remove the
> explicit printks, I'm not sure where else printk might be triggered
> (like via lockdep warnings, for instance), where it might be unsafe.
>
> Peter/Thomas: Any thoughts on the deferred printk buffer? Does printk
> already have something like this? Any other ideas here?
I was thinking about something like that for RT as on RT printk is a
complete nightmare. It's simple to implement that, but as we know from
the RT experience it can lead to painful loss of debug output.
Assume you printk inside such a region, which just fills the dmesg
buffer and schedules the delayed output. Now in that same region you
run into a deadlock which causes the whole machine to freeze. Then you
won't see the debug output, which might actually give you the hint why
the system deadlocked ....
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
