Messages in this thread | | | Date | Thu, 5 Mar 2020 17:44:23 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH v2] exit: Move preemption fixup up, move blocking operations down |
| |
On Thu, 5 Mar 2020 23:30:13 +0100 Jann Horn <jannh@google.com> wrote:
> 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
Ah, I didn't scroll down enough to see it was just moved.
> you want, I can change it in the same patch... something like this on > top? Does that look reasonable?
No, an update to the text should be done as a separate patch, as it is a different type of change.
Thanks,
-- Steve
> > 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);
| |