mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: linux-kernel@vger.kernel.org,
	Sumit Semwal <sumit.semwal@linaro.org>,
	Shuah Khan <shuahkh@osg.samsung.com>, Tejun Heo <tj@kernel.org>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>, Kees Cook <keescook@chromium.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Nikolay Aleksandrov <nikolay@cumulusnetworks.com>,
	"David S. Miller" <davem@davemloft.net>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Alexander Potapenko <glider@google.com>,
	linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linaro-mm-sig@lists.linaro.org
Subject: Re: [PATCH 2/9] async: Introduce kfence, a N:M completion mechanism
Date: Wed, 13 Jul 2016 11:38:52 +0200	[thread overview]
Message-ID: <20160713093852.GZ30921@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <1466759333-4703-3-git-send-email-chris@chris-wilson.co.uk>

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.
> + */

  reply	other threads:[~2016-07-13  9:39 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-24  9:08 Introduce fences for N:M completion variables Chris Wilson
2016-06-24  9:08 ` [PATCH 1/9] lib: Add kselftests for async-domains Chris Wilson
2016-06-24  9:08 ` [PATCH 2/9] async: Introduce kfence, a N:M completion mechanism Chris Wilson
2016-07-13  9:38   ` Peter Zijlstra [this message]
2016-07-13 10:20     ` Chris Wilson
2016-07-13 11:02       ` Daniel Vetter
2016-07-13 10:26   ` Peter Zijlstra
2016-06-24  9:08 ` [PATCH 3/9] async: Extend kfence to allow struct embedding Chris Wilson
2016-07-13 10:31   ` Peter Zijlstra
2016-06-24  9:08 ` [PATCH 4/9] async: Extend kfences for listening on DMA fences Chris Wilson
2016-06-24  9:08 ` [PATCH 5/9] async: Wrap hrtimer to provide a time source for a kfence Chris Wilson
2016-07-13 10:32   ` Peter Zijlstra
2016-06-24  9:08 ` [PATCH 6/9] async: Add a convenience wrapper for waiting on implicit dma-buf Chris Wilson
2016-06-24  9:08 ` [PATCH 7/9] async: Add support for explicit fine-grained barriers Chris Wilson
2016-06-24  9:08 ` [PATCH 8/9] async: Add execution barriers Chris Wilson
2016-06-24  9:08 ` [PATCH 9/9] async: Introduce a dependency resolver for parallel execution Chris Wilson
2016-07-17 12:58 ` Introduce fences for N:M completion variables [v2] Chris Wilson
2016-07-17 12:58   ` [PATCH v2 1/7] kfence: Introduce kfence, a N:M completion mechanism Chris Wilson
2016-07-17 12:58   ` [PATCH v2 2/7] kfence: Wrap hrtimer to provide a time source for a kfence Chris Wilson
2016-07-17 12:58   ` [PATCH v2 3/7] kfence: Extend kfences for listening on DMA fences Chris Wilson
2016-07-17 12:58   ` [PATCH v2 4/7] async: Add kselftests for async-domains Chris Wilson
2016-07-17 12:58   ` [PATCH v2 5/7] async: Add support for explicit fine-grained barriers Chris Wilson
2016-07-17 12:58   ` [PATCH v2 6/7] async: Add execution barriers Chris Wilson
2016-07-17 12:58   ` [PATCH v2 7/7] async: Introduce a dependency resolver for parallel execution Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160713093852.GZ30921@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=aryabinin@virtuozzo.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dave@stgolabs.net \
    --cc=davem@davemloft.net \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=keescook@chromium.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mingo@kernel.org \
    --cc=nikolay@cumulusnetworks.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=shuahkh@osg.samsung.com \
    --cc=sumit.semwal@linaro.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome