doc: completion: context, scope and language fixes

PATCH In review Nicholas Mc Guire 5 participants 12 messages Feb 20, 2015 – Mar 29, 2015 Documentation

Share & tools

Nicholas Mc Guire

author

2015-02-20 17:33 UTC #0 lore raw

PATCH 1/1 doc: completion: context, scope and language fixes

Fix for imprecise/wrong statements on context in which wait_for_completion*() can be called, updated notes on "going out of scope" problems and some language fixups.

Signed-off-by Nicholas Mc Guire <[email protected]>

---

This hopfully address all of the issues Ingo Molnar <[email protected]> noted in https://lkml.org/lkml/2015/2/18/690.

Patch is against 3.19.0 (linux-stable)

completions - wait for completion handling
==========================================

This document was originally written based on 3.18.0 (linux-next)

Introduction:
-------------

If you have one or more threads of execution that must wait for some process to have reached a point or a specific state, completions can provide a race free solution to this problem. Semantically they are somewhat like a pthread_barriers and have similar use-cases.

Completions are a code synchronization mechanism which are preferable to any misuse of locks. Any time you think of using yield() or some quirky msleep(1); loop to allow something else to proceed, you probably want to look into using one of the wait_for_completion*() calls instead. The advantage of using completions is clear intent of the code, but also more efficient code as both threads can continue until the result is actually needed.

Completions are built on top of the generic event infrastructure in Linux, with the event reduced to a simple flag appropriately called "done" in struct completion, that tells the waiting threads of execution if they can continue safely.

As completions are scheduling related, the code is found in kernel/sched/completion.c - for details on completion design and implementation see completions-design.txt

Usage:
------

There are three parts to using completions, the initialization of the struct completion, the waiting part through a call to one of the variants of wait_for_completion() and the signaling side through a call to complete() or complete_all(). Further there are some helper functions for checking the state of completions.

To use completions one needs to include <linux/completion.h> and create a variable of type struct completion. The structure used for handling of completions is:

	struct completion {
		unsigned int done;
		wait_queue_head_t wait;
	};

providing the wait queue to place tasks on for waiting and the flag for indicating the state of affairs.

Completions should be named to convey the intent of the waiter. A good example is:

	wait_for_completion(&early_console_added);

	complete(&early_console_added);

Good naming (as always) helps code readability.

Initializing completions:
-------------------------

Initialization of dynamically allocated completions, often embedded in other structures, is done with:

	void init_completion(&done);

Initialization is accomplished by initializing the wait queue and setting the default state to "not available", that is, "done" is set to 0.

The re-initialization function, reinit_completion(), simply resets the done element to "not available", thus again to 0, without touching the wait queue. Calling init_completion() on the same completion object is most likely a bug as it re-initializes the queue to an empty queue and enqueued tasks could get "lost" - use reinit_completion() in that case.

For static declaration and initialization, macros are available. These are:

static DECLARE_COMPLETION(setup_done)

used for static declarations in file scope. Within functions the static initialization should always use:

DECLARE_COMPLETION_ONSTACK(setup_done)

suitable for automatic/local variables on the stack and will make lockdep happy. Note also that one needs to make *sure* the completion passed to work threads remains in-scope, and no references remain to on-stack data when the initiating function returns.

Using on-stack completions for code that calls any of the _timeout or _interruptible/_killable variants is not advisable as they will require additional synchronization to prevent the on-stack completion object in the timeout/signal cases from going out of scope. Consider using dynamically allocated completions when intending to use the _interruptible/_killable or _timeout variants of wait_for_completion().

Waiting for completions:
------------------------

For a thread of execution to wait for some concurrent work to finish, it calls wait_for_completion() on the initialized completion structure. A typical usage scenario is:

	structure completion setup_done;
	init_completion(&setup_done);
	initialize_work(...,&setup_done,...)

	/* run non-dependent code */              /* do setup */

	wait_for_completion(&setup_done);         complete(setup_done)

This is not implying any temporal order on wait_for_completion() and the call to complete() - if the call to complete() happened before the call to wait_for_completion() then the waiting side simply will continue immediately as all dependencies are satisfied if not it will block until completion is signaled by complete().

Note that wait_for_completion() is calling spin_lock_irq/spin_unlock_irq so it can only be called safely when you know that interrupts are enabled. Calling it from hard-irq or irqs-off atomic contexts will result in hard to detect spurious enabling of interrupts.

wait_for_completion():

void wait_for_completion(struct completion *done):

The default behavior is to wait without a timeout and mark the task as uninterruptible. wait_for_completion() and its variants are only safe in process context (as they can sleep) but not in atomic context, interrupt context, with disabled irqs. or preemption is disabled - see also try_wait_for_completion() below for handling completion in atomic/interrupt context.

As all variants of wait_for_completion() can (obviously) block for a long time, you probably don't want to call this with held mutexes.

Variants available:
-------------------

The below variants all return status and this status should be checked in most(/all) cases - in cases where the status is deliberately not checked you probably want to make a note explaining this (e.g. see arch/arm/kernel/smp.c:__cpu_up()).

A common problem that occurs is to have unclean assignment of return types, so care should be taken with assigning return-values to variables of proper type. Checking for the specific meaning of return values also has been found to be quite inaccurate e.g. constructs like
if (!wait_for_completion_interruptible_timeout(...)) would execute the same code path for successful completion and for the interrupted case - which is probably not what you want.

int wait_for_completion_interruptible(struct completion *done)

This function marks the task TASK_INTERRUPTIBLE. If a signal was received while waiting it will return -ERESTARTSYS and 0 otherwise.

unsigned long wait_for_completion_timeout(struct completion *done,
unsigned long timeout)

The task is marked as TASK_UNINTERRUPTIBLE and will wait at most 'timeout' (in jiffies). If timeout occurs it returns 0 else the remaining time in jiffies (but at least 1). Timeouts are preferably passed by msecs_to_jiffies() or usecs_to_jiffies(). If the returned timeout value is deliberately ignored a comment should probably explain why (e.g. see drivers/mfd/wm8350-core.c wm8350_read_auxadc())

	long wait_for_completion_interruptible_timeout(
		struct completion *done, unsigned long timeout)

