From 4988aaa6e508614e5d4c4f08723635fc8191188b Mon Sep 17 00:00:00 2001 From: Nicholas Mc Guire Date: Fri, 20 Feb 2015 12:28:48 -0500 Subject: [PATCH] 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 Acked-by: Ingo Molnar Signed-off-by: Jonathan Corbet --- Documentation/scheduler/completion.txt | 99 ++++++++++++++------------ 1 file changed, 55 insertions(+), 44 deletions(-) diff --git a/Documentation/scheduler/completion.txt b/Documentation/scheduler/completion.txt index f77651eca31e..083d9c931b8d 100644 --- a/Documentation/scheduler/completion.txt +++ b/Documentation/scheduler/completion.txt @@ -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. -- 2.39.5