There is no point to keep the task ref across the check for lock
owner. Drop the ref before that, so the protection context is clear.
Found while documenting the chain walk.
Signed-off-by: Thomas Gleixner <[email protected]>
---
kernel/locking/rtmutex.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Index: tip/kernel/locking/rtmutex.c
===================================================================
--- tip.orig/kernel/locking/rtmutex.c
+++ tip/kernel/locking/rtmutex.c
@@ -411,6 +411,8 @@ static int rt_mutex_adjust_prio_chain(st
/* Release the task */
raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+ put_task_struct(task);
+
if (!rt_mutex_owner(lock)) {
/*
* If the requeue above changed the top waiter, then we need
@@ -420,9 +422,8 @@ static int rt_mutex_adjust_prio_chain(st
if (top_waiter != rt_mutex_top_waiter(lock))
wake_up_process(rt_mutex_top_waiter(lock)->task);
raw_spin_unlock(&lock->wait_lock);
- goto out_put_task;
+ return 0;
}
- put_task_struct(task);
/* Grab the next task */
task = rt_mutex_owner(lock);
On Mon, 09 Jun 2014 20:28:06 -0000
Thomas Gleixner <[email protected]> wrote:
> There is no point to keep the task ref across the check for lock
> owner. Drop the ref before that, so the protection context is clear.
>
> Found while documenting the chain walk.
This looks fine, I just hate the subject. I don't see how it is
'deobfuscating" the chain walk. How about:
rtmutex: No need to keep task ref when checking lock ownership
Reviewed-by: Steven Rostedt <[email protected]>
-- Steve
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> kernel/locking/rtmutex.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> Index: tip/kernel/locking/rtmutex.c
> ===================================================================
> --- tip.orig/kernel/locking/rtmutex.c
> +++ tip/kernel/locking/rtmutex.c
> @@ -411,6 +411,8 @@ static int rt_mutex_adjust_prio_chain(st
>
> /* Release the task */
> raw_spin_unlock_irqrestore(&task->pi_lock, flags);
> + put_task_struct(task);
> +
> if (!rt_mutex_owner(lock)) {
> /*
> * If the requeue above changed the top waiter, then we need
> @@ -420,9 +422,8 @@ static int rt_mutex_adjust_prio_chain(st
> if (top_waiter != rt_mutex_top_waiter(lock))
> wake_up_process(rt_mutex_top_waiter(lock)->task);
> raw_spin_unlock(&lock->wait_lock);
> - goto out_put_task;
> + return 0;
> }
> - put_task_struct(task);
>
> /* Grab the next task */
> task = rt_mutex_owner(lock);
>
On Mon, Jun 9, 2014 at 1:28 PM, Thomas Gleixner <[email protected]> wrote:
> There is no point to keep the task ref across the check for lock
> owner. Drop the ref before that, so the protection context is clear.
>
> Found while documenting the chain walk.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Jason Low <[email protected]>
On 06/10/2014 04:59 AM, Steven Rostedt wrote:
> On Mon, 09 Jun 2014 20:28:06 -0000
> Thomas Gleixner <[email protected]> wrote:
>
>> There is no point to keep the task ref across the check for lock
>> owner. Drop the ref before that, so the protection context is clear.
>>
>> Found while documenting the chain walk.
>
> This looks fine, I just hate the subject. I don't see how it is
> 'deobfuscating" the chain walk. How about:
>
> rtmutex: No need to keep task ref when checking lock ownership
>
> Reviewed-by: Steven Rostedt <[email protected]>
Reviewed-by: Lai Jiangshan <[email protected]>
Thanks,
Lai
On Mon, Jun 09, 2014 at 08:28:06PM -0000, Thomas Gleixner wrote:
> There is no point to keep the task ref across the check for lock
> owner. Drop the ref before that, so the protection context is clear.
>
> Found while documenting the chain walk.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
I also am not a huge fan of the verbiage of the commit message here.
Steven's makes more sense to me, and you basically say the same thing
in the actual commit message.
Reviewed-by: Brad Mouring <[email protected]>