Messages in this thread | | | From | Jann Horn <> | Date | Thu, 5 Mar 2020 23:30:13 +0100 | Subject | Re: [PATCH v2] exit: Move preemption fixup up, move blocking operations down |
| |
On Thu, Mar 5, 2020 at 11:13 PM Steven Rostedt <rostedt@goodmis.org> wrote: > On Thu, 5 Mar 2020 23:06:57 +0100 > Jann Horn <jannh@google.com> wrote: > > > With CONFIG_DEBUG_ATOMIC_SLEEP=y and CONFIG_CGROUPS=y, kernel oopses in > > non-preemptible context look untidy; after the main oops, the kernel prints > > a "sleeping function called from invalid context" report because > > exit_signals() -> cgroup_threadgroup_change_begin() -> percpu_down_read() > > can sleep, and that happens before the preempt_count_set(PREEMPT_ENABLED) > > fixup. > > > > It looks like the same thing applies to profile_task_exit() and > > kcov_task_exit(). > > > > Fix it by moving the preemption fixup up and the calls to > > profile_task_exit() and kcov_task_exit() down. [...] > > + if (unlikely(in_atomic())) { > > + pr_info("note: %s[%d] exited with preempt_count %d\n", > > + current->comm, task_pid_nr(current), > > + preempt_count()); > > This should be more than a pr_info. It should also probably state the > "Dazed and confused, best to reboot" message. > > Because if something crashed in a non preempt section, it may likely be > holding a lock that it will never release, causing a soon to be deadlock!
I didn't write that code, I'm just moving it around. :P But I guess if you want, I can change it in the same patch... something like this on top? Does that look reasonable?
if (unlikely(in_atomic())) { - pr_info("note: %s[%d] exited with preempt_count %d\n", + pr_emerg("note: %s[%d] exited with preempt_count %d, system might deadlock, please reboot\n", current->comm, task_pid_nr(current), preempt_count()); preempt_count_set(PREEMPT_ENABLED);
| |