Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1381370 > unrolled thread
| Started by | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| First post | 2016-04-18 10:30 +0200 |
| Last post | 2016-04-20 15:20 +0200 |
| Articles | 7 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter Thomas Gleixner <tglx@linutronix.de> - 2016-04-18 10:30 +0200
Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter Xunlei Pang <xpang@redhat.com> - 2016-04-18 10:50 +0200
Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter Thomas Gleixner <tglx@linutronix.de> - 2016-04-18 11:10 +0200
Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter Xunlei Pang <xpang@redhat.com> - 2016-04-18 11:50 +0200
Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter Peter Zijlstra <peterz@infradead.org> - 2016-04-20 14:30 +0200
Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter Thomas Gleixner <tglx@linutronix.de> - 2016-04-20 14:50 +0200
Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter Peter Zijlstra <peterz@infradead.org> - 2016-04-20 15:20 +0200
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2016-04-18 10:30 +0200 |
| Subject | Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter |
| Message-ID | <rpcJc-2fT-17@gated-at.bofh.it> |
On Thu, 14 Apr 2016, Xunlei Pang wrote: > We should deboost before waking the high-prio task such that > we don't run two tasks with the 'same' priority. No. This is fundamentaly broken. T1 (prio 0) lock(X) --> preemption T2 (prio 10) lock(X) boost(T1) schedule() T1 (prio 10) unlock(X) deboost() (prio 0) --> preemption T3 (prio 5) .... Classic priority inversion enabled by a mechanism to avoid it. Brilliant stuff. Thanks, tglx
[toc] | [next] | [standalone]
| From | Xunlei Pang <xpang@redhat.com> |
|---|---|
| Date | 2016-04-18 10:50 +0200 |
| Subject | Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter |
| Message-ID | <rpd2x-2oe-11@gated-at.bofh.it> |
| In reply to | #1381370 |
On 2016/04/18 at 16:23, Thomas Gleixner wrote: > On Thu, 14 Apr 2016, Xunlei Pang wrote: >> We should deboost before waking the high-prio task such that >> we don't run two tasks with the 'same' priority. > No. This is fundamentaly broken. > > T1 (prio 0) lock(X) > > --> preemption > > T2 (prio 10) lock(X) > boost(T1) > schedule() > > T1 (prio 10) unlock(X) We add a preempt_disable() before deboost to avoid the breakage, there's also some comment about this in the patch's code. Regards, Xunlei > deboost() > (prio 0) > > --> preemption > > T3 (prio 5) .... > > Classic priority inversion enabled by a mechanism to avoid it. Brilliant > stuff. > > Thanks, > > tglx
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2016-04-18 11:10 +0200 |
| Message-ID | <rpdlU-2Qd-21@gated-at.bofh.it> |
| In reply to | #1381381 |
On Mon, 18 Apr 2016, Xunlei Pang wrote: > On 2016/04/18 at 16:23, Thomas Gleixner wrote: > > On Thu, 14 Apr 2016, Xunlei Pang wrote: > >> We should deboost before waking the high-prio task such that > >> we don't run two tasks with the 'same' priority. > > No. This is fundamentaly broken. > > > > T1 (prio 0) lock(X) > > > > --> preemption > > > > T2 (prio 10) lock(X) > > boost(T1) > > schedule() > > > > T1 (prio 10) unlock(X) > > We add a preempt_disable() before deboost to avoid the breakage, > there's also some comment about this in the patch's code. So the changelog is useless and misleading. Neither does it explain what's wrong with having two tasks with the same priority in running state. Thanks, tglx
[toc] | [prev] | [next] | [standalone]
| From | Xunlei Pang <xpang@redhat.com> |
|---|---|
| Date | 2016-04-18 11:50 +0200 |
| Subject | Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter |
| Message-ID | <rpdYC-3eO-9@gated-at.bofh.it> |
| In reply to | #1381401 |
On 2016/04/18 at 17:02, Thomas Gleixner wrote: > On Mon, 18 Apr 2016, Xunlei Pang wrote: >> On 2016/04/18 at 16:23, Thomas Gleixner wrote: >>> On Thu, 14 Apr 2016, Xunlei Pang wrote: >>>> We should deboost before waking the high-prio task such that >>>> we don't run two tasks with the 'same' priority. >>> No. This is fundamentaly broken. >>> >>> T1 (prio 0) lock(X) >>> >>> --> preemption >>> >>> T2 (prio 10) lock(X) >>> boost(T1) >>> schedule() >>> >>> T1 (prio 10) unlock(X) >> We add a preempt_disable() before deboost to avoid the breakage, >> there's also some comment about this in the patch's code. > So the changelog is useless and misleading. Neither does it explain what's > wrong with having two tasks with the same priority in running state. Sorry about that, will improve it. Regards, Xunlei > > Thanks, > > tglx > > > >
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-04-20 14:30 +0200 |
| Subject | Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter |
| Message-ID | <rpZqx-7Qn-15@gated-at.bofh.it> |
| In reply to | #1381401 |
On Mon, Apr 18, 2016 at 11:02:28AM +0200, Thomas Gleixner wrote: > On Mon, 18 Apr 2016, Xunlei Pang wrote: > > On 2016/04/18 at 16:23, Thomas Gleixner wrote: > > > On Thu, 14 Apr 2016, Xunlei Pang wrote: > > >> We should deboost before waking the high-prio task such that > > >> we don't run two tasks with the 'same' priority. > > > No. This is fundamentaly broken. > > > > > > T1 (prio 0) lock(X) > > > > > > --> preemption > > > > > > T2 (prio 10) lock(X) > > > boost(T1) > > > schedule() > > > > > > T1 (prio 10) unlock(X) > > > > We add a preempt_disable() before deboost to avoid the breakage, > > there's also some comment about this in the patch's code. > > So the changelog is useless and misleading. Neither does it explain what's > wrong with having two tasks with the same priority in running state. So its semantically icky to have the two tasks running off the same state and practically icky when you consider bandwidth inheritance -- where the boosted task wants to explicitly modify the state of the booster. In that latter case you really want to unboost before you let the booster run again. However, you noted we need to deal with this case due to the whole optimistic spinning crap anyway :/
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2016-04-20 14:50 +0200 |
| Message-ID | <rpZJV-7YC-39@gated-at.bofh.it> |
| In reply to | #1383334 |
On Wed, 20 Apr 2016, Peter Zijlstra wrote: > On Mon, Apr 18, 2016 at 11:02:28AM +0200, Thomas Gleixner wrote: > > On Mon, 18 Apr 2016, Xunlei Pang wrote: > > > We add a preempt_disable() before deboost to avoid the breakage, > > > there's also some comment about this in the patch's code. > > > > So the changelog is useless and misleading. Neither does it explain what's > > wrong with having two tasks with the same priority in running state. > > So its semantically icky to have the two tasks running off the same > state and practically icky when you consider bandwidth inheritance -- > where the boosted task wants to explicitly modify the state of the > booster. > > In that latter case you really want to unboost before you let the > booster run again. I understand that. That doesn't make the changelog any better, which mumbles about priorities :( > However, you noted we need to deal with this case due to the whole > optimistic spinning crap anyway :/ Right, but that's another dimension of madness. Both tasks are on a cpu. The reason why we boost the lock holder before spinning is to make sure that it does not get preempted by something of medium priority before dropping the lock. That really gets interesting with bandwith inheritance .... Thanks, tglx
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-04-20 15:20 +0200 |
| Subject | Re: [PATCH v3 1/6] rtmutex: Deboost before waking up the top waiter |
| Message-ID | <rq0cW-8rY-23@gated-at.bofh.it> |
| In reply to | #1383350 |
On Wed, Apr 20, 2016 at 02:43:29PM +0200, Thomas Gleixner wrote: > > So its semantically icky to have the two tasks running off the same > > state and practically icky when you consider bandwidth inheritance -- > > where the boosted task wants to explicitly modify the state of the > > booster. > > > > In that latter case you really want to unboost before you let the > > booster run again. > > I understand that. That doesn't make the changelog any better, which mumbles > about priorities :( Agreed. > > However, you noted we need to deal with this case due to the whole > > optimistic spinning crap anyway :/ > > Right, but that's another dimension of madness. Both tasks are on a cpu. > The reason why we boost the lock holder before spinning is to make > sure that it does not get preempted by something of medium priority > before dropping the lock. Right; I figured that out pretty quickly, which is why this patch does a preempt_disable() over the unboost+wakeup. FWIW, the immediate reason for this patch is that is ensures the new p->pi_task pointer, points to something that exists. > That really gets interesting with bandwith inheritance .... I'm more worried about the optimistic spinning case..
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web