[PATCH] rt/workqueue: assign the waiters higher priority to the workqueue thread

[PATCH] rt/workqueue: assign the waiters higher priority to the workqueue thread

[Frederic Weisbecker]

When a work is embedeed inside the worklist of a barrier, it must check
the priority of more waiters than usual:

- the propagated higher priority of the barrier ancestors (the barrier in
  which our barrier parent is embedeed in)

- the priority of the task waiting for the completion of the barrier
  we are embedeed in

- the higher priority work which follows our barrier ancestor

But once this check is done, we forget to assign the higher priority
found and simply assign the priority of the current work to the current
workqueue thread, thus omitting the PI from all waiters.

This patch fixes it and also adds some comments, the current code is rather
complex.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/workqueue.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 90a54eb..e7ff441 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -325,17 +325,34 @@ again:
 		 */
 		struct lockdep_map lockdep_map = work->lockdep_map;
 #endif
+		/*
+		 * Iteration over each "waiter" priority to see which one
+		 * we have to inherit.
+		 */
 
+		/* Firstly: the current job */
 		prio = work->entry.prio;
+
+		/* In a barrier's worklist, we must handle yet other waiters */
 		if (unlikely(worklist != &cwq->worklist)) {
+			/*
+			 * If we are in a nested barrier, check the propagated
+			 * priority of the "grandfather" barrier waiting for us.
+			 */
 			prio = min(prio, cwq->barrier->prev_prio);
+
+			/* Check the task waiting for the parent barrier */
 			prio = min(prio, cwq->barrier->waiter_prio);
+
+			/* Check the higher priority work that follows the
+			 * ancestor barrier
+			 */
 			prio = min(prio, plist_first(&cwq->worklist)->prio);
 		}
 		prio = max(prio, 0);
 
 		if (likely(cwq->thread->prio != prio))
-			rt_mutex_setprio(cwq->thread, work->entry.prio);
+			rt_mutex_setprio(cwq->thread, prio);
 
 		cwq->current_work = work;
 		plist_del(&work->entry, worklist);
-- 
1.6.1

Re: [PATCH] rt/workqueue: assign the waiters higher priority to the workqueue thread

[Steven Rostedt]

Thanks Frederic, I'll take a look at this on Monday

-- Steve



On Sat, 21 Feb 2009, Frederic Weisbecker wrote:

> When a work is embedeed inside the worklist of a barrier, it must check
> the priority of more waiters than usual:
> 
> - the propagated higher priority of the barrier ancestors (the barrier in
>   which our barrier parent is embedeed in)
> 
> - the priority of the task waiting for the completion of the barrier
>   we are embedeed in
> 
> - the higher priority work which follows our barrier ancestor
> 
> But once this check is done, we forget to assign the higher priority
> found and simply assign the priority of the current work to the current
> workqueue thread, thus omitting the PI from all waiters.
> 
> This patch fixes it and also adds some comments, the current code is rather
> complex.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> ---
>  kernel/workqueue.c |   19 ++++++++++++++++++-
>  1 files changed, 18 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 90a54eb..e7ff441 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -325,17 +325,34 @@ again:
>  		 */
>  		struct lockdep_map lockdep_map = work->lockdep_map;
>  #endif
> +		/*
> +		 * Iteration over each "waiter" priority to see which one
> +		 * we have to inherit.
> +		 */
>  
> +		/* Firstly: the current job */
>  		prio = work->entry.prio;
> +
> +		/* In a barrier's worklist, we must handle yet other waiters */
>  		if (unlikely(worklist != &cwq->worklist)) {
> +			/*
> +			 * If we are in a nested barrier, check the propagated
> +			 * priority of the "grandfather" barrier waiting for us.
> +			 */
>  			prio = min(prio, cwq->barrier->prev_prio);
> +
> +			/* Check the task waiting for the parent barrier */
>  			prio = min(prio, cwq->barrier->waiter_prio);
> +
> +			/* Check the higher priority work that follows the
> +			 * ancestor barrier
> +			 */
>  			prio = min(prio, plist_first(&cwq->worklist)->prio);
>  		}
>  		prio = max(prio, 0);
>  
>  		if (likely(cwq->thread->prio != prio))
> -			rt_mutex_setprio(cwq->thread, work->entry.prio);
> +			rt_mutex_setprio(cwq->thread, prio);
>  
>  		cwq->current_work = work;
>  		plist_del(&work->entry, worklist);
> -- 
> 1.6.1
> 
> 
> 
>