This function passes a timeout in jiffies and marking the task as TASK_INTERRUPTIBLE. If a signal was received it will return -ERESTARTSYS, 0 if completion timed out and the remaining time in jiffies if completion occurred.

Further variants include _killable which passes TASK_KILLABLE as the designated tasks state and will return -ERESTARTSYS if interrupted or else 0 if completion was achieved as well as a _timeout variant.

long wait_for_completion_killable(struct completion *done) long wait_for_completion_killable_timeout(struct completion *done,
unsigned long timeout)

The _io variants wait_for_completion_io() behave the same as the non-_io variants, except for accounting waiting time as waiting on IO, which has an impact on how the task is accounted in scheduling stats.

void wait_for_completion_io(struct completion *done) unsigned long wait_for_completion_io_timeout(struct completion *done
unsigned long timeout)

Signaling completions:
----------------------

A thread that wants to signal that the conditions for continuation have been achieved calls complete() to signal exactly one of the waiters that it can continue.

void complete(struct completion *done)

or calls complete_all() to signal all current and future waiters.

void complete_all(struct completion *done)

The signaling will work as expected even if completions are signaled before a thread starts waiting. This is achieved by the waiter "consuming" (decrementing) the done element of struct completion. Waiting threads wakeup order is the same in which they were enqueued (FIFO order).

If complete() is called multiple times then this will allow for that number of waiters to continue - each call to complete() will simply increment the done element. Calling complete_all() multiple times is a bug though. Both complete() and complete_all() can be called in hard-irq/atomic context safely.

There only can be one thread calling complete() or complete_all() on a particular struct completion at any time - serialized through the wait queue spinlock. Any such concurrent calls to complete() or complete_all() probably are a design bug.

Signaling completion from hard-irq context is fine as it will appropriately lock with spin_lock_irqsave/spin_unlock_irqrestore and it will never sleep.

try_wait_for_completion()/completion_done():
--------------------------------------------

The try_wait_for_completion() function will not put the thread on the wait queue but rather returns false if it would need to enqueue (block) the thread, else it consumes any posted completions and returns true.

bool try_wait_for_completion(struct completion *done)

Finally to check state of a completion without changing it in any way is provided by completion_done() returning false if there is any posted completion that was not yet consumed by waiters implying that there are waiters and true otherwise;

bool completion_done(struct completion *done)

Both try_wait_for_completion() and completion_done() are safe to be called in hard-irq or atomic context.

 Documentation/scheduler/completion.txt |   99 ++++++++++++++++++--------------
 1 file changed, 55 insertions(+), 44 deletions(-)

Documentation/scheduler/completion.txt +55 ~~-44~~

index f77651e..fc263c1 100644
@@ -11,11 +11,11 @@ to have reached a point or a specific state, completions can provide a race
 free solution to this problem. Semantically they are somewhat like a
 pthread_barriers and have similar use-cases.
 
-Completions are a code synchronization mechanism that is preferable to any
+Completions are a code synchronization mechanism which are preferable to any
 misuse of locks. Any time you think of using yield() or some quirky
 msleep(1); loop to allow something else to proceed, you probably want to
 look into using one of the wait_for_completion*() calls instead. The
-advantage of using completions is clear intent of the code but also more
+advantage of using completions is clear intent of the code, but also more
 efficient code as both threads can continue until the result is actually
 needed.

@@ -24,7 +24,7 @@ with the event reduced to a simple flag appropriately called "done" in
 struct completion, that tells the waiting threads of execution if they
 can continue safely.
 
-As completions are scheduling related the code is found in
+As completions are scheduling related, the code is found in
 kernel/sched/completion.c - for details on completion design and
 implementation see completions-design.txt

@@ -32,9 +32,9 @@ implementation see completions-design.txt
 Usage:
 ------
 
-There are three parts to the using completions, the initialization of the
+There are three parts to using completions, the initialization of the
 struct completion, the waiting part through a call to one of the variants of
-wait_for_completion() and the signaling side through a call to complete(),
+wait_for_completion() and the signaling side through a call to complete()
 or complete_all(). Further there are some helper functions for checking the
 state of completions.

@@ -50,7 +50,7 @@ handling of completions is:
 providing the wait queue to place tasks on for waiting and the flag for
 indicating the state of affairs.
 
-Completions should be named to convey the intent of the waiter.  A good
+Completions should be named to convey the intent of the waiter. A good
 example is:
 
 	wait_for_completion(&early_console_added);

@@ -73,7 +73,7 @@ the default state to "not available", that is, "done" is set to 0.
 
 The re-initialization function, reinit_completion(), simply resets the
 done element to "not available", thus again to 0, without touching the
-wait queue. Calling init_completion() on the same completions object is
+wait queue. Calling init_completion() on the same completion object is
 most likely a bug as it re-initializes the queue to an empty queue and
 enqueued tasks could get "lost" - use reinit_completion() in that case.

@@ -87,10 +87,17 @@ initialization should always use:
 	DECLARE_COMPLETION_ONSTACK(setup_done)
 
 suitable for automatic/local variables on the stack and will make lockdep
-happy. Note also that one needs to making *sure* the completion passt to
+happy. Note also that one needs to make *sure* the completion passed to
 work threads remains in-scope, and no references remain to on-stack data
 when the initiating function returns.
 
+Using on-stack completions for code that calls any of the _timeout or
+_interruptible/_killable variants is not advisable as they will require
+additional synchronization to prevent the on-stack completion object in
+the timeout/signal cases from going out of scope. Consider using dynamically
+allocated completions when intending to use the _interruptible/_killable
+or _timeout variants of wait_for_completion().
+
 
 Waiting for completions:
 ------------------------

@@ -101,21 +108,22 @@ A typical usage scenario is:
 
 	structure completion setup_done;
 	init_completion(&setup_done);
-	initialze_work(...,&setup_done,...)
+	initialize_work(...,&setup_done,...)
 
 	/* run non-dependent code */              /* do setup */
 
