diff mbox series

[3/3] rtmutex: remove unnecessary adjust prio

Message ID 1499395922-542-3-git-send-email-alex.shi@linaro.org
State Superseded
Headers show
Series None | expand

Commit Message

Alex Shi July 7, 2017, 2:52 a.m. UTC
We don't need to adjust prio before new pi_waiter adding. The prio
only need update after pi_waiter change or task priority change.

Signed-off-by: Alex Shi <alex.shi@linaro.org>

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Juri Lelli <juri.lelli@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
To: linux-kernel@vger.kernel.org
To: Ingo Molnar <mingo@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/rtmutex.c | 1 -
 1 file changed, 1 deletion(-)

-- 
2.7.4

Comments

Alex Shi July 11, 2017, 2:39 p.m. UTC | #1
Any comments for this little change? It's passed on 0day testing.

Thanks
Alex

On 07/07/2017 10:52 AM, Alex Shi wrote:
> We don't need to adjust prio before new pi_waiter adding. The prio

> only need update after pi_waiter change or task priority change.

> 

> Signed-off-by: Alex Shi <alex.shi@linaro.org>

> Cc: Steven Rostedt <rostedt@goodmis.org>

> Cc: Sebastian Siewior <bigeasy@linutronix.de>

> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>

> Cc: Juri Lelli <juri.lelli@arm.com>

> Cc: Thomas Gleixner <tglx@linutronix.de>

> To: linux-kernel@vger.kernel.org

> To: Ingo Molnar <mingo@redhat.com>

> To: Peter Zijlstra <peterz@infradead.org>

> ---

>  kernel/locking/rtmutex.c | 1 -

>  1 file changed, 1 deletion(-)

> 

> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c

> index 28cd09e..d1fe41f 100644

> --- a/kernel/locking/rtmutex.c

> +++ b/kernel/locking/rtmutex.c

> @@ -963,7 +963,6 @@ static int task_blocks_on_rt_mutex(struct rt_mutex *lock,

>  		return -EDEADLK;

>  

>  	raw_spin_lock(&task->pi_lock);

> -	rt_mutex_adjust_prio(task);

>  	waiter->task = task;

>  	waiter->lock = lock;

>  	waiter->prio = task->prio;

>
Steven Rostedt July 12, 2017, 2:14 p.m. UTC | #2
On Tue, 11 Jul 2017 22:39:24 +0800
Alex Shi <alex.shi@linaro.org> wrote:

> Any comments for this little change? It's passed on 0day testing.


I think the problem was that this was a third patch after two
documentation patches. Where, people put documentation review at the
bottom of their priority list.

This should have been sent as separate patch on its own.

> 

> Thanks

> Alex

> 

> On 07/07/2017 10:52 AM, Alex Shi wrote:

> > We don't need to adjust prio before new pi_waiter adding. The prio

> > only need update after pi_waiter change or task priority change.

> > 

> > Signed-off-by: Alex Shi <alex.shi@linaro.org>

> > Cc: Steven Rostedt <rostedt@goodmis.org>

> > Cc: Sebastian Siewior <bigeasy@linutronix.de>

> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>

> > Cc: Juri Lelli <juri.lelli@arm.com>

> > Cc: Thomas Gleixner <tglx@linutronix.de>

> > To: linux-kernel@vger.kernel.org

> > To: Ingo Molnar <mingo@redhat.com>

> > To: Peter Zijlstra <peterz@infradead.org>

> > ---

> >  kernel/locking/rtmutex.c | 1 -

> >  1 file changed, 1 deletion(-)

> > 

> > diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c

> > index 28cd09e..d1fe41f 100644

> > --- a/kernel/locking/rtmutex.c

> > +++ b/kernel/locking/rtmutex.c

> > @@ -963,7 +963,6 @@ static int task_blocks_on_rt_mutex(struct rt_mutex *lock,

> >  		return -EDEADLK;

> >  

> >  	raw_spin_lock(&task->pi_lock);

> > -	rt_mutex_adjust_prio(task);


Interesting, I did some git mining and this was added with the original
entry of the rtmutex.c (23f78d4a0). Looking at even that version, I
don't see the purpose of adjusting the task prio here. It is done
before anything changes in the task.

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>


-- Steve


> >  	waiter->task = task;

> >  	waiter->lock = lock;

> >  	waiter->prio = task->prio;

> >
Peter Zijlstra July 12, 2017, 4:35 p.m. UTC | #3
On Wed, Jul 12, 2017 at 10:14:49AM -0400, Steven Rostedt wrote:
> On Tue, 11 Jul 2017 22:39:24 +0800

> Alex Shi <alex.shi@linaro.org> wrote:

> 

> > Any comments for this little change? It's passed on 0day testing.

> 

> I think the problem was that this was a third patch after two

> documentation patches. Where, people put documentation review at the

> bottom of their priority list.

> 

> This should have been sent as separate patch on its own.


My problem was the sparse changelog, which forces me to think hard and
thus is landed on the 'later' queue, which moves at glacial speeds.
Alex Shi July 13, 2017, 6:12 a.m. UTC | #4
On 07/12/2017 10:14 PM, Steven Rostedt wrote:
> On Tue, 11 Jul 2017 22:39:24 +0800

> Alex Shi <alex.shi@linaro.org> wrote:

> 

>> Any comments for this little change? It's passed on 0day testing.

> 

> I think the problem was that this was a third patch after two

> documentation patches. Where, people put documentation review at the

> bottom of their priority list.

> 

> This should have been sent as separate patch on its own.

> 


Got it. I will resend it.

<snip>...

> 

> Interesting, I did some git mining and this was added with the original

> entry of the rtmutex.c (23f78d4a0). Looking at even that version, I

> don't see the purpose of adjusting the task prio here. It is done

> before anything changes in the task.

> 

> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>


Thanks Steven!

Regards
Alex
Alex Shi July 13, 2017, 6:13 a.m. UTC | #5
On 07/13/2017 12:35 AM, Peter Zijlstra wrote:
> On Wed, Jul 12, 2017 at 10:14:49AM -0400, Steven Rostedt wrote:

>> On Tue, 11 Jul 2017 22:39:24 +0800

>> Alex Shi <alex.shi@linaro.org> wrote:

>>

>>> Any comments for this little change? It's passed on 0day testing.

>>

>> I think the problem was that this was a third patch after two

>> documentation patches. Where, people put documentation review at the

>> bottom of their priority list.

>>

>> This should have been sent as separate patch on its own.

> 

> My problem was the sparse changelog, which forces me to think hard and

> thus is landed on the 'later' queue, which moves at glacial speeds.

> 


Yes, I should mentioned I didn't find out any reasons from the history
changelogs.

Thanks for reminder this! :)

Regards
Alex
diff mbox series

Patch

diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 28cd09e..d1fe41f 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -963,7 +963,6 @@  static int task_blocks_on_rt_mutex(struct rt_mutex *lock,
 		return -EDEADLK;
 
 	raw_spin_lock(&task->pi_lock);
-	rt_mutex_adjust_prio(task);
 	waiter->task = task;
 	waiter->lock = lock;
 	waiter->prio = task->prio;