Messages in this thread | | | Date | Tue, 14 Jun 2016 14:07:07 +0100 | From | Juri Lelli <> | Subject | Re: [RFC][PATCH 2/8] sched/rtmutex/deadline: Fix a PI crash for deadline tasks |
| |
On 14/06/16 20:53, Xunlei Pang wrote: > On 2016/06/14 at 18:21, Juri Lelli wrote: > > Hi, > > > > On 07/06/16 21:56, Peter Zijlstra wrote: > >> From: Xunlei Pang <xlpang@redhat.com> > >> > >> A crash happened while I was playing with deadline PI rtmutex. > >> > >> BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 > >> IP: [<ffffffff810eeb8f>] rt_mutex_get_top_task+0x1f/0x30 > >> PGD 232a75067 PUD 230947067 PMD 0 > >> Oops: 0000 [#1] SMP > >> CPU: 1 PID: 10994 Comm: a.out Not tainted > >> > >> Call Trace: > >> [<ffffffff810b658c>] enqueue_task+0x2c/0x80 > >> [<ffffffff810ba763>] activate_task+0x23/0x30 > >> [<ffffffff810d0ab5>] pull_dl_task+0x1d5/0x260 > >> [<ffffffff810d0be6>] pre_schedule_dl+0x16/0x20 > >> [<ffffffff8164e783>] __schedule+0xd3/0x900 > >> [<ffffffff8164efd9>] schedule+0x29/0x70 > >> [<ffffffff8165035b>] __rt_mutex_slowlock+0x4b/0xc0 > >> [<ffffffff81650501>] rt_mutex_slowlock+0xd1/0x190 > >> [<ffffffff810eeb33>] rt_mutex_timed_lock+0x53/0x60 > >> [<ffffffff810ecbfc>] futex_lock_pi.isra.18+0x28c/0x390 > >> [<ffffffff810ed8b0>] do_futex+0x190/0x5b0 > >> [<ffffffff810edd50>] SyS_futex+0x80/0x180 > >> > > This seems to be caused by the race condition you detail below between > > load balancing and PI code. I tried to reproduce the BUG on my box, but > > it looks hard to get. Do you have a reproducer I can give a try? > > > >> This is because rt_mutex_enqueue_pi() and rt_mutex_dequeue_pi() > >> are only protected by pi_lock when operating pi waiters, while > >> rt_mutex_get_top_task(), will access them with rq lock held but > >> not holding pi_lock. > >> > >> In order to tackle it, we introduce new "pi_top_task" pointer > >> cached in task_struct, and add new rt_mutex_update_top_task() > >> to update its value, it can be called by rt_mutex_setprio() > >> which held both owner's pi_lock and rq lock. Thus "pi_top_task" > >> can be safely accessed by enqueue_task_dl() under rq lock. > >> > >> [XXX this next section is unparsable] > > Yes, a bit hard to understand. However, am I correct in assuming this > > patch and the previous one should fix this problem? Or are there still > > other races causing issues? > > Yes, these two patches can fix the problem. > > > > >> One problem is when rt_mutex_adjust_prio()->...->rt_mutex_setprio(), > >> at that time rtmutex lock was released and owner was marked off, > >> this can cause "pi_top_task" dereferenced to be a running one(as it > >> can be falsely woken up by others before rt_mutex_setprio() is > >> made to update "pi_top_task"). We solve this by directly calling > >> __rt_mutex_adjust_prio() in mark_wakeup_next_waiter() which held > >> pi_lock and rtmutex lock, and remove rt_mutex_adjust_prio(). Since > >> now we moved the deboost point, in order to avoid current to be > >> preempted due to deboost earlier before wake_up_q(), we also moved > >> preempt_disable() before unlocking rtmutex. > >> > >> Cc: Steven Rostedt <rostedt@goodmis.org> > >> Cc: Ingo Molnar <mingo@redhat.com> > >> Cc: Juri Lelli <juri.lelli@arm.com> > >> Originally-From: Peter Zijlstra <peterz@infradead.org> > >> Signed-off-by: Xunlei Pang <xlpang@redhat.com> > >> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > >> Link: http://lkml.kernel.org/r/1461659449-19497-2-git-send-email-xlpang@redhat.com > > The idea of this fix makes sense to me. But, I would like to be able to > > see the BUG and test the fix. What I have is a test in which I create N > > DEADLINE workers that share a PI mutex. They get migrated around and > > seem to stress PI code. But I couldn't hit the BUG yet. Maybe I let it > > run for some more time. > > You can use this reproducer attached(gcc crash_deadline_pi.c -lpthread -lrt ). > Start multiple instances, then it will hit the bug very soon. >
Great, thanks! I'll use it.
Best,
- Juri
| |