linux-kernel - Re: [PATCH 04/12] workqueue: simplify is_chained

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 26 Sep 2012 11:28:37 -0700
From:	Tejun Heo <tj@...nel.org>
To:	Lai Jiangshan <laijs@...fujitsu.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/12] workqueue: simplify is_chained_work()

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@...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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/