lkml.org 
[lkml]   [2016]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: 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;
    }

    \
     
     \ /
      Last update: 2016-10-26 17:42    [W:3.315 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site