Re: [PATCH 2/9] async: Introduce kfence, a N:M completion mechanism

From: Peter Zijlstra
Date: Wed Jul 13 2016 - 05:39:25 EST


On Fri, Jun 24, 2016 at 10:08:46AM +0100, Chris Wilson wrote:
> diff --git a/kernel/async.c b/kernel/async.c
> index d2edd6efec56..d0bcb7cc4884 100644
> --- a/kernel/async.c
> +++ b/kernel/async.c
> @@ -50,6 +50,7 @@ asynchronous and synchronous parts of the kernel.
>
> #include <linux/async.h>
> #include <linux/atomic.h>
> +#include <linux/kfence.h>
> #include <linux/ktime.h>
> #include <linux/export.h>
> #include <linux/wait.h>

So why does this live in async.c? It got its own header, why not also
give it its own .c file?

Also, I'm not a particular fan of the k* naming, but I see 'fence' is
already taken.

> +/**
> + * DOC: kfence overview
> + *
> + * kfences provide synchronisation barriers between multiple processes.
> + * They are very similar to completions, or a pthread_barrier. Where
> + * kfence differs from completions is their ability to track multiple
> + * event sources rather than being a singular "completion event". Similar
> + * to completions, multiple processes or other kfences can listen or wait
> + * upon a kfence to signal its completion.
> + *
> + * The kfence is a one-shot flag, signaling that work has progressed passed
> + * a certain point (as measured by completion of all events the kfence is
> + * listening for) and the waiters upon kfence may proceed.
> + *
> + * kfences provide both signaling and waiting routines:
> + *
> + * kfence_pending()
> + *
> + * indicates that the kfence should itself wait for another signal. A
> + * kfence created by kfence_create() starts in the pending state.

I would much prefer:

* - kfence_pending(): indicates that the kfence should itself wait for
* another signal. A kfence created by kfence_create() starts in the
* pending state.

Which is much clearer in what text belongs where.

Also, what !? I don't get what this function does.

> + *
> + * kfence_signal()
> + *
> + * undoes the earlier pending notification and allows the fence to complete
> + * if all pending events have then been signaled.

So I know _signal() is the posix thing, but seeing how we already
completions, how about being consistent with those and use _complete()
for this?

> + *
> + * kfence_wait()
> + *
> + * allows the caller to sleep (uninterruptibly) until the fence is complete.

whitespace to separate the description of kfence_wait() from whatever
else follows.

> + * Meanwhile,
> + *
> + * kfence_complete()
> + *
> + * reports whether or not the kfence has been passed.

kfence_done(), again to match completions.

> + *
> + * This interface is very similar to completions, with the exception of
> + * allowing multiple pending / signals to be sent before the kfence is
> + * complete.
> + *
> + * kfence_add() / kfence_add_completion()
> + *
> + * sets the kfence to wait upon another fence, or completion respectively.
> + *
> + * Unlike completions, kfences are expected to live inside more complex graphs
> + * and form the basis for parallel execution of interdependent and so are
> + * reference counted. A kfence may be created using,
> + *
> + * kfence_create()
> + *
> + * The fence starts off pending a single signal. Once you have finished
> + * setting up the fence, use kfence_signal() to allow it to wait upon
> + * its event sources.
> + *
> + * Use
> + *
> + * kfence_get() / kfence_put
> + *
> + * to acquire or release a reference on kfence respectively.
> + */