-	wait_for_completion(&seupt_done);         complete(setup_done)
+	wait_for_completion(&setup_done);         complete(setup_done)
 
-This is not implying any temporal order of wait_for_completion() and the
+This is not implying any temporal order on wait_for_completion() and the
 call to complete() - if the call to complete() happened before the call
 to wait_for_completion() then the waiting side simply will continue
-immediately as all dependencies are satisfied.
+immediately as all dependencies are satisfied if not it will block until
+completion is signaled by complete().
 
 Note that wait_for_completion() is calling spin_lock_irq/spin_unlock_irq
 so it can only be called safely when you know that interrupts are enabled.
-Calling it from hard-irq context will result in hard to detect spurious
-enabling of interrupts.
+Calling it from hard-irq or irqs-off atomic contexts will result in hard
+to detect spurious enabling of interrupts.
 
 wait_for_completion():

@@ -123,10 +131,13 @@ wait_for_completion():
 
 The default behavior is to wait without a timeout and mark the task as
 uninterruptible. wait_for_completion() and its variants are only safe
-in soft-interrupt or process context but not in hard-irq context.
+in process context (as they can sleep) but not in atomic context,
+interrupt context, with disabled irqs. or preemption is disabled - see also
+try_wait_for_completion() below for handling completion in atomic/interrupt
+context.
+
 As all variants of wait_for_completion() can (obviously) block for a long
-time, you probably don't want to call this with held locks - see also
-try_wait_for_completion() below.
+time, you probably don't want to call this with held mutexes.
 
 
 Variants available:

@@ -141,20 +152,20 @@ A common problem that occurs is to have unclean assignment of return types,
 so care should be taken with assigning return-values to variables of proper
 type. Checking for the specific meaning of return values also has been found
 to be quite inaccurate e.g. constructs like
-if(!wait_for_completion_interruptible_timeout(...)) would execute the same
+if (!wait_for_completion_interruptible_timeout(...)) would execute the same
 code path for successful completion and for the interrupted case - which is
 probably not what you want.
 
 	int wait_for_completion_interruptible(struct completion *done)
 
-marking the task TASK_INTERRUPTIBLE. If a signal was received while waiting.
-It will return -ERESTARTSYS and 0 otherwise.
+This function marks the task TASK_INTERRUPTIBLE. If a signal was received
+while waiting it will return -ERESTARTSYS and 0 otherwise.
 
 	unsigned long wait_for_completion_timeout(struct completion *done,
 		unsigned long timeout)
 
-The task is marked as TASK_UNINTERRUPTIBLE and will wait at most timeout
-(in jiffies). If timeout occurs it return 0 else the remaining time in
+The task is marked as TASK_UNINTERRUPTIBLE and will wait at most 'timeout'
+(in jiffies). If timeout occurs it returns 0 else the remaining time in
 jiffies (but at least 1). Timeouts are preferably passed by msecs_to_jiffies()
 or usecs_to_jiffies(). If the returned timeout value is deliberately ignored
 a comment should probably explain why (e.g. see drivers/mfd/wm8350-core.c

@@ -163,21 +174,21 @@ wm8350_read_auxadc())
 	long wait_for_completion_interruptible_timeout(
 		struct completion *done, unsigned long timeout)
 
-passing a timeout in jiffies and marking the task as TASK_INTERRUPTIBLE. If a
-signal was received it will return -ERESTARTSYS, 0 if completion timed-out and
-the remaining time in jiffies if completion occurred.
+This function passes a timeout in jiffies and marking the task as
+TASK_INTERRUPTIBLE. If a signal was received it will return -ERESTARTSYS, 0 if
+completion timed out and the remaining time in jiffies if completion occurred.
 
 Further variants include _killable which passes TASK_KILLABLE as the
-designated tasks state and will return a -ERESTARTSYS if interrupted or
-else 0 if completions was achieved as well as a _timeout variant.
+designated tasks state and will return -ERESTARTSYS if interrupted or
+else 0 if completion was achieved as well as a _timeout variant.
 
 	long wait_for_completion_killable(struct completion *done)
 	long wait_for_completion_killable_timeout(struct completion *done,
 		unsigned long timeout)
 
-The _io variants wait_for_completion_io behave the same as the non-_io
+The _io variants wait_for_completion_io() behave the same as the non-_io
 variants, except for accounting waiting time as waiting on IO, which has
