From: Juri Lelli <juri.lelli@arm.com>
To: xlpang@redhat.com
Cc: Peter Zijlstra <peterz@infradead.org>,
mingo@kernel.org, tglx@linutronix.de, rostedt@goodmis.org,
linux-kernel@vger.kernel.org, mathieu.desnoyers@efficios.com,
jdesfossez@efficios.com, bristot@redhat.com,
Ingo Molnar <mingo@redhat.com>
Subject: Re: [RFC][PATCH 2/8] sched/rtmutex/deadline: Fix a PI crash for deadline tasks
Date: Tue, 14 Jun 2016 14:07:07 +0100 [thread overview]
Message-ID: <20160614130707.GJ5981@e106622-lin> (raw)
In-Reply-To: <575FFE58.1090102@redhat.com>
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
next prev parent reply other threads:[~2016-06-14 13:07 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-07 19:56 [RFC][PATCH 0/8] PI and assorted failings Peter Zijlstra
2016-06-07 19:56 ` [RFC][PATCH 1/8] rtmutex: Deboost before waking up the top waiter Peter Zijlstra
2016-06-14 9:09 ` Juri Lelli
2016-06-14 12:54 ` Peter Zijlstra
2016-06-14 13:20 ` Juri Lelli
2016-06-14 13:59 ` Peter Zijlstra
2016-06-14 16:36 ` Davidlohr Bueso
2016-06-14 17:01 ` Juri Lelli
2016-06-14 18:22 ` Steven Rostedt
2016-06-07 19:56 ` [RFC][PATCH 2/8] sched/rtmutex/deadline: Fix a PI crash for deadline tasks Peter Zijlstra
2016-06-14 10:21 ` Juri Lelli
2016-06-14 12:30 ` Peter Zijlstra
2016-06-14 12:53 ` Xunlei Pang
2016-06-14 13:07 ` Juri Lelli [this message]
2016-06-14 16:39 ` Juri Lelli
2016-06-14 18:42 ` Steven Rostedt
2016-06-14 20:28 ` Peter Zijlstra
2016-06-15 16:14 ` Steven Rostedt
2016-06-07 19:56 ` [RFC][PATCH 3/8] sched/deadline/rtmutex: Dont miss the dl_runtime/dl_period update Peter Zijlstra
2016-06-14 10:43 ` Juri Lelli
2016-06-14 12:09 ` Peter Zijlstra
2016-06-15 16:30 ` Steven Rostedt
2016-06-15 17:55 ` Peter Zijlstra
2016-06-07 19:56 ` [RFC][PATCH 4/8] rtmutex: Remove rt_mutex_fastunlock() Peter Zijlstra
2016-06-15 16:43 ` Steven Rostedt
2016-06-07 19:56 ` [RFC][PATCH 5/8] rtmutex: Clean up Peter Zijlstra
2016-06-14 12:08 ` Juri Lelli
2016-06-14 12:32 ` Peter Zijlstra
2016-06-14 12:41 ` Juri Lelli
2016-06-07 19:56 ` [RFC][PATCH 6/8] sched/rtmutex: Refactor rt_mutex_setprio() Peter Zijlstra
2016-06-14 13:14 ` Juri Lelli
2016-06-14 14:08 ` Peter Zijlstra
2016-06-07 19:56 ` [RFC][PATCH 7/8] sched,tracing: Update trace_sched_pi_setprio() Peter Zijlstra
2016-06-07 19:56 ` [RFC][PATCH 8/8] rtmutex: Fix PI chain order integrity Peter Zijlstra
2016-06-14 17:39 ` Juri Lelli
2016-06-14 19:44 ` Peter Zijlstra
2016-06-15 7:25 ` Juri Lelli
2016-06-27 12:23 ` Peter Zijlstra
2016-06-27 12:40 ` Thomas Gleixner
2016-06-28 9:05 ` Juri Lelli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160614130707.GJ5981@e106622-lin \
--to=juri.lelli@arm.com \
--cc=bristot@redhat.com \
--cc=jdesfossez@efficios.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=xlpang@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).