linux-kernel - Re: [PATCH] workqueue: Guarantee work function memory ordering

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]

Message-ID: <20140222144010.GE12830@htj.dyndns.org>
Date:	Sat, 22 Feb 2014 09:40:10 -0500
From:	Tejun Heo <tj@...nel.org>
To:	Peter Hurley <peter@...leysoftware.com>
Cc:	Lai Jiangshan <laijs@...fujitsu.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] workqueue: Guarantee work function memory ordering

On Sat, Feb 22, 2014 at 07:11:51AM -0500, Peter Hurley wrote:
> Users of the workqueue api may assume the workqueue provides a
> memory ordering guarantee for re-queued work items; ie., that
> if a work item is not queue-able then the previously queued
> work instance is not running and so any memory operations
> which occur before queuing the work will be visible to the work
> function.
> 
> For example, code of the form:
>     add new data to work on
>     queue work
> assumes that this latest data is acted upon, either by the
> newly queued instance (if it could be queued) or by the not-yet-
> running instance (if a new instance could not be queued).
> 
> Provide this implicit memory ordering guarantee; prevent
> speculative loads in the work function from occurring before
> the current work instance is marked not pending (and thus has
> started). This, in turn, guarantees that stores occurring before
> schedule_work/queue_work are visible to either the not-yet-running
> work instance (if new work could not be queued) or that new work
> is queued (and thus ensuring the new data is acted upon).
> 
> Note that preventing early stores is unnecessary because no
> conclusion can be reached about the state of the work function
> from outside the work function by ordering early stores after
> clearing PENDING (other than testing PENDING).
> 
> Signed-off-by: Peter Hurley <peter@...leysoftware.com>
> ---
>  kernel/workqueue.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 82ef9f3..a4b241d 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -2201,6 +2201,16 @@ __acquires(&pool->lock)
>  
>  	spin_unlock_irq(&pool->lock);
>  
> +	/*
> +	 * Paired with the implied mb of test_and_set_bit(PENDING).
> +	 * Guarantees speculative loads which occur in the work item
> +	 * function do not complete before PENDING is cleared in
> +	 * set_work_pool_and_clear_pending() above. In turn, this
> +	 * ensures that stores are either visible to the not-yet-
> +	 * running work instance or a new instance is queueable.
> +	 */
> +	smp_rmb();

Wouldn't it make more sense to have the above right after
clear_pending?

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/