-an impact on how scheduling is calculated.
+an impact on how the task is accounted in scheduling stats.
 
 	void wait_for_completion_io(struct completion *done)
 	unsigned long wait_for_completion_io_timeout(struct completion *done

@@ -187,13 +198,13 @@ an impact on how scheduling is calculated.
 Signaling completions:
 ----------------------
 
-A thread of execution that wants to signal that the conditions for
-continuation have been achieved calls complete() to signal exactly one
-of the waiters that it can continue.
+A thread that wants to signal that the conditions for continuation have been
+achieved calls complete() to signal exactly one of the waiters that it can
+continue.
 
 	void complete(struct completion *done)
 
-or calls complete_all to signal all current and future waiters.
+or calls complete_all() to signal all current and future waiters.
 
 	void complete_all(struct completion *done)

@@ -205,32 +216,32 @@ wakeup order is the same in which they were enqueued (FIFO order).
 If complete() is called multiple times then this will allow for that number
 of waiters to continue - each call to complete() will simply increment the
 done element. Calling complete_all() multiple times is a bug though. Both
-complete() and complete_all() can be called in hard-irq context safely.
+complete() and complete_all() can be called in hard-irq/atomic context safely.
 
 There only can be one thread calling complete() or complete_all() on a
-particular struct completions at any time - serialized through the wait
+particular struct completion at any time - serialized through the wait
 queue spinlock. Any such concurrent calls to complete() or complete_all()
 probably are a design bug.
 
 Signaling completion from hard-irq context is fine as it will appropriately
-lock with spin_lock_irqsave/spin_unlock_irqrestore.
+lock with spin_lock_irqsave/spin_unlock_irqrestore and it will never sleep.
 
 
 try_wait_for_completion()/completion_done():
 --------------------------------------------
 
-The try_wait_for_completion will not put the thread on the wait queue but
-rather returns false if it would need to enqueue (block) the thread, else it
-consumes any posted completions and returns true.
+The try_wait_for_completion() function will not put the thread on the wait
+queue but rather returns false if it would need to enqueue (block) the thread,
+else it consumes any posted completions and returns true.
 
-     bool try_wait_for_completion(struct completion *done)
+	bool try_wait_for_completion(struct completion *done)
 
-Finally to check state of a completions without changing it in any way is
-provided by completion_done() returning false if there are any posted
+Finally to check state of a completion without changing it in any way is
+provided by completion_done() returning false if there is any posted
 completion that was not yet consumed by waiters implying that there are
 waiters and true otherwise;
 
-     bool completion_done(struct completion *done)
+	bool completion_done(struct completion *done)
 
 Both try_wait_for_completion() and completion_done() are safe to be called in
-hard-irq context.
+hard-irq or atomic context.

--
1.7.10.4

Jonathan Corbet

maintainer

2015-03-26 17:25 UTC #1 lore raw

Quoted 2 lines · On Fri, 20 Feb 2015 12:28:48 -0500 Nicholas Mc Guire <[email protected]> wrote:

This hopfully address all of the issues Ingo Molnar <[email protected]> noted in https://lkml.org/lkml/2015/2/18/690.

This looks OK to me, modulo some small English quibbles. Ingo, does this satisfy your concerns?

Thanks,

jon

Ingo Molnar

2015-03-27 07:56 UTC #2 lore raw

Quoted 8 lines · Jonathan Corbet

Quoted 2 lines · On Fri, 20 Feb 2015 12:28:48 -0500 Nicholas Mc Guire <[email protected]> wrote:
This hopfully address all of the issues Ingo Molnar <[email protected]> noted in https://lkml.org/lkml/2015/2/18/690.
This looks OK to me, modulo some small English quibbles. Ingo, does this satisfy your concerns?

I wasn't Cc:-ed to the patch and it wasn't Cc:-ed to lkml either :-(

(It would be very useful if documentation update patches were Cc:-ed to the maintainers of the affected subsystem, to double check things and so.)

Thanks,

Ingo

Peter Zijlstra

2015-03-27 09:46 UTC #3 lore raw

Quoted 2 lines · Ingo Molnar

I wasn't Cc:-ed to the patch and it wasn't Cc:-ed to lkml either :-(

/me hands mingo a strong cup of tea...

http://marc.info/?l=linux-kernel&m=142445364429042&w=2

Ingo Molnar

2015-03-27 10:11 UTC #4 lore raw

Quoted 7 lines · Peter Zijlstra

Quoted 2 lines · Ingo Molnar
I wasn't Cc:-ed to the patch and it wasn't Cc:-ed to lkml either :-(
/me hands mingo a strong cup of tea...
http://marc.info/?l=linux-kernel&m=142445364429042&w=2

Hm, then it somehow got spam-filtered out :-/

Could someone forward me the actual patch please?

Thanks,

Ingo

Nicholas Mc Guire

2015-03-27 10:52 UTC #5 lore raw

In reply to Peter Zijlstra

Quoted 8 lines · Peter Zijlstra

Quoted 2 lines · Ingo Molnar
I wasn't Cc:-ed to the patch and it wasn't Cc:-ed to lkml either :-(
/me hands mingo a strong cup of tea...
http://marc.info/?l=linux-kernel&m=142445364429042&w=2

strange: I sent it to

To: Jonathan Corbet <[email protected]>
Cc: Ingo Molnar <[email protected]>,
Valdis Kletnieks <[email protected]>,
Peter Zijlstra <[email protected]>, [email protected], [email protected], Nicholas Mc Guire <[email protected]>

No idea how that happened. Did the mail violate some size-limit or content restriction ?

thx!
hofrat

Ingo Molnar

2015-03-27 11:12 UTC #6 lore raw

Quoted 12 lines · Nicholas Mc Guire

Quoted 8 lines · Peter Zijlstra
Quoted 2 lines · Ingo Molnar
I wasn't Cc:-ed to the patch and it wasn't Cc:-ed to lkml either :-(
/me hands mingo a strong cup of tea...
http://marc.info/?l=linux-kernel&m=142445364429042&w=2
strange: I sent it to

Quoted 8 lines

To: Jonathan Corbet <[email protected]>
Cc: Ingo Molnar <[email protected]>,
Valdis Kletnieks <[email protected]>,
Peter Zijlstra <[email protected]>, [email protected], [email protected], Nicholas Mc Guire <[email protected]>
No idea how that happened. Did the mail violate some size-limit or content restriction ?

I don't think so, since it got into the marc.info archive - so the problem is probably on my side.

In any case it would be nice if I could see Jonathan's modified version, so that I can read the latest and greatest.

Thanks,

Ingo

Ingo Molnar

2015-03-27 11:10 UTC #7 lore raw

In reply to Jonathan Corbet

Quoted 8 lines · Jonathan Corbet

Quoted 2 lines · On Fri, 20 Feb 2015 12:28:48 -0500 Nicholas Mc Guire <[email protected]> wrote:
This hopfully address all of the issues Ingo Molnar <[email protected]> noted in https://lkml.org/lkml/2015/2/18/690.
This looks OK to me, modulo some small English quibbles. Ingo, does this satisfy your concerns?

Mind sending me the final version, modified by you?

Thanks,

Ingo

Jonathan Corbet

maintainer

2015-03-27 16:20 UTC #8 lore raw

Quoted 1 line · On Fri, 27 Mar 2015 12:10:42 +0100 Ingo Molnar <[email protected]> wrote:

Mind sending me the final version, modified by you?

OK, this is the current version. For completeness (so to speak) I'll send the patches as well.

jon

completions - wait for completion handling
==========================================

This document was originally written based on 3.18.0 (linux-next)

Introduction:
-------------

If you have one or more threads of execution that must wait for some process to have reached a point or a specific state, completions can provide a race-free solution to this problem. Semantically they are somewhat like a pthread_barrier and have similar use-cases.

Completions are a code synchronization mechanism which is preferable to any misuse of locks. Any time you think of using yield() or some quirky msleep(1) loop to allow something else to proceed, you probably want to look into using one of the wait_for_completion*() calls instead. The advantage of using completions is clear intent of the code, but also more efficient code as both threads can continue until the result is actually needed.

Completions are built on top of the generic event infrastructure in Linux, with the event reduced to a simple flag (appropriately called "done") in struct completion that tells the waiting threads of execution if they can continue safely.

As completions are scheduling related, the code is found in kernel/sched/completion.c - for details on completion design and implementation see completions-design.txt

Usage:
------

To use completions one needs to include <linux/completion.h> and create a variable of type struct completion. The structure used for handling of completions is:

	struct completion {
		unsigned int done;
		wait_queue_head_t wait;
	};

providing the wait queue to place tasks on for waiting and the flag for indicating the state of affairs.

Completions should be named to convey the intent of the waiter. A good example is:

	wait_for_completion(&early_console_added);

	complete(&early_console_added);

Good naming (as always) helps code readability.

Initializing completions:
-------------------------

Initialization of dynamically allocated completions, often embedded in other structures, is done with:

	void init_completion(&done);

Initialization is accomplished by initializing the wait queue and setting the default state to "not available", that is, "done" is set to 0.

The re-initialization function, reinit_completion(), simply resets the done element to "not available", thus again to 0, without touching the wait queue. Calling init_completion() twice on the same completion object is most likely a bug as it re-initializes the queue to an empty queue and enqueued tasks could get "lost" - use reinit_completion() in that case.

For static declaration and initialization, macros are available. These are:

static DECLARE_COMPLETION(setup_done)

used for static declarations in file scope. Within functions the static initialization should always use:

DECLARE_COMPLETION_ONSTACK(setup_done)

Waiting for completions:
------------------------

For a thread of execution to wait for some concurrent work to finish, it calls wait_for_completion() on the initialized completion structure. A typical usage scenario is:

	struct completion setup_done;
	init_completion(&setup_done);
	initialize_work(...,&setup_done,...)

	/* run non-dependent code */              /* do setup */

	wait_for_completion(&setup_done);         complete(setup_done)

Note that wait_for_completion() is calling spin_lock_irq()/spin_unlock_irq(), so it can only be called safely when you know that interrupts are enabled. Calling it from hard-irq or irqs-off atomic contexts will result in hard-to-detect spurious enabling of interrupts.

wait_for_completion():

void wait_for_completion(struct completion *done):

The default behavior is to wait without a timeout and to mark the task as uninterruptible. wait_for_completion() and its variants are only safe in process context (as they can sleep) but not in atomic context, interrupt context, with disabled irqs. or preemption is disabled - see also try_wait_for_completion() below for handling completion in atomic/interrupt context.

As all variants of wait_for_completion() can (obviously) block for a long time, you probably don't want to call this with held mutexes.

Variants available:
-------------------

int wait_for_completion_interruptible(struct completion *done)

This function marks the task TASK_INTERRUPTIBLE. If a signal was received while waiting it will return -ERESTARTSYS; 0 otherwise.

unsigned long wait_for_completion_timeout(struct completion *done,
unsigned long timeout)

The task is marked as TASK_UNINTERRUPTIBLE and will wait at most 'timeout' (in jiffies). If timeout occurs it returns 0 else the remaining time in jiffies (but at least 1). Timeouts are preferably calculated with msecs_to_jiffies() or usecs_to_jiffies(). If the returned timeout value is deliberately ignored a comment should probably explain why (e.g. see drivers/mfd/wm8350-core.c wm8350_read_auxadc())

	long wait_for_completion_interruptible_timeout(
		struct completion *done, unsigned long timeout)

This function passes a timeout in jiffies and marks the task as TASK_INTERRUPTIBLE. If a signal was received it will return -ERESTARTSYS; otherwise it returns 0 if the completion timed out or the remaining time in jiffies if completion occurred.

Further variants include _killable which uses TASK_KILLABLE as the designated tasks state and will return -ERESTARTSYS if it is interrupted or else 0 if completion was achieved. There is a _timeout variant as well:

long wait_for_completion_killable(struct completion *done) long wait_for_completion_killable_timeout(struct completion *done,
unsigned long timeout)

void wait_for_completion_io(struct completion *done) unsigned long wait_for_completion_io_timeout(struct completion *done
unsigned long timeout)

Signaling completions:
----------------------

A thread that wants to signal that the conditions for continuation have been achieved calls complete() to signal exactly one of the waiters that it can continue.

void complete(struct completion *done)

or calls complete_all() to signal all current and future waiters.

void complete_all(struct completion *done)

Signaling completion from hard-irq context is fine as it will appropriately lock with spin_lock_irqsave/spin_unlock_irqrestore and it will never sleep.

try_wait_for_completion()/completion_done():
--------------------------------------------

The try_wait_for_completion() function will not put the thread on the wait queue but rather returns false if it would need to enqueue (block) the thread, else it consumes one posted completion and returns true.

bool try_wait_for_completion(struct completion *done)

Finally, to check the state of a completion without changing it in any way, call completion_done(), which returns false if there are no posted completions that were not yet consumed by waiters (implying that there are waiters) and true otherwise;

bool completion_done(struct completion *done)

Both try_wait_for_completion() and completion_done() are safe to be called in hard-irq or atomic context.

Jonathan Corbet

maintainer

2015-03-27 16:21 UTC #9 lore raw

PATCH 1/2 doc: completion: context, scope and language fixes

Quoted 1 line

From 486036741940254bb677810e30bdb5aab3191c5c Mon Sep 17 00:00:00 2001

From: Nicholas Mc Guire <[email protected]>
Date: Fri, 20 Feb 2015 12:28:48 -0500
Subject: [PATCH 1/2] doc: completion: context, scope and language fixes

Fix for imprecise/wrong statements on context in which wait_for_completion*() can be called, updated notes on "going out of scope" problems and some language fixups.

Signed-off-by Nicholas Mc Guire <[email protected]>

Signed-off-by Jonathan Corbet <[email protected]>

---
 Documentation/scheduler/completion.txt | 99 +++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 44 deletions(-)

Documentation/scheduler/completion.txt +55 ~~-44~~

index f77651eca31e..083d9c931b8d 100644
@@ -11,11 +11,11 @@ to have reached a point or a specific state, completions can provide a race
 free solution to this problem. Semantically they are somewhat like a
 pthread_barriers and have similar use-cases.
 
-Completions are a code synchronization mechanism that is preferable to any
+Completions are a code synchronization mechanism which are preferable to any
 misuse of locks. Any time you think of using yield() or some quirky
 msleep(1); loop to allow something else to proceed, you probably want to
 look into using one of the wait_for_completion*() calls instead. The
-advantage of using completions is clear intent of the code but also more
+advantage of using completions is clear intent of the code, but also more
 efficient code as both threads can continue until the result is actually
 needed.

@@ -24,7 +24,7 @@ with the event reduced to a simple flag appropriately called "done" in
 struct completion, that tells the waiting threads of execution if they
 can continue safely.
 
-As completions are scheduling related the code is found in
+As completions are scheduling related, the code is found in
 kernel/sched/completion.c - for details on completion design and
 implementation see completions-design.txt

@@ -32,9 +32,9 @@ implementation see completions-design.txt
 Usage:
 ------
 
-There are three parts to the using completions, the initialization of the
+There are three parts to using completions, the initialization of the
 struct completion, the waiting part through a call to one of the variants of
-wait_for_completion() and the signaling side through a call to complete(),
+wait_for_completion() and the signaling side through a call to complete()
 or complete_all(). Further there are some helper functions for checking the
 state of completions.

@@ -50,7 +50,7 @@ handling of completions is:
 providing the wait queue to place tasks on for waiting and the flag for
 indicating the state of affairs.
 
-Completions should be named to convey the intent of the waiter.  A good
+Completions should be named to convey the intent of the waiter. A good
 example is:
 
 	wait_for_completion(&early_console_added);

@@ -73,7 +73,7 @@ the default state to "not available", that is, "done" is set to 0.
 
 The re-initialization function, reinit_completion(), simply resets the
 done element to "not available", thus again to 0, without touching the
-wait queue. Calling init_completion() on the same completions object is
+wait queue. Calling init_completion() on the same completion object is
 most likely a bug as it re-initializes the queue to an empty queue and
 enqueued tasks could get "lost" - use reinit_completion() in that case.

@@ -87,10 +87,17 @@ initialization should always use:
 	DECLARE_COMPLETION_ONSTACK(setup_done)
 
 suitable for automatic/local variables on the stack and will make lockdep
-happy. Note also that one needs to making *sure* the completion passt to
+happy. Note also that one needs to make *sure* the completion passed to
 work threads remains in-scope, and no references remain to on-stack data
 when the initiating function returns.
 
+Using on-stack completions for code that calls any of the _timeout or
+_interruptible/_killable variants is not advisable as they will require
+additional synchronization to prevent the on-stack completion object in
+the timeout/signal cases from going out of scope. Consider using dynamically
+allocated completions when intending to use the _interruptible/_killable
+or _timeout variants of wait_for_completion().
+
 
 Waiting for completions:
 ------------------------

@@ -101,21 +108,22 @@ A typical usage scenario is:
 
 	structure completion setup_done;
 	init_completion(&setup_done);
-	initialze_work(...,&setup_done,...)
+	initialize_work(...,&setup_done,...)
 
 	/* run non-dependent code */              /* do setup */
 
-	wait_for_completion(&seupt_done);         complete(setup_done)
+	wait_for_completion(&setup_done);         complete(setup_done)
 
-This is not implying any temporal order of wait_for_completion() and the
+This is not implying any temporal order on wait_for_completion() and the
 call to complete() - if the call to complete() happened before the call
 to wait_for_completion() then the waiting side simply will continue
-immediately as all dependencies are satisfied.
+immediately as all dependencies are satisfied if not it will block until
+completion is signaled by complete().
 
 Note that wait_for_completion() is calling spin_lock_irq/spin_unlock_irq
 so it can only be called safely when you know that interrupts are enabled.
-Calling it from hard-irq context will result in hard to detect spurious
-enabling of interrupts.
+Calling it from hard-irq or irqs-off atomic contexts will result in hard
+to detect spurious enabling of interrupts.
 
 wait_for_completion():

@@ -123,10 +131,13 @@ wait_for_completion():
 
 The default behavior is to wait without a timeout and mark the task as
 uninterruptible. wait_for_completion() and its variants are only safe
-in soft-interrupt or process context but not in hard-irq context.
+in process context (as they can sleep) but not in atomic context,
+interrupt context, with disabled irqs. or preemption is disabled - see also
+try_wait_for_completion() below for handling completion in atomic/interrupt
+context.
+
 As all variants of wait_for_completion() can (obviously) block for a long
-time, you probably don't want to call this with held locks - see also
-try_wait_for_completion() below.
+time, you probably don't want to call this with held mutexes.
 
 
 Variants available:

@@ -141,20 +152,20 @@ A common problem that occurs is to have unclean assignment of return types,
 so care should be taken with assigning return-values to variables of proper
 type. Checking for the specific meaning of return values also has been found
 to be quite inaccurate e.g. constructs like
-if(!wait_for_completion_interruptible_timeout(...)) would execute the same
+if (!wait_for_completion_interruptible_timeout(...)) would execute the same
 code path for successful completion and for the interrupted case - which is
 probably not what you want.
 
 	int wait_for_completion_interruptible(struct completion *done)
 
-marking the task TASK_INTERRUPTIBLE. If a signal was received while waiting.
-It will return -ERESTARTSYS and 0 otherwise.
+This function marks the task TASK_INTERRUPTIBLE. If a signal was received
+while waiting it will return -ERESTARTSYS and 0 otherwise.
 
 	unsigned long wait_for_completion_timeout(struct completion *done,
 		unsigned long timeout)
 
-The task is marked as TASK_UNINTERRUPTIBLE and will wait at most timeout
-(in jiffies). If timeout occurs it return 0 else the remaining time in
+The task is marked as TASK_UNINTERRUPTIBLE and will wait at most 'timeout'
+(in jiffies). If timeout occurs it returns 0 else the remaining time in
 jiffies (but at least 1). Timeouts are preferably passed by msecs_to_jiffies()
 or usecs_to_jiffies(). If the returned timeout value is deliberately ignored
 a comment should probably explain why (e.g. see drivers/mfd/wm8350-core.c

@@ -163,21 +174,21 @@ wm8350_read_auxadc())
 	long wait_for_completion_interruptible_timeout(
 		struct completion *done, unsigned long timeout)
 
-passing a timeout in jiffies and marking the task as TASK_INTERRUPTIBLE. If a
-signal was received it will return -ERESTARTSYS, 0 if completion timed-out and
-the remaining time in jiffies if completion occurred.
+This function passes a timeout in jiffies and marking the task as
+TASK_INTERRUPTIBLE. If a signal was received it will return -ERESTARTSYS, 0 if
+completion timed out and the remaining time in jiffies if completion occurred.
 
 Further variants include _killable which passes TASK_KILLABLE as the
-designated tasks state and will return a -ERESTARTSYS if interrupted or
-else 0 if completions was achieved as well as a _timeout variant.
+designated tasks state and will return -ERESTARTSYS if interrupted or
+else 0 if completion was achieved as well as a _timeout variant.
 
 	long wait_for_completion_killable(struct completion *done)
 	long wait_for_completion_killable_timeout(struct completion *done,
 		unsigned long timeout)
 
-The _io variants wait_for_completion_io behave the same as the non-_io
+The _io variants wait_for_completion_io() behave the same as the non-_io
 variants, except for accounting waiting time as waiting on IO, which has
-an impact on how scheduling is calculated.
+an impact on how the task is accounted in scheduling stats.
 
 	void wait_for_completion_io(struct completion *done)
 	unsigned long wait_for_completion_io_timeout(struct completion *done

@@ -187,13 +198,13 @@ an impact on how scheduling is calculated.
 Signaling completions:
 ----------------------
 
-A thread of execution that wants to signal that the conditions for
-continuation have been achieved calls complete() to signal exactly one
-of the waiters that it can continue.
+A thread that wants to signal that the conditions for continuation have been
+achieved calls complete() to signal exactly one of the waiters that it can
+continue.
 
 	void complete(struct completion *done)
 
-or calls complete_all to signal all current and future waiters.
+or calls complete_all() to signal all current and future waiters.
 
 	void complete_all(struct completion *done)

@@ -205,32 +216,32 @@ wakeup order is the same in which they were enqueued (FIFO order).
 If complete() is called multiple times then this will allow for that number
 of waiters to continue - each call to complete() will simply increment the
 done element. Calling complete_all() multiple times is a bug though. Both
-complete() and complete_all() can be called in hard-irq context safely.
+complete() and complete_all() can be called in hard-irq/atomic context safely.
 
 There only can be one thread calling complete() or complete_all() on a
-particular struct completions at any time - serialized through the wait
+particular struct completion at any time - serialized through the wait
 queue spinlock. Any such concurrent calls to complete() or complete_all()
 probably are a design bug.
 
 Signaling completion from hard-irq context is fine as it will appropriately
-lock with spin_lock_irqsave/spin_unlock_irqrestore.
+lock with spin_lock_irqsave/spin_unlock_irqrestore and it will never sleep.
 
 
 try_wait_for_completion()/completion_done():
 --------------------------------------------
 
-The try_wait_for_completion will not put the thread on the wait queue but
-rather returns false if it would need to enqueue (block) the thread, else it
-consumes any posted completions and returns true.
+The try_wait_for_completion() function will not put the thread on the wait
+queue but rather returns false if it would need to enqueue (block) the thread,
+else it consumes any posted completions and returns true.
 
-     bool try_wait_for_completion(struct completion *done)
+	bool try_wait_for_completion(struct completion *done)
 
-Finally to check state of a completions without changing it in any way is
-provided by completion_done() returning false if there are any posted
+Finally to check state of a completion without changing it in any way is
+provided by completion_done() returning false if there is any posted
 completion that was not yet consumed by waiters implying that there are
 waiters and true otherwise;
 
-     bool completion_done(struct completion *done)
+	bool completion_done(struct completion *done)
 
 Both try_wait_for_completion() and completion_done() are safe to be called in
-hard-irq context.
+hard-irq or atomic context.

Signature · 1 line

2.1.0

Jonathan Corbet

maintainer

2015-03-27 16:22 UTC #10 lore raw

PATCH 2/2 docs/completion.txt: Various tweaks and corrections

In reply to Jonathan Corbet

Quoted 1 line

From 5fbce97c75f0ac713af4ac8563058e8a023f07b4 Mon Sep 17 00:00:00 2001

From: Jonathan Corbet <[email protected]>
Date: Fri, 27 Mar 2015 10:16:35 -0600
Subject: [PATCH 2/2] docs/completion.txt: Various tweaks and corrections

Mostly language improvements to the new completions.txt document, but there is also a semantic correction in the description of completion_done() at the very end.

Signed-off-by Jonathan Corbet <[email protected]>

---
 Documentation/scheduler/completion.txt | 59 +++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 29 deletions(-)

Documentation/scheduler/completion.txt +30 ~~-29~~

index 083d9c931b8d..2622bc7a188b 100644
@@ -7,21 +7,21 @@ Introduction:
 -------------
 
 If you have one or more threads of execution that must wait for some process
-to have reached a point or a specific state, completions can provide a race
-free solution to this problem. Semantically they are somewhat like a
-pthread_barriers and have similar use-cases.
+to have reached a point or a specific state, completions can provide a
+race-free solution to this problem. Semantically they are somewhat like a
+pthread_barrier and have similar use-cases.
 
-Completions are a code synchronization mechanism which are preferable to any
+Completions are a code synchronization mechanism which is preferable to any
 misuse of locks. Any time you think of using yield() or some quirky
-msleep(1); loop to allow something else to proceed, you probably want to
+msleep(1) loop to allow something else to proceed, you probably want to
 look into using one of the wait_for_completion*() calls instead. The
 advantage of using completions is clear intent of the code, but also more
 efficient code as both threads can continue until the result is actually
 needed.
 
 Completions are built on top of the generic event infrastructure in Linux,
-with the event reduced to a simple flag appropriately called "done" in
-struct completion, that tells the waiting threads of execution if they
+with the event reduced to a simple flag (appropriately called "done") in
+struct completion that tells the waiting threads of execution if they
 can continue safely.
 
 As completions are scheduling related, the code is found in

@@ -73,7 +73,7 @@ the default state to "not available", that is, "done" is set to 0.
 
 The re-initialization function, reinit_completion(), simply resets the
 done element to "not available", thus again to 0, without touching the
-wait queue. Calling init_completion() on the same completion object is
+wait queue. Calling init_completion() twice on the same completion object is
 most likely a bug as it re-initializes the queue to an empty queue and
 enqueued tasks could get "lost" - use reinit_completion() in that case.

@@ -106,7 +106,7 @@ For a thread of execution to wait for some concurrent work to finish, it
 calls wait_for_completion() on the initialized completion structure.
 A typical usage scenario is:
 
-	structure completion setup_done;
+	struct completion setup_done;
 	init_completion(&setup_done);
 	initialize_work(...,&setup_done,...)

@@ -120,16 +120,16 @@ to wait_for_completion() then the waiting side simply will continue
 immediately as all dependencies are satisfied if not it will block until
 completion is signaled by complete().
 
-Note that wait_for_completion() is calling spin_lock_irq/spin_unlock_irq
+Note that wait_for_completion() is calling spin_lock_irq()/spin_unlock_irq(),
 so it can only be called safely when you know that interrupts are enabled.
-Calling it from hard-irq or irqs-off atomic contexts will result in hard
-to detect spurious enabling of interrupts.
+Calling it from hard-irq or irqs-off atomic contexts will result in
+hard-to-detect spurious enabling of interrupts.
 
 wait_for_completion():
 
 	void wait_for_completion(struct completion *done):
 
-The default behavior is to wait without a timeout and mark the task as
+The default behavior is to wait without a timeout and to mark the task as
 uninterruptible. wait_for_completion() and its variants are only safe
 in process context (as they can sleep) but not in atomic context,
 interrupt context, with disabled irqs. or preemption is disabled - see also

@@ -159,28 +159,29 @@ probably not what you want.
 	int wait_for_completion_interruptible(struct completion *done)
 
 This function marks the task TASK_INTERRUPTIBLE. If a signal was received
-while waiting it will return -ERESTARTSYS and 0 otherwise.
+while waiting it will return -ERESTARTSYS; 0 otherwise.
 
 	unsigned long wait_for_completion_timeout(struct completion *done,
 		unsigned long timeout)
 
 The task is marked as TASK_UNINTERRUPTIBLE and will wait at most 'timeout'
 (in jiffies). If timeout occurs it returns 0 else the remaining time in
-jiffies (but at least 1). Timeouts are preferably passed by msecs_to_jiffies()
-or usecs_to_jiffies(). If the returned timeout value is deliberately ignored
-a comment should probably explain why (e.g. see drivers/mfd/wm8350-core.c
-wm8350_read_auxadc())
+jiffies (but at least 1). Timeouts are preferably calculated with
+msecs_to_jiffies() or usecs_to_jiffies(). If the returned timeout value is
+deliberately ignored a comment should probably explain why (e.g. see
+drivers/mfd/wm8350-core.c wm8350_read_auxadc())
 
 	long wait_for_completion_interruptible_timeout(
 		struct completion *done, unsigned long timeout)
 
-This function passes a timeout in jiffies and marking the task as
-TASK_INTERRUPTIBLE. If a signal was received it will return -ERESTARTSYS, 0 if
-completion timed out and the remaining time in jiffies if completion occurred.
+This function passes a timeout in jiffies and marks the task as
+TASK_INTERRUPTIBLE. If a signal was received it will return -ERESTARTSYS;
+otherwise it returns 0 if the completion timed out or the remaining time in
+jiffies if completion occurred.
 
-Further variants include _killable which passes TASK_KILLABLE as the
-designated tasks state and will return -ERESTARTSYS if interrupted or
-else 0 if completion was achieved as well as a _timeout variant.
+Further variants include _killable which uses TASK_KILLABLE as the
+designated tasks state and will return -ERESTARTSYS if it is interrupted or
+else 0 if completion was achieved.  There is a _timeout variant as well:
 
 	long wait_for_completion_killable(struct completion *done)
 	long wait_for_completion_killable_timeout(struct completion *done,

@@ -232,14 +233,14 @@ try_wait_for_completion()/completion_done():
 
 The try_wait_for_completion() function will not put the thread on the wait
 queue but rather returns false if it would need to enqueue (block) the thread,
-else it consumes any posted completions and returns true.
+else it consumes one posted completion and returns true.
 
 	bool try_wait_for_completion(struct completion *done)
 
-Finally to check state of a completion without changing it in any way is
-provided by completion_done() returning false if there is any posted
-completion that was not yet consumed by waiters implying that there are
-waiters and true otherwise;
+Finally, to check the state of a completion without changing it in any way, 
+call completion_done(), which returns false if there are no posted
+completions that were not yet consumed by waiters (implying that there are
+waiters) and true otherwise;
 
 	bool completion_done(struct completion *done)

Signature · 1 line

2.1.0

Ingo Molnar

Acked-by

2015-03-29 07:00 UTC #11 lore raw

In reply to Jonathan Corbet

Quoted 6 lines · Jonathan Corbet

Quoted 1 line · On Fri, 27 Mar 2015 12:10:42 +0100 Ingo Molnar <[email protected]> wrote:
Mind sending me the final version, modified by you?
OK, this is the current version. [...]

Much appreciated!

Acked-by Ingo Molnar <[email protected]>

Thanks,

Ingo