Re: [PATCH 04/12] workqueue: simplify is_chained_work()

[Tejun Heo <tj@kernel.org>]

On Thu, Sep 27, 2012 at 01:20:35AM +0800, Lai Jiangshan wrote:
> is_chained_work() is too complicated. we can simply found out
> whether current task is worker by PF_WQ_WORKER or wq->rescuer.
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
>  kernel/workqueue.c |   36 ++++++++++++------------------------
>  1 files changed, 12 insertions(+), 24 deletions(-)
> 
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index e41c562..c718b94 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1182,34 +1182,22 @@ static void insert_work(struct cpu_workqueue_struct *cwq,
>  
>  /*
>   * Test whether @work is being queued from another work executing on the
> - * same workqueue.  This is rather expensive and should only be used from
> - * cold paths.
> + * same workqueue.
>   */
>  static bool is_chained_work(struct workqueue_struct *wq)
>  {
> -	unsigned long flags;
> -	unsigned int cpu;
> +	struct worker *worker = NULL;
>  
> -	for_each_gcwq_cpu(cpu) {
> -		struct global_cwq *gcwq = get_gcwq(cpu);
> -		struct worker *worker;
> -		struct hlist_node *pos;
> -		int i;
> +	if (wq->rescuer && current == wq->rescuer->task) /* rescuer_thread() */
> +		worker = wq->rescuer;
> +	else if (current->flags & PF_WQ_WORKER) /* worker_thread() */
> +		worker = kthread_data(current);
>  
> -		spin_lock_irqsave(&gcwq->lock, flags);
> -		for_each_busy_worker(worker, i, pos, gcwq) {
> -			if (worker->task != current)
> -				continue;
> -			spin_unlock_irqrestore(&gcwq->lock, flags);
> -			/*
> -			 * I'm @worker, no locking necessary.  See if @work
> -			 * is headed to the same workqueue.
> -			 */
> -			return worker->current_cwq->wq == wq;
> -		}
> -		spin_unlock_irqrestore(&gcwq->lock, flags);
> -	}
> -	return false;
> +	/*
> +	 * I'm @worker, no locking necessary.  See if @work
> +	 * is headed to the same workqueue.
> +	 */
> +	return worker && worker->current_cwq->wq == wq;

How about,

	if (wq->rescuer && current == wq->rescuer->task)
		worker = wq->rescuer;
	else if (current->flags & PF_WQ_WORKER)
		worker = kthread_data(current);
	else
		return NULL;

	return worker->curent_cwq->wq == wq;

Thanks.

-- 
tejun