Messages in this thread Patch in this message | | | Date | Wed, 26 Oct 2016 17:42:13 +0200 | From | Peter Zijlstra <> | Subject | Re: complete_all and "forever" completions |
| |
On Wed, Oct 26, 2016 at 05:10:01AM -0700, Dmitry Torokhov wrote: > On Wed, Oct 26, 2016 at 10:45:35AM +0200, Peter Zijlstra wrote: > > On Tue, Oct 25, 2016 at 03:30:54PM -0700, Dmitry Torokhov wrote:
> > > Or do we need something like this in > > > do_wait_for_common(): > > > > > > if (x->done < UINT_MAX/2) > > > x->done--; > > > > Depends a bit, do you really want this? Seems a bit daft to keep asking > > if its done already, seems like a waste of cycles to me. > > > > The use case I am after is: > > 1. There is a device that is extremely dumb without firmware > 2. The driver uses request_firmware_nowait() and signals completion from > the firmware loading callback to let the reset of the driver know that > firmware has been done loading (successfully or otherwise) > 3. The driver uses wait_for_completion() in both remove() and suspend() > methods to wait for the firmware to finish loading. > > While remove() happens at most once per device instance, suspend() may > happen unbound number of times (theoretically). > > So the question is: should complete_all have this "forever" semantic > (IOW is documentation right about the intent) or do we need a new > primitive for this? From the cursory glance of users of complete_all() > all of them expect completion to stay in signalled state either forever, > or until they call reinit_completion() explicitly.
Nah, if we need this we should fix this one. Adding similar but slightly different primitives is a pain.
But I think you might need slightly more than the proposed change, the case I worry about is doing complete_all() when done != 0 (which isn't all that strange).
Does something like so work?
--- kernel/sched/completion.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c index 8d0f35debf35..5deab9c789df 100644 --- a/kernel/sched/completion.c +++ b/kernel/sched/completion.c @@ -51,7 +51,7 @@ void complete_all(struct completion *x) unsigned long flags; spin_lock_irqsave(&x->wait.lock, flags); - x->done += UINT_MAX/2; + x->done = UINT_MAX/2; __wake_up_locked(&x->wait, TASK_NORMAL, 0); spin_unlock_irqrestore(&x->wait.lock, flags); } @@ -79,7 +79,10 @@ do_wait_for_common(struct completion *x, if (!x->done) return timeout; } - x->done--; + + if (x->done != UINT_MAX/2) + x->done--; + return timeout ?: 1; }
| |