Re: [PATCH] dma-buf: fix debugfs versus rcu and fence dumping v2
Quoting jgli...@redhat.com (2018-12-06 15:47:04) > From: Jérôme Glisse > > The debugfs take reference on fence without dropping them. Also the > rcu section are not well balance. Fix all that ... Wouldn't the code be a lot simpler (and a consistent snapshot) if it used reservation_object_get_fences_rcu()? -Chris
Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error
Quoting Ezequiel Garcia (2018-05-14 22:28:31)
> On Mon, 2018-05-14 at 18:48 +0200, Daniel Vetter wrote:
> > On Fri, May 11, 2018 at 08:27:41AM +0100, Chris Wilson wrote:
> > > Quoting Ezequiel Garcia (2018-05-09 21:14:49)
> > > > Change how dma_fence_add_callback() behaves, when the fence
> > > > has error-signaled by the time it is being add. After this commit,
> > > > dma_fence_add_callback() returns the fence error, if it
> > > > has error-signaled before dma_fence_add_callback() is called.
> > >
> > > Why? What problem are you trying to solve? fence->error does not imply
> > > that the fence has yet been signaled, and the caller wants a callback
> > > when it is signaled.
> >
> > On top this is incosistent, e.g. we don't do the same for any of the other
> > dma_fence interfaces. Plus there's the issue that you might alias errno
> > values with fence errno values.
> >
>
> Right.
>
> > I think keeping the error codes from the functions you're calling distinct
> > from the error code of the fence itself makes a lot of sense. The first
> > tells you whether your request worked out (or why not), the second tells
> > you whether the asynchronous dma operation (gpu rendering, page flip,
> > whatever) that the dma_fence represents worked out (or why not). That's 2
> > distinct things imo.
> >
> > Might be good to show us the driver code that needs this behaviour so we
> > can discuss how to best handle your use-case.
> >
>
> This change arose while discussing the in-fences support for video4linux.
> Here's the patch that calls dma_fence_add_callback
> https://lkml.org/lkml/2018/5/4/766.
>
> The code snippet currently looks something like this:
>
> if (vb->in_fence) {
> ret = dma_fence_add_callback(vb->in_fence, &vb->fence_cb,
>
> vb2_qbuf_fence_cb);
> /* is the fence signaled? */
> if (ret == -ENOENT) {
>
> dma_fence_put(vb->in_fence);
> vb->in_fence = NULL;
> } else if (ret)
> {
> goto unlock;
> }
> }
>
> In this use case, if the callback is added successfully,
> the video4linux core defers the activation of the buffer
> until the fence signals.
>
> If the fence is signaled (currently disregarding of errors)
> then the buffer is assumed to be ready to be activated,
> and so it gets queued for hardware usage.
>
> Giving some more thought to this, I'm not so sure what is
> the right action if a fence signaled with error. In this case,
> it appears to me that we shouldn't be using this buffer
> if its in-fence is in error, but perhaps I'm missing
> something.
What I have in mind for async errors is to skip the operation and
propagate the error onto the next fence. Mostly because those async
errors may include fatal errors such as unable to pin the backing
storage for the operation, but even "trivial" errors such as an early
operation failing means that this request is then subject to garbage-in,
garbage-out. However, for trivial errors I would just propagate the
error status (so the caller knows something went wrong if they care, but
in all likelihood no one will notice) and continue on with the glitchy
operation.
-Chris
Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error
Quoting Ezequiel Garcia (2018-05-09 21:14:49) > Change how dma_fence_add_callback() behaves, when the fence > has error-signaled by the time it is being add. After this commit, > dma_fence_add_callback() returns the fence error, if it > has error-signaled before dma_fence_add_callback() is called. Why? What problem are you trying to solve? fence->error does not imply that the fence has yet been signaled, and the caller wants a callback when it is signaled. -Chris
Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error
Quoting Ezequiel Garcia (2018-05-10 13:51:56)
> On Wed, 2018-05-09 at 19:42 -0300, Gustavo Padovan wrote:
> > Hi Ezequiel,
> >
> > On Wed, 2018-05-09 at 17:14 -0300, Ezequiel Garcia wrote:
> > > Change how dma_fence_add_callback() behaves, when the fence
> > > has error-signaled by the time it is being add. After this commit,
> > > dma_fence_add_callback() returns the fence error, if it
> > > has error-signaled before dma_fence_add_callback() is called.
> > >
> > > Signed-off-by: Ezequiel Garcia
> > > ---
> > > drivers/dma-buf/dma-fence.c | 10 ++
> > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-
> > > fence.c
> > > index 4edb9fd3cf47..298b440c5b68 100644
> > > --- a/drivers/dma-buf/dma-fence.c
> > > +++ b/drivers/dma-buf/dma-fence.c
> > > @@ -226,7 +226,8 @@ EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
> > > *
> > > * Note that the callback can be called from an atomic context. If
> > > * fence is already signaled, this function will return -ENOENT (and
> > > - * *not* call the callback)
> > > + * *not* call the callback). If the fence is error-signaled, this
> > > + * function returns the fence error.
> > > *
> > > * Add a software callback to the fence. Same restrictions apply to
> > > * refcount as it does to dma_fence_wait, however the caller doesn't
> > > need to
> > > @@ -235,8 +236,8 @@ EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
> > > * after it signals with dma_fence_signal. The callback itself can
> > > be called
> > > * from irq context.
> > > *
> > > - * Returns 0 in case of success, -ENOENT if the fence is already
> > > signaled
> > > - * and -EINVAL in case of error.
> > > + * Returns 0 in case of success, -ENOENT (or the error value) if the
> > > fence is
> > > + * already signaled and -EINVAL in case of error.
> > > */
> > > int dma_fence_add_callback(struct dma_fence *fence, struct
> > > dma_fence_cb *cb,
> > >dma_fence_func_t func)
> > > @@ -250,7 +251,8 @@ int dma_fence_add_callback(struct dma_fence
> > > *fence, struct dma_fence_cb *cb,
> > >
> > > if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> > > INIT_LIST_HEAD(&cb->node);
> > > - return -ENOENT;
> > > + ret = (fence->error < 0) ? fence->error : -ENOENT;
> > > + return ret;
> > > }
> >
> > It looks good to me, but I'd first go check all place we call it in the
> > kernel because I have some memory of callers relying on the -ENOENT
> > return code for some decision. I might be wrong though.
> >
> >
>
> I checked all users before submitting this patch.
>
> git grep " = dma_fence_add_callback"
> drivers/gpu/drm/i915/i915_sw_fence.c: ret = dma_fence_add_callback(dma,
> &cb->base, func);
>
> And from what I could see, all of them handle the error
> properly.
Err, no. That error then is propagated back to userspace, and that is
not part of our ABI...
-Chris
Re: [PATCH 04/15] dma-fence: Make ->wait callback optional
Quoting Daniel Vetter (2018-05-03 15:25:52) > Almost everyone uses dma_fence_default_wait. > > v2: Also remove the BUG_ON(!ops->wait) (Chris). I just don't get the rationale for implicit over explicit. -Chris
Re: [PATCH 02/15] dma-fence: Make ->enable_signaling optional
Quoting Daniel Vetter (2018-05-03 15:25:50)
> @@ -560,7 +567,7 @@ dma_fence_init(struct dma_fence *fence, const struct
> dma_fence_ops *ops,
>spinlock_t *lock, u64 context, unsigned seqno)
> {
> BUG_ON(!lock);
> - BUG_ON(!ops || !ops->wait || !ops->enable_signaling ||
> + BUG_ON(!ops || !ops->wait ||
>!ops->get_driver_name || !ops->get_timeline_name);
One thing I was wondering about (following the discussion of rhashtable
on lwn) was inlining this function and passing dma_fence_ops by value.
And seeing if that eliminates the branch and makes smaller code
(probably not, mostly idling wondering about that technique) and kills
off the BUGs (can then be BUILD_BUG_ON).
-Chris
Re: [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2
Quoting Christian König (2018-03-16 14:22:32) [snip, probably lost too must context] > This allows for full grown pipelining, e.g. the exporter can say I need > to move the buffer for some operation. Then let the move operation wait > for all existing fences in the reservation object and install the fence > of the move operation as exclusive fence. Ok, the situation I have in mind is the non-pipelined case: revoking dma-buf for mmu_invalidate_range or shrink_slab. I would need a completion event that can be waited on the cpu for all the invalidate callbacks. (Essentially an atomic_t counter plus struct completion; a lighter version of dma_fence, I wonder where I've seen that before ;) Even so, it basically means passing a fence object down to the async callbacks for them to signal when they are complete. Just to handle the non-pipelined version. :| -Chris
Re: [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2
Quoting Christian König (2018-03-16 13:20:45)
> @@ -326,6 +338,29 @@ struct dma_buf_attachment {
> struct device *dev;
> struct list_head node;
> void *priv;
> +
> + /**
> +* @invalidate_mappings:
> +*
> +* Optional callback provided by the importer of the attachment which
> +* must be set before mappings are created.
> +*
> +* If provided the exporter can avoid pinning the backing store while
> +* mappings exists.
Hmm, no I don't think it avoids the pinning issue entirely. As it stands,
the importer doesn't have a page refcount and so they all rely on the
exporter keeping the dmabuf pages pinned while attached. What can happen
is that given the invalidate cb, the importers can revoke their
attachments, letting the exporter recover the pages/sg, and then start
again from scratch.
That also neatly answers what happens if not all importers provide an
invalidate cb, or fail, the dmabuf remains pinned and the exporter must
retreat.
> +*
> +* The function is called with the lock of the reservation object
> +* associated with the dma_buf held and the mapping function must be
> +* called with this lock held as well. This makes sure that no mapping
> +* is created concurrently with an ongoing invalidation.
> +*
> +* After the callback all existing mappings are still valid until all
> +* fences in the dma_bufs reservation object are signaled, but should
> be
> +* destroyed by the importer as soon as possible.
> +*
> +* New mappings can be created immediately, but can't be used before
> the
> +* exclusive fence in the dma_bufs reservation object is signaled.
> +*/
> + void (*invalidate_mappings)(struct dma_buf_attachment *attach);
The intent is that the invalidate is synchronous and immediate, while
locked? We are looking at recursing back into the dma_buf functions to
remove each attachment from the invalidate cb (as well as waiting for
dma), won't that cause some nasty issues?
-Chris
Re: [PATCH] reservation: don't wait when timeout=0
Quoting Christian König (2017-11-21 15:49:55) > Am 21.11.2017 um 15:59 schrieb Rob Clark: > > On Tue, Nov 21, 2017 at 9:38 AM, Chris Wilson > > wrote: > >> Quoting Rob Clark (2017-11-21 14:08:46) > >>> If we are testing if a reservation object's fences have been > >>> signaled with timeout=0 (non-blocking), we need to pass 0 for > >>> timeout to dma_fence_wait_timeout(). > >>> > >>> Plus bonus spelling correction. > >>> > >>> Signed-off-by: Rob Clark > >>> --- > >>> drivers/dma-buf/reservation.c | 11 +-- > >>> 1 file changed, 9 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > >>> index dec3a815455d..71f51140a9ad 100644 > >>> --- a/drivers/dma-buf/reservation.c > >>> +++ b/drivers/dma-buf/reservation.c > >>> @@ -420,7 +420,7 @@ EXPORT_SYMBOL_GPL(reservation_object_get_fences_rcu); > >>>* > >>>* RETURNS > >>>* Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or > >>> - * greater than zer on success. > >>> + * greater than zero on success. > >>>*/ > >>> long reservation_object_wait_timeout_rcu(struct reservation_object *obj, > >>> bool wait_all, bool intr, > >>> @@ -483,7 +483,14 @@ long reservation_object_wait_timeout_rcu(struct > >>> reservation_object *obj, > >>> goto retry; > >>> } > >>> > >>> - ret = dma_fence_wait_timeout(fence, intr, ret); > >>> + /* > >>> +* Note that dma_fence_wait_timeout() will return 1 if > >>> +* the fence is already signaled, so in the wait_all > >>> +* case when we go through the retry loop again, ret > >>> +* will be greater than 0 and we don't want this to > >>> +* cause _wait_timeout() to block > >>> +*/ > >>> + ret = dma_fence_wait_timeout(fence, intr, timeout ? ret : > >>> 0); > >> One should ask if we should just fix the interface to stop returning > >> incorrect results (stop "correcting" a completion with 0 jiffies remaining > >> as 1). A timeout can be distinguished by -ETIME (or your pick of errno). > > perhaps -EBUSY, if we go that route (although maybe it should be a > > follow-on patch, this one is suitable for backport to stable/lts if > > one should so choose..) > > > > I think current approach was chosen to match schedule_timeout() and > > other such functions that take a timeout in jiffies. Not making a > > judgement on whether that is a good or bad reason.. > > We intentionally switched away from that to be in sync with the > wait_event_* interface. > > Returning 1 when a function with a zero timeout succeeds is actually > quite common in the kernel. We actually had this conversation last time, and outside of that it isn't. Looking at all the convolution to first return 1, then undo the damage in the caller, it looks pretty silly. -Chris
Re: [PATCH] reservation: don't wait when timeout=0
Quoting Rob Clark (2017-11-21 14:08:46) > If we are testing if a reservation object's fences have been > signaled with timeout=0 (non-blocking), we need to pass 0 for > timeout to dma_fence_wait_timeout(). > > Plus bonus spelling correction. > > Signed-off-by: Rob Clark > --- > drivers/dma-buf/reservation.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > index dec3a815455d..71f51140a9ad 100644 > --- a/drivers/dma-buf/reservation.c > +++ b/drivers/dma-buf/reservation.c > @@ -420,7 +420,7 @@ EXPORT_SYMBOL_GPL(reservation_object_get_fences_rcu); > * > * RETURNS > * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or > - * greater than zer on success. > + * greater than zero on success. > */ > long reservation_object_wait_timeout_rcu(struct reservation_object *obj, > bool wait_all, bool intr, > @@ -483,7 +483,14 @@ long reservation_object_wait_timeout_rcu(struct > reservation_object *obj, > goto retry; > } > > - ret = dma_fence_wait_timeout(fence, intr, ret); > + /* > +* Note that dma_fence_wait_timeout() will return 1 if > +* the fence is already signaled, so in the wait_all > +* case when we go through the retry loop again, ret > +* will be greater than 0 and we don't want this to > +* cause _wait_timeout() to block > +*/ > + ret = dma_fence_wait_timeout(fence, intr, timeout ? ret : 0); One should ask if we should just fix the interface to stop returning incorrect results (stop "correcting" a completion with 0 jiffies remaining as 1). A timeout can be distinguished by -ETIME (or your pick of errno). -Chris
Re: [PATCH] dma-buf: remove redundant initialization of sg_table
Quoting Colin King (2017-09-15 00:05:16) > From: Colin Ian King > > sg_table is being initialized and is never read before it is updated > again later on, hence making the initialization redundant. Remove > the initialization. > > Detected by clang scan-build: > "warning: Value stored to 'sg_table' during its initialization is > never read" > > Signed-off-by: Colin Ian King Reviewed-by: Chris Wilson -Chris
Re: [PATCH] dma-fence: fix dma_fence_get_rcu_safe
Quoting Christian König (2017-09-11 10:57:57) > Am 11.09.2017 um 11:23 schrieb Chris Wilson: > > Quoting Christian König (2017-09-11 10:06:50) > >> Am 11.09.2017 um 10:59 schrieb Chris Wilson: > >>> Quoting Christian König (2017-09-11 09:50:40) > >>>> Sorry for the delayed response, but your mail somehow ended up in the > >>>> Spam folder. > >>>> > >>>> Am 04.09.2017 um 15:40 schrieb Chris Wilson: > >>>>> Quoting Christian König (2017-09-04 14:27:33) > >>>>>> From: Christian König > >>>>>> > >>>>>> The logic is buggy and unnecessary complex. When dma_fence_get_rcu() > >>>>>> fails to > >>>>>> acquire a reference it doesn't necessary mean that there is no fence > >>>>>> at all. > >>>>>> > >>>>>> It usually mean that the fence was replaced by a new one and in this > >>>>>> situation > >>>>>> we certainly want to have the new one as result and *NOT* NULL. > >>>>> Which is not guaranteed by the code you wrote either. > >>>>> > >>>>> The point of the comment is that the mb is only inside the successful > >>>>> kref_atomic_inc_unless_zero, and that only after that mb do you know > >>>>> whether or not you have the current fence. > >>>>> > >>>>> You can argue that you want to replace the > >>>>> if (!dma_fence_get_rcu()) > >>>>> return NULL > >>>>> with > >>>>> if (!dma_fence_get_rcu() > >>>>> continue; > >>>>> but it would be incorrect to say that by simply ignoring the > >>>>> post-condition check that you do have the right fence. > >>>> You are completely missing the point here. > >>>> > >>>> It is irrelevant if you have the current fence or not when you return. > >>>> You can only guarantee that it is the current fence when you take a look > >>>> and that is exactly what we want to avoid. > >>>> > >>>> So the existing code is complete nonsense. Instead what we need to > >>>> guarantee is that we return *ANY* fence which we can grab a reference > >>>> for. > >>> Not quite. We can grab a reference on a fence that was already freed and > >>> reused between the rcu_dereference() and dma_fence_get_rcu(). > >> Reusing a memory structure before the RCU grace period is completed is > >> illegal, otherwise the whole RCU approach won't work. > > RCU only protects that the pointer remains valid. If you use > > SLAB_TYPESAFE_BY_RCU, it is possible to reuse the pointer within a grace > > period. It does happen and that is the point the comment is trying to > > make. > > Yeah, but that is illegal with a fence objects. > > When anybody allocates fences this way it breaks at least > reservation_object_get_fences_rcu(), > reservation_object_wait_timeout_rcu() and > reservation_object_test_signaled_single(). Many, many months ago I sent patches to fix them all. -Chris
Re: [PATCH] dma-fence: fix dma_fence_get_rcu_safe
Quoting Christian König (2017-09-11 10:06:50) > Am 11.09.2017 um 10:59 schrieb Chris Wilson: > > Quoting Christian König (2017-09-11 09:50:40) > >> Sorry for the delayed response, but your mail somehow ended up in the > >> Spam folder. > >> > >> Am 04.09.2017 um 15:40 schrieb Chris Wilson: > >>> Quoting Christian König (2017-09-04 14:27:33) > >>>> From: Christian König > >>>> > >>>> The logic is buggy and unnecessary complex. When dma_fence_get_rcu() > >>>> fails to > >>>> acquire a reference it doesn't necessary mean that there is no fence at > >>>> all. > >>>> > >>>> It usually mean that the fence was replaced by a new one and in this > >>>> situation > >>>> we certainly want to have the new one as result and *NOT* NULL. > >>> Which is not guaranteed by the code you wrote either. > >>> > >>> The point of the comment is that the mb is only inside the successful > >>> kref_atomic_inc_unless_zero, and that only after that mb do you know > >>> whether or not you have the current fence. > >>> > >>> You can argue that you want to replace the > >>>if (!dma_fence_get_rcu()) > >>>return NULL > >>> with > >>>if (!dma_fence_get_rcu() > >>>continue; > >>> but it would be incorrect to say that by simply ignoring the > >>> post-condition check that you do have the right fence. > >> You are completely missing the point here. > >> > >> It is irrelevant if you have the current fence or not when you return. > >> You can only guarantee that it is the current fence when you take a look > >> and that is exactly what we want to avoid. > >> > >> So the existing code is complete nonsense. Instead what we need to > >> guarantee is that we return *ANY* fence which we can grab a reference for. > > Not quite. We can grab a reference on a fence that was already freed and > > reused between the rcu_dereference() and dma_fence_get_rcu(). > > Reusing a memory structure before the RCU grace period is completed is > illegal, otherwise the whole RCU approach won't work. RCU only protects that the pointer remains valid. If you use SLAB_TYPESAFE_BY_RCU, it is possible to reuse the pointer within a grace period. It does happen and that is the point the comment is trying to make. -Chris
Re: [PATCH] dma-fence: fix dma_fence_get_rcu_safe
Quoting Christian König (2017-09-11 09:50:40) > Sorry for the delayed response, but your mail somehow ended up in the > Spam folder. > > Am 04.09.2017 um 15:40 schrieb Chris Wilson: > > Quoting Christian König (2017-09-04 14:27:33) > >> From: Christian König > >> > >> The logic is buggy and unnecessary complex. When dma_fence_get_rcu() fails > >> to > >> acquire a reference it doesn't necessary mean that there is no fence at > >> all. > >> > >> It usually mean that the fence was replaced by a new one and in this > >> situation > >> we certainly want to have the new one as result and *NOT* NULL. > > Which is not guaranteed by the code you wrote either. > > > > The point of the comment is that the mb is only inside the successful > > kref_atomic_inc_unless_zero, and that only after that mb do you know > > whether or not you have the current fence. > > > > You can argue that you want to replace the > > if (!dma_fence_get_rcu()) > > return NULL > > with > > if (!dma_fence_get_rcu() > > continue; > > but it would be incorrect to say that by simply ignoring the > > post-condition check that you do have the right fence. > > You are completely missing the point here. > > It is irrelevant if you have the current fence or not when you return. > You can only guarantee that it is the current fence when you take a look > and that is exactly what we want to avoid. > > So the existing code is complete nonsense. Instead what we need to > guarantee is that we return *ANY* fence which we can grab a reference for. Not quite. We can grab a reference on a fence that was already freed and reused between the rcu_dereference() and dma_fence_get_rcu(). -Chris
Re: [PATCH] dma-fence: fix dma_fence_get_rcu_safe
Quoting Christian König (2017-09-04 14:27:33) > From: Christian König > > The logic is buggy and unnecessary complex. When dma_fence_get_rcu() fails to > acquire a reference it doesn't necessary mean that there is no fence at all. > > It usually mean that the fence was replaced by a new one and in this situation > we certainly want to have the new one as result and *NOT* NULL. Which is not guaranteed by the code you wrote either. The point of the comment is that the mb is only inside the successful kref_atomic_inc_unless_zero, and that only after that mb do you know whether or not you have the current fence. You can argue that you want to replace the if (!dma_fence_get_rcu()) return NULL with if (!dma_fence_get_rcu() continue; but it would be incorrect to say that by simply ignoring the post-condition check that you do have the right fence. -Chris
Re: [PATCH] dma-buf/sw_sync: Fix timeline/pt overflow cases
Quoting Sean Paul (2017-06-28 16:51:11)
> Protect against long-running processes from overflowing the timeline
> and creating fences that go back in time. While we're at it, avoid
> overflowing while we're incrementing the timeline.
>
> Signed-off-by: Sean Paul
> ---
> drivers/dma-buf/sw_sync.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 69c5ff36e2f9..40934619ed88 100644
> --- a/drivers/dma-buf/sw_sync.c
> +++ b/drivers/dma-buf/sw_sync.c
> @@ -142,7 +142,7 @@ static void sync_timeline_signal(struct sync_timeline
> *obj, unsigned int inc)
>
> spin_lock_irqsave(&obj->child_list_lock, flags);
>
> - obj->value += inc;
> + obj->value += min(inc, ~0x0U - obj->value);
The timeline uses u32 seqno, so just obj->value += min(inc, INT_MAX);
Better of course would be to report the error,
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 69c5ff36e2f9..2503cf884018 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -345,6 +345,9 @@ static long sw_sync_ioctl_inc(struct sync_timeline *obj,
unsigned long arg)
if (copy_from_user(&value, (void __user *)arg, sizeof(value)))
return -EFAULT;
+ if (value > INT_MAX)
+ return -EINVAL;
+
sync_timeline_signal(obj, value);
>
> list_for_each_entry_safe(pt, next, &obj->active_list_head,
> active_list) {
> @@ -178,6 +178,11 @@ static struct sync_pt *sync_pt_create(struct
> sync_timeline *obj, int size,
> return NULL;
>
> spin_lock_irqsave(&obj->child_list_lock, flags);
> + if (value < obj->value) {
> + spin_unlock_irqrestore(&obj->child_list_lock, flags);
> + return NULL;
> + }
Needs a u32 check. if ((int)(value - obj->value) < 0) return some_error;
-Chris
Re: [PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
On Mon, Dec 19, 2016 at 10:40:41AM +0900, Inki Dae wrote:
>
>
> 2016년 08월 16일 01:02에 Daniel Vetter 이(가) 쓴 글:
> > On Mon, Aug 15, 2016 at 04:42:18PM +0100, Chris Wilson wrote:
> >> Rendering operations to the dma-buf are tracked implicitly via the
> >> reservation_object (dmabuf->resv). This is used to allow poll() to
> >> wait upon outstanding rendering (or just query the current status of
> >> rendering). The dma-buf sync ioctl allows userspace to prepare the
> >> dma-buf for CPU access, which should include waiting upon rendering.
> >> (Some drivers may need to do more work to ensure that the dma-buf mmap
> >> is coherent as well as complete.)
> >>
> >> v2: Always wait upon the reservation object implicitly. We choose to do
> >> it after the native handler in case it can do so more efficiently.
> >>
> >> Testcase: igt/prime_vgem
> >> Testcase: igt/gem_concurrent_blit # *vgem*
> >> Signed-off-by: Chris Wilson
> >> Cc: Sumit Semwal
> >> Cc: Daniel Vetter
> >> Cc: Eric Anholt
> >> Cc: linux-media@vger.kernel.org
> >> Cc: dri-de...@lists.freedesktop.org
> >> Cc: linaro-mm-...@lists.linaro.org
> >> Cc: linux-ker...@vger.kernel.org
> >> ---
> >> drivers/dma-buf/dma-buf.c | 23 +++
> >> 1 file changed, 23 insertions(+)
> >>
> >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> >> index ddaee60ae52a..cf04d249a6a4 100644
> >> --- a/drivers/dma-buf/dma-buf.c
> >> +++ b/drivers/dma-buf/dma-buf.c
> >> @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct
> >> dma_buf_attachment *attach,
> >> }
> >> EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
> >>
> >> +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >> +enum dma_data_direction direction)
> >> +{
> >> + bool write = (direction == DMA_BIDIRECTIONAL ||
> >> +direction == DMA_TO_DEVICE);
> >> + struct reservation_object *resv = dmabuf->resv;
> >> + long ret;
> >> +
> >> + /* Wait on any implicit rendering fences */
> >> + ret = reservation_object_wait_timeout_rcu(resv, write, true,
> >> +MAX_SCHEDULE_TIMEOUT);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + return 0;
> >> +}
> >>
> >> /**
> >> * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf
> >> from the
> >> @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >>if (dmabuf->ops->begin_cpu_access)
> >>ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
> >>
> >> + /* Ensure that all fences are waited upon - but we first allow
> >> + * the native handler the chance to do so more efficiently if it
> >> + * chooses. A double invocation here will be reasonably cheap no-op.
> >> + */
> >> + if (ret == 0)
> >> + ret = __dma_buf_begin_cpu_access(dmabuf, direction);
> >
> > Not sure we should wait first and the flush or the other way round. But I
> > don't think it'll matter for any current dma-buf exporter, so meh.
> >
>
> Sorry for late comment. I wonder there is no problem in case that GPU or
> other DMA device tries to access this dma buffer after
> dma_buf_begin_cpu_access call.
> I think in this case, they - GPU or DMA devices - would make a mess of the
> dma buffer while CPU is accessing the buffer.
>
> This patch is in mainline already so if this is real problem then I think we
> sould choose,
> 1. revert this patch from mainline
That scenario is irrespective of this patch. It just depends on there
being concurrent CPU access with destructive DMA access (or vice-versa).
> 2. make sure to prevent other DMA devices to try to access the buffer while
> CPU is accessing the buffer.
Is the safeguard you want, and the one employed elsewhere, which you could
accomplish by adding a fence to the reservation object for the CPU access
in begin_access and signaling from end_access. It would need to be an
autosignaled fence because userspace may forget to end its access
(or otherwise be terminated whilst holding the fence).
Everyone using the mmap without begin/end can of course still reek havoc
on the buffer.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH 1/4] lib/scatterlist: Fix offset type in sg_alloc_table_from_pages
On Fri, Nov 11, 2016 at 08:50:17AM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Scatterlist entries have an unsigned int for the offset so > correct the sg_alloc_table_from_pages function accordingly. > > Since these are offsets withing a page, unsigned int is > wide enough. > > Also converts callers which were using unsigned long locally > with the lower_32_bits annotation to make it explicitly > clear what is happening. > > v2: Use offset_in_page. (Chris Wilson) > > Signed-off-by: Tvrtko Ursulin > Cc: Masahiro Yamada > Cc: Pawel Osciak > Cc: Marek Szyprowski > Cc: Kyungmin Park > Cc: Tomasz Stanislawski > Cc: Matt Porter > Cc: Alexandre Bounine > Cc: linux-media@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > Acked-by: Marek Szyprowski (v1) If there were kerneldoc, it would nicely explain that having an offset larger then a page is silly when passing in array of pages. Changes elsewhere look ok (personally I'd be happy with just offset_in_page(), 4GiB superpages are somebody else's problem :) Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages for allocating object backing store
On Fri, Oct 21, 2016 at 03:11:22PM +0100, Tvrtko Ursulin wrote:
> @@ -2236,18 +2233,16 @@ i915_gem_object_get_pages_gtt(struct
> drm_i915_gem_object *obj)
> BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS);
> BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS);
>
> - max_segment = swiotlb_max_size();
> - if (!max_segment)
> - max_segment = rounddown(UINT_MAX, PAGE_SIZE);
> -
> - st = kmalloc(sizeof(*st), GFP_KERNEL);
> - if (st == NULL)
> - return -ENOMEM;
> -
> page_count = obj->base.size / PAGE_SIZE;
> - if (sg_alloc_table(st, page_count, GFP_KERNEL)) {
> - kfree(st);
> + pages = drm_malloc_gfp(page_count, sizeof(struct page *),
> +GFP_TEMPORARY | __GFP_ZERO);
> + if (!pages)
> return -ENOMEM;
Full circle! The whole reason this exists was to avoid that vmalloc. I
don't really want it back...
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH 11/11] dma-buf: Do a fast lockless check for poll with timeout=0
On Fri, Sep 23, 2016 at 03:50:44PM +0200, Daniel Vetter wrote: > On Mon, Aug 29, 2016 at 08:08:34AM +0100, Chris Wilson wrote: > > Currently we install a callback for performing poll on a dma-buf, > > irrespective of the timeout. This involves taking a spinlock, as well as > > unnecessary work, and greatly reduces scaling of poll(.timeout=0) across > > multiple threads. > > > > We can query whether the poll will block prior to installing the > > callback to make the busy-query fast. > > > > Single thread: 60% faster > > 8 threads on 4 (+4 HT) cores: 600% faster > > > > Still not quite the perfect scaling we get with a native busy ioctl, but > > poll(dmabuf) is faster due to the quicker lookup of the object and > > avoiding drm_ioctl(). > > > > Signed-off-by: Chris Wilson > > Cc: Sumit Semwal > > Cc: linux-media@vger.kernel.org > > Cc: dri-de...@lists.freedesktop.org > > Cc: linaro-mm-...@lists.linaro.org > > Reviewed-by: Daniel Vetter > > Need to strike the r-b here, since Christian König pointed out that > objects won't magically switch signalling on. Oh, it also means that commit fb8b7d2b9d80e1e71f379e57355936bd2b024be9 Author: Jammy Zhou Date: Wed Jan 21 18:35:47 2015 +0800 reservation: wait only with non-zero timeout specified (v3) When the timeout value passed to reservation_object_wait_timeout_rcu is zero, no wait should be done if the fences are not signaled. Return '1' for idle and '0' for busy if the specified timeout is '0' to keep consistent with the case of non-zero timeout. v2: call fence_put if not signaled in the case of timeout==0 v3: switch to reservation_object_test_signaled_rcu Signed-off-by: Jammy Zhou Reviewed-by: Christian König Reviewed-by: Alex Deucher Reviewed-By: Maarten Lankhorst Signed-off-by: Sumit Semwal is wrong. And reservation_object_test_signaled_rcu() is unreliable. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH 11/11] dma-buf: Do a fast lockless check for poll with timeout=0
On Fri, Sep 23, 2016 at 03:50:44PM +0200, Daniel Vetter wrote: > On Mon, Aug 29, 2016 at 08:08:34AM +0100, Chris Wilson wrote: > > Currently we install a callback for performing poll on a dma-buf, > > irrespective of the timeout. This involves taking a spinlock, as well as > > unnecessary work, and greatly reduces scaling of poll(.timeout=0) across > > multiple threads. > > > > We can query whether the poll will block prior to installing the > > callback to make the busy-query fast. > > > > Single thread: 60% faster > > 8 threads on 4 (+4 HT) cores: 600% faster > > > > Still not quite the perfect scaling we get with a native busy ioctl, but > > poll(dmabuf) is faster due to the quicker lookup of the object and > > avoiding drm_ioctl(). > > > > Signed-off-by: Chris Wilson > > Cc: Sumit Semwal > > Cc: linux-media@vger.kernel.org > > Cc: dri-de...@lists.freedesktop.org > > Cc: linaro-mm-...@lists.linaro.org > > Reviewed-by: Daniel Vetter > > Need to strike the r-b here, since Christian König pointed out that > objects won't magically switch signalling on. Propagating a flag through to sync_file is trivial, but not through to the dma_buf->resv. Looks like dma-buf will be without a fast busy query, which I guess in the grand scheme of things (i.e. dma-buf itself is not intended to be used as a fence) is not that important. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [PATCH 11/11] dma-buf: Do a fast lockless check for poll with timeout=0
On Fri, Sep 23, 2016 at 03:50:44PM +0200, Daniel Vetter wrote: > On Mon, Aug 29, 2016 at 08:08:34AM +0100, Chris Wilson wrote: > > Currently we install a callback for performing poll on a dma-buf, > > irrespective of the timeout. This involves taking a spinlock, as well as > > unnecessary work, and greatly reduces scaling of poll(.timeout=0) across > > multiple threads. > > > > We can query whether the poll will block prior to installing the > > callback to make the busy-query fast. > > > > Single thread: 60% faster > > 8 threads on 4 (+4 HT) cores: 600% faster > > > > Still not quite the perfect scaling we get with a native busy ioctl, but > > poll(dmabuf) is faster due to the quicker lookup of the object and > > avoiding drm_ioctl(). > > > > Signed-off-by: Chris Wilson > > Cc: Sumit Semwal > > Cc: linux-media@vger.kernel.org > > Cc: dri-de...@lists.freedesktop.org > > Cc: linaro-mm-...@lists.linaro.org > > Reviewed-by: Daniel Vetter > > Need to strike the r-b here, since Christian König pointed out that > objects won't magically switch signalling on. The point being here that we don't even want to switch signaling on! :) Christian's point was that not all fences guarantee forward progress irrespective of whether signaling is enabled or not, and fences are not required to guarantee forward progress without signaling even if they provide an ops->signaled(). -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [PATCH 10/11] dma-buf: Use seqlock to close RCU race in test_signaled_single
On Fri, Sep 23, 2016 at 03:49:26PM +0200, Daniel Vetter wrote: > On Mon, Aug 29, 2016 at 08:08:33AM +0100, Chris Wilson wrote: > > With the seqlock now extended to cover the lookup of the fence and its > > testing, we can perform that testing solely under the seqlock guard and > > avoid the effective locking and serialisation of acquiring a reference to > > the request. As the fence is RCU protected we know it cannot disappear > > as we test it, the same guarantee that made it safe to acquire the > > reference previously. The seqlock tests whether the fence was replaced > > as we are testing it telling us whether or not we can trust the result > > (if not, we just repeat the test until stable). > > > > Signed-off-by: Chris Wilson > > Cc: Sumit Semwal > > Cc: linux-media@vger.kernel.org > > Cc: dri-de...@lists.freedesktop.org > > Cc: linaro-mm-...@lists.linaro.org > > Not entirely sure this is safe for non-i915 drivers. We might now call > ->signalled on a zombie fence (i.e. refcount == 0, but not yet kfreed). > i915 can do that, but other drivers might go boom. All fences must be under RCU guard, or is that the sticking point? Given that, the problem is fence reallocation within the RCU grace period. If we can mandate that fence reallocation must be safe for concurrent fence->ops->*(), we can use this technique to avoid the serialisation barrier otherwise required. In the simple stress test, the difference is an order of magnitude, and test_signaled_rcu is often on a path where every memory barrier quickly adds up (at least for us). So is it just that you worry that others using SLAB_DESTROY_BY_RCU won't ensure their fence is safe during the reallocation? -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dma-buf/sync-file: Avoid enable fence signaling if poll(.timeout=0)
If we being polled with a timeout of zero, a nonblocking busy query,
we don't need to install any fence callbacks as we will not be waiting.
As we only install the callback once, the overhead comes from the atomic
bit test that also causes serialisation between threads.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Gustavo Padovan
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/dma-buf/sync_file.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 486d29c1a830..abb5fdab75fd 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -306,7 +306,8 @@ static unsigned int sync_file_poll(struct file *file,
poll_table *wait)
poll_wait(file, &sync_file->wq, wait);
- if (!test_and_set_bit(POLL_ENABLED, &sync_file->fence->flags)) {
+ if (!poll_does_not_wait(wait) &&
+ !test_and_set_bit(POLL_ENABLED, &sync_file->fence->flags)) {
if (fence_add_callback(sync_file->fence, &sync_file->cb,
fence_check_cb_func) < 0)
wake_up_all(&sync_file->wq);
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/11] dma-buf: Do a fast lockless check for poll with timeout=0
Currently we install a callback for performing poll on a dma-buf,
irrespective of the timeout. This involves taking a spinlock, as well as
unnecessary work, and greatly reduces scaling of poll(.timeout=0) across
multiple threads.
We can query whether the poll will block prior to installing the
callback to make the busy-query fast.
Single thread: 60% faster
8 threads on 4 (+4 HT) cores: 600% faster
Still not quite the perfect scaling we get with a native busy ioctl, but
poll(dmabuf) is faster due to the quicker lookup of the object and
avoiding drm_ioctl().
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Reviewed-by: Daniel Vetter
---
drivers/dma-buf/dma-buf.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index cf04d249a6a4..c7a7bc579941 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -156,6 +156,18 @@ static unsigned int dma_buf_poll(struct file *file,
poll_table *poll)
if (!events)
return 0;
+ if (poll_does_not_wait(poll)) {
+ if (events & POLLOUT &&
+ !reservation_object_test_signaled_rcu(resv, true))
+ events &= ~(POLLOUT | POLLIN);
+
+ if (events & POLLIN &&
+ !reservation_object_test_signaled_rcu(resv, false))
+ events &= ~POLLIN;
+
+ return events;
+ }
+
retry:
seq = read_seqcount_begin(&resv->seq);
rcu_read_lock();
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/11] dma-buf: Introduce fence_get_rcu_safe()
This variant of fence_get_rcu() takes an RCU protected pointer to a
fence and carefully returns a reference to the fence ensuring that it is
not reallocated as it does. This is required when mixing fences and
SLAB_DESTROY_BY_RCU - although it serves a more pedagogical function atm
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
Cc: Sumit Semwal
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/fence.h | 56 ++-
1 file changed, 51 insertions(+), 5 deletions(-)
diff --git a/include/linux/fence.h b/include/linux/fence.h
index 0d763053f97a..c9c5ba98c302 100644
--- a/include/linux/fence.h
+++ b/include/linux/fence.h
@@ -183,6 +183,16 @@ void fence_release(struct kref *kref);
void fence_free(struct fence *fence);
/**
+ * fence_put - decreases refcount of the fence
+ * @fence: [in]fence to reduce refcount of
+ */
+static inline void fence_put(struct fence *fence)
+{
+ if (fence)
+ kref_put(&fence->refcount, fence_release);
+}
+
+/**
* fence_get - increases refcount of the fence
* @fence: [in]fence to increase refcount of
*
@@ -210,13 +220,49 @@ static inline struct fence *fence_get_rcu(struct fence
*fence)
}
/**
- * fence_put - decreases refcount of the fence
- * @fence: [in]fence to reduce refcount of
+ * fence_get_rcu_safe - acquire a reference to an RCU tracked fence
+ * @fence: [in]pointer to fence to increase refcount of
+ *
+ * Function returns NULL if no refcount could be obtained, or the fence.
+ * This function handles acquiring a reference to a fence that may be
+ * reallocated within the RCU grace period (such as with SLAB_DESTROY_BY_RCU),
+ * so long as the caller is using RCU on the pointer to the fence.
+ *
+ * An alternative mechanism is to employ a seqlock to protect a bunch of
+ * fences, such as used by struct reservation_object. When using a seqlock,
+ * the seqlock must be taken before and checked after a reference to the
+ * fence is acquired (as shown here).
+ *
+ * The caller is required to hold the RCU read lock.
*/
-static inline void fence_put(struct fence *fence)
+static inline struct fence *fence_get_rcu_safe(struct fence * __rcu *fencep)
{
- if (fence)
- kref_put(&fence->refcount, fence_release);
+ do {
+ struct fence *fence;
+
+ fence = rcu_dereference(*fencep);
+ if (!fence || !fence_get_rcu(fence))
+ return NULL;
+
+ /* The atomic_inc_not_zero() inside fence_get_rcu()
+* provides a full memory barrier upon success (such as now).
+* This is paired with the write barrier from assigning
+* to the __rcu protected fence pointer so that if that
+* pointer still matches the current fence, we know we
+* have successfully acquire a reference to it. If it no
+* longer matches, we are holding a reference to some other
+* reallocated pointer. This is possible if the allocator
+* is using a freelist like SLAB_DESTROY_BY_RCU where the
+* fence remains valid for the RCU grace period, but it
+* may be reallocated. When using such allocators, we are
+* responsible for ensuring the reference we get is to
+* the right fence, as below.
+*/
+ if (fence == rcu_access_pointer(*fencep))
+ return rcu_pointer_handoff(fence);
+
+ fence_put(fence);
+ } while (1);
}
int fence_signal(struct fence *fence);
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/11] dma-buf: Restart reservation_object_test_signaled_rcu() after writes
In order to be completely generic, we have to double check the read
seqlock after acquiring a reference to the fence. If the driver is
allocating fences from a SLAB_DESTROY_BY_RCU, or similar freelist, then
within an RCU grace period a fence may be freed and reallocated. The RCU
read side critical section does not prevent this reallocation, instead
we have to inspect the reservation's seqlock to double check if the
fences have been reassigned as we were acquiring our reference.
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
Cc: Maarten Lankhorst
Cc: Christian König
Cc: Alex Deucher
Cc: Sumit Semwal
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/dma-buf/reservation.c | 30 ++
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 3369e4668e96..e74493e7332b 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -474,12 +474,13 @@ bool reservation_object_test_signaled_rcu(struct
reservation_object *obj,
bool test_all)
{
unsigned seq, shared_count;
- int ret = true;
+ int ret;
+ rcu_read_lock();
retry:
+ ret = true;
shared_count = 0;
seq = read_seqcount_begin(&obj->seq);
- rcu_read_lock();
if (test_all) {
unsigned i;
@@ -490,46 +491,35 @@ retry:
if (fobj)
shared_count = fobj->shared_count;
- if (read_seqcount_retry(&obj->seq, seq))
- goto unlock_retry;
-
for (i = 0; i < shared_count; ++i) {
struct fence *fence = rcu_dereference(fobj->shared[i]);
ret = reservation_object_test_signaled_single(fence);
if (ret < 0)
- goto unlock_retry;
+ goto retry;
else if (!ret)
break;
}
- /*
-* There could be a read_seqcount_retry here, but nothing cares
-* about whether it's the old or newer fence pointers that are
-* signaled. That race could still have happened after checking
-* read_seqcount_retry. If you care, use ww_mutex_lock.
-*/
+ if (read_seqcount_retry(&obj->seq, seq))
+ goto retry;
}
if (!shared_count) {
struct fence *fence_excl = rcu_dereference(obj->fence_excl);
- if (read_seqcount_retry(&obj->seq, seq))
- goto unlock_retry;
-
if (fence_excl) {
ret = reservation_object_test_signaled_single(
fence_excl);
if (ret < 0)
- goto unlock_retry;
+ goto retry;
+
+ if (read_seqcount_retry(&obj->seq, seq))
+ goto retry;
}
}
rcu_read_unlock();
return ret;
-
-unlock_retry:
- rcu_read_unlock();
- goto retry;
}
EXPORT_SYMBOL_GPL(reservation_object_test_signaled_rcu);
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/11] dma-buf: Restart reservation_object_wait_timeout_rcu() after writes
In order to be completely generic, we have to double check the read
seqlock after acquiring a reference to the fence. If the driver is
allocating fences from a SLAB_DESTROY_BY_RCU, or similar freelist, then
within an RCU grace period a fence may be freed and reallocated. The RCU
read side critical section does not prevent this reallocation, instead
we have to inspect the reservation's seqlock to double check if the
fences have been reassigned as we were acquiring our reference.
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
Cc: Maarten Lankhorst
Cc: Christian König
Cc: Alex Deucher
Cc: Sumit Semwal
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/dma-buf/reservation.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 10fd441dd4ed..3369e4668e96 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -388,9 +388,6 @@ retry:
if (fobj)
shared_count = fobj->shared_count;
- if (read_seqcount_retry(&obj->seq, seq))
- goto unlock_retry;
-
for (i = 0; i < shared_count; ++i) {
struct fence *lfence = rcu_dereference(fobj->shared[i]);
@@ -413,9 +410,6 @@ retry:
if (!shared_count) {
struct fence *fence_excl = rcu_dereference(obj->fence_excl);
- if (read_seqcount_retry(&obj->seq, seq))
- goto unlock_retry;
-
if (fence_excl &&
!test_bit(FENCE_FLAG_SIGNALED_BIT, &fence_excl->flags)) {
if (!fence_get_rcu(fence_excl))
@@ -430,6 +424,11 @@ retry:
rcu_read_unlock();
if (fence) {
+ if (read_seqcount_retry(&obj->seq, seq)) {
+ fence_put(fence);
+ goto retry;
+ }
+
ret = fence_wait_timeout(fence, intr, ret);
fence_put(fence);
if (ret > 0 && wait_all && (i + 1 < shared_count))
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/11] dma-buf: Restart reservation_object_get_fences_rcu() after writes
In order to be completely generic, we have to double check the read
seqlock after acquiring a reference to the fence. If the driver is
allocating fences from a SLAB_DESTROY_BY_RCU, or similar freelist, then
within an RCU grace period a fence may be freed and reallocated. The RCU
read side critical section does not prevent this reallocation, instead
we have to inspect the reservation's seqlock to double check if the
fences have been reassigned as we were acquiring our reference.
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
Cc: Maarten Lankhorst
Cc: Christian König
Cc: Alex Deucher
Cc: Sumit Semwal
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/dma-buf/reservation.c | 71 +++
1 file changed, 31 insertions(+), 40 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 723d8af988e5..10fd441dd4ed 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -280,18 +280,24 @@ int reservation_object_get_fences_rcu(struct
reservation_object *obj,
unsigned *pshared_count,
struct fence ***pshared)
{
- unsigned shared_count = 0;
- unsigned retry = 1;
- struct fence **shared = NULL, *fence_excl = NULL;
- int ret = 0;
+ struct fence **shared = NULL;
+ struct fence *fence_excl;
+ unsigned shared_count;
+ int ret = 1;
- while (retry) {
+ do {
struct reservation_object_list *fobj;
unsigned seq;
+ unsigned i;
- seq = read_seqcount_begin(&obj->seq);
+ shared_count = i = 0;
rcu_read_lock();
+ seq = read_seqcount_begin(&obj->seq);
+
+ fence_excl = rcu_dereference(obj->fence_excl);
+ if (fence_excl && !fence_get_rcu(fence_excl))
+ goto unlock;
fobj = rcu_dereference(obj->fence);
if (fobj) {
@@ -309,52 +315,37 @@ int reservation_object_get_fences_rcu(struct
reservation_object *obj,
}
ret = -ENOMEM;
- shared_count = 0;
break;
}
shared = nshared;
- memcpy(shared, fobj->shared, sz);
shared_count = fobj->shared_count;
- } else
- shared_count = 0;
- fence_excl = rcu_dereference(obj->fence_excl);
-
- retry = read_seqcount_retry(&obj->seq, seq);
- if (retry)
- goto unlock;
-
- if (!fence_excl || fence_get_rcu(fence_excl)) {
- unsigned i;
for (i = 0; i < shared_count; ++i) {
- if (fence_get_rcu(shared[i]))
- continue;
-
- /* uh oh, refcount failed, abort and retry */
- while (i--)
- fence_put(shared[i]);
-
- if (fence_excl) {
- fence_put(fence_excl);
- fence_excl = NULL;
- }
-
- retry = 1;
- break;
+ shared[i] = rcu_dereference(fobj->shared[i]);
+ if (!fence_get_rcu(shared[i]))
+ break;
}
- } else
- retry = 1;
+ }
+
+ if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
+ while (i--)
+ fence_put(shared[i]);
+ fence_put(fence_excl);
+ goto unlock;
+ }
+ ret = 0;
unlock:
rcu_read_unlock();
- }
- *pshared_count = shared_count;
- if (shared_count)
- *pshared = shared;
- else {
- *pshared = NULL;
+ } while (ret);
+
+ if (!shared_count) {
kfree(shared);
+ shared = NULL;
}
+
+ *pshared_count = shared_count;
+ *pshared = shared;
*pfence_excl = fence_excl;
return ret;
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/11] dma-buf: Use seqlock to close RCU race in test_signaled_single
With the seqlock now extended to cover the lookup of the fence and its
testing, we can perform that testing solely under the seqlock guard and
avoid the effective locking and serialisation of acquiring a reference to
the request. As the fence is RCU protected we know it cannot disappear
as we test it, the same guarantee that made it safe to acquire the
reference previously. The seqlock tests whether the fence was replaced
as we are testing it telling us whether or not we can trust the result
(if not, we just repeat the test until stable).
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/dma-buf/reservation.c | 32
1 file changed, 4 insertions(+), 28 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index e74493e7332b..1ddffa5adb5a 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -442,24 +442,6 @@ unlock_retry:
}
EXPORT_SYMBOL_GPL(reservation_object_wait_timeout_rcu);
-
-static inline int
-reservation_object_test_signaled_single(struct fence *passed_fence)
-{
- struct fence *fence, *lfence = passed_fence;
- int ret = 1;
-
- if (!test_bit(FENCE_FLAG_SIGNALED_BIT, &lfence->flags)) {
- fence = fence_get_rcu(lfence);
- if (!fence)
- return -1;
-
- ret = !!fence_is_signaled(fence);
- fence_put(fence);
- }
- return ret;
-}
-
/**
* reservation_object_test_signaled_rcu - Test if a reservation object's
* fences have been signaled.
@@ -474,7 +456,7 @@ bool reservation_object_test_signaled_rcu(struct
reservation_object *obj,
bool test_all)
{
unsigned seq, shared_count;
- int ret;
+ bool ret;
rcu_read_lock();
retry:
@@ -494,10 +476,8 @@ retry:
for (i = 0; i < shared_count; ++i) {
struct fence *fence = rcu_dereference(fobj->shared[i]);
- ret = reservation_object_test_signaled_single(fence);
- if (ret < 0)
- goto retry;
- else if (!ret)
+ ret = fence_is_signaled(fence);
+ if (!ret)
break;
}
@@ -509,11 +489,7 @@ retry:
struct fence *fence_excl = rcu_dereference(obj->fence_excl);
if (fence_excl) {
- ret = reservation_object_test_signaled_single(
- fence_excl);
- if (ret < 0)
- goto retry;
-
+ ret = fence_is_signaled(fence_excl);
if (read_seqcount_retry(&obj->seq, seq))
goto retry;
}
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma-buf: Do a fast lockless check for poll with timeout=0
On Sun, Aug 28, 2016 at 09:33:54PM +0100, Chris Wilson wrote: > On Sun, Aug 28, 2016 at 05:37:47PM +0100, Chris Wilson wrote: > > Currently we install a callback for performing poll on a dma-buf, > > irrespective of the timeout. This involves taking a spinlock, as well as > > unnecessary work, and greatly reduces scaling of poll(.timeout=0) across > > multiple threads. > > > > We can query whether the poll will block prior to installing the > > callback to make the busy-query fast. > > > > Single thread: 60% faster > > 8 threads on 4 (+4 HT) cores: 600% faster > > Hmm, this only really applies to the idle case. > reservation_object_test_signaled_rcu() is still a major bottleneck when > busy, due to the dance inside reservation_object_test_signaled_single() The fix is not difficult, just requires extending the seqlock to catch the RCU race (i.e. earlier patches). I'll resend that series in the morning. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma-buf: Do a fast lockless check for poll with timeout=0
On Sun, Aug 28, 2016 at 05:37:47PM +0100, Chris Wilson wrote: > Currently we install a callback for performing poll on a dma-buf, > irrespective of the timeout. This involves taking a spinlock, as well as > unnecessary work, and greatly reduces scaling of poll(.timeout=0) across > multiple threads. > > We can query whether the poll will block prior to installing the > callback to make the busy-query fast. > > Single thread: 60% faster > 8 threads on 4 (+4 HT) cores: 600% faster Hmm, this only really applies to the idle case. reservation_object_test_signaled_rcu() is still a major bottleneck when busy, due to the dance inside reservation_object_test_signaled_single() -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dma-buf: Do a fast lockless check for poll with timeout=0
Currently we install a callback for performing poll on a dma-buf,
irrespective of the timeout. This involves taking a spinlock, as well as
unnecessary work, and greatly reduces scaling of poll(.timeout=0) across
multiple threads.
We can query whether the poll will block prior to installing the
callback to make the busy-query fast.
Single thread: 60% faster
8 threads on 4 (+4 HT) cores: 600% faster
Still not quite the perfect scaling we get with a native busy ioctl, but
poll(dmabuf) is faster due to the quicker lookup of the object and
avoiding drm_ioctl().
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/dma-buf/dma-buf.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index cf04d249a6a4..c7a7bc579941 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -156,6 +156,18 @@ static unsigned int dma_buf_poll(struct file *file,
poll_table *poll)
if (!events)
return 0;
+ if (poll_does_not_wait(poll)) {
+ if (events & POLLOUT &&
+ !reservation_object_test_signaled_rcu(resv, true))
+ events &= ~(POLLOUT | POLLIN);
+
+ if (events & POLLIN &&
+ !reservation_object_test_signaled_rcu(resv, false))
+ events &= ~POLLIN;
+
+ return events;
+ }
+
retry:
seq = read_seqcount_begin(&resv->seq);
rcu_read_lock();
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
Rendering operations to the dma-buf are tracked implicitly via the
reservation_object (dmabuf->resv). This is used to allow poll() to
wait upon outstanding rendering (or just query the current status of
rendering). The dma-buf sync ioctl allows userspace to prepare the
dma-buf for CPU access, which should include waiting upon rendering.
(Some drivers may need to do more work to ensure that the dma-buf mmap
is coherent as well as complete.)
v2: Always wait upon the reservation object implicitly. We choose to do
it after the native handler in case it can do so more efficiently.
Testcase: igt/prime_vgem
Testcase: igt/gem_concurrent_blit # *vgem*
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Daniel Vetter
Cc: Eric Anholt
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Cc: linux-ker...@vger.kernel.org
---
drivers/dma-buf/dma-buf.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ddaee60ae52a..cf04d249a6a4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment
*attach,
}
EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
+static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
+ enum dma_data_direction direction)
+{
+ bool write = (direction == DMA_BIDIRECTIONAL ||
+ direction == DMA_TO_DEVICE);
+ struct reservation_object *resv = dmabuf->resv;
+ long ret;
+
+ /* Wait on any implicit rendering fences */
+ ret = reservation_object_wait_timeout_rcu(resv, write, true,
+ MAX_SCHEDULE_TIMEOUT);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
/**
* dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from
the
@@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
if (dmabuf->ops->begin_cpu_access)
ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
+ /* Ensure that all fences are waited upon - but we first allow
+* the native handler the chance to do so more efficiently if it
+* chooses. A double invocation here will be reasonably cheap no-op.
+*/
+ if (ret == 0)
+ ret = __dma_buf_begin_cpu_access(dmabuf, direction);
+
return ret;
}
EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
--
2.8.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] dma-buf: Wait on the reservation object when sync'ing before CPU access
Rendering operations to the dma-buf are tracked implicitly via the
reservation_object (dmabuf->resv). This is used to allow poll() to
wait upon outstanding rendering (or just query the current status of
rendering). The dma-buf sync ioctl allows userspace to prepare the
dma-buf for CPU access, which should include waiting upon rendering.
(Some drivers may need to do more work to ensure that the dma-buf mmap
is coherent as well as complete.)
v2: Always wait upon the reservation object implicitly. We choose to do
it after the native handler in case it can do so more efficiently.
Testcase: igt/prime_vgem
Testcase: igt/gem_concurrent_blit # *vgem*
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Daniel Vetter
Cc: Eric Anholt
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Cc: linux-ker...@vger.kernel.org
---
drivers/dma-buf/dma-buf.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ddaee60ae52a..cf04d249a6a4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment
*attach,
}
EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
+static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
+ enum dma_data_direction direction)
+{
+ bool write = (direction == DMA_BIDIRECTIONAL ||
+ direction == DMA_TO_DEVICE);
+ struct reservation_object *resv = dmabuf->resv;
+ long ret;
+
+ /* Wait on any implicit rendering fences */
+ ret = reservation_object_wait_timeout_rcu(resv, write, true,
+ MAX_SCHEDULE_TIMEOUT);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
/**
* dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from
the
@@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
if (dmabuf->ops->begin_cpu_access)
ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
+ /* Ensure that all fences are waited upon - but we first allow
+* the native handler the chance to do so more efficiently if it
+* chooses. A double invocation here will be reasonably cheap no-op.
+*/
+ if (ret == 0)
+ ret = __dma_buf_begin_cpu_access(dmabuf, direction);
+
return ret;
}
EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
--
2.8.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dma-buf: Release module reference on creation failure
If we fail to create the anon file, we need to remember to release the
module reference on the owner.
Signed-off-by: Chris Wilson
Reviewed-by: Joonas Lahtinen
Cc: Joonas Lahtinen
Cc: Sumit Semwal
Cc: Daniel Vetter
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/dma-buf/dma-buf.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 20ce0687b111..ddaee60ae52a 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -334,6 +334,7 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
struct reservation_object *resv = exp_info->resv;
struct file *file;
size_t alloc_size = sizeof(struct dma_buf);
+ int ret;
if (!exp_info->resv)
alloc_size += sizeof(struct reservation_object);
@@ -357,8 +358,8 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
dmabuf = kzalloc(alloc_size, GFP_KERNEL);
if (!dmabuf) {
- module_put(exp_info->owner);
- return ERR_PTR(-ENOMEM);
+ ret = -ENOMEM;
+ goto err_module;
}
dmabuf->priv = exp_info->priv;
@@ -379,8 +380,8 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf,
exp_info->flags);
if (IS_ERR(file)) {
- kfree(dmabuf);
- return ERR_CAST(file);
+ ret = PTR_ERR(file);
+ goto err_dmabuf;
}
file->f_mode |= FMODE_LSEEK;
@@ -394,6 +395,12 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
mutex_unlock(&db_list.lock);
return dmabuf;
+
+err_dmabuf:
+ kfree(dmabuf);
+err_module:
+ module_put(exp_info->owner);
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(dma_buf_export);
--
2.8.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/7] kfence: Extend kfences for listening on DMA fences
dma-buf provides an interfaces for receiving notifications from DMA
hardware, and for implicitly tracking fences used for rendering into
dma-buf. We want to be able to use these event sources along with kfence
for easy collection and combining with other events.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/dma-buf/fence.c | 58 +++
drivers/dma-buf/reservation.c | 48 +++
include/linux/fence.h | 6 +
include/linux/kfence.h| 2 ++
include/linux/reservation.h | 7 ++
kernel/kfence.c | 8 ++
6 files changed, 129 insertions(+)
diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
index 7b05dbe9b296..3f06b3b1b4cc 100644
--- a/drivers/dma-buf/fence.c
+++ b/drivers/dma-buf/fence.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#define CREATE_TRACE_POINTS
#include
@@ -530,3 +531,60 @@ fence_init(struct fence *fence, const struct fence_ops
*ops,
trace_fence_init(fence);
}
EXPORT_SYMBOL(fence_init);
+
+struct dma_fence_cb {
+ struct fence_cb base;
+ struct kfence *fence;
+};
+
+static void dma_kfence_wake(struct fence *dma, struct fence_cb *data)
+{
+ struct dma_fence_cb *cb = container_of(data, typeof(*cb), base);
+
+ kfence_complete(cb->fence);
+ kfence_put(cb->fence);
+ kfree(cb);
+}
+
+/**
+ * kfence_await_dma_fence - set the fence to wait upon a DMA fence
+ * @fence: this kfence
+ * @dma: target DMA fence to wait upon
+ * @gfp: the allowed allocation type
+ *
+ * kfence_add_dma() causes the @fence to wait upon completion of a DMA fence.
+ *
+ * Returns 1 if the @fence was successfully to the waitqueue of @dma, 0
+ * if @dma was already signaled (and so not added), or a negative error code.
+ */
+int kfence_await_dma_fence(struct kfence *fence, struct fence *dma, gfp_t gfp)
+{
+ struct dma_fence_cb *cb;
+ int ret;
+
+ if (fence_is_signaled(dma))
+ return 0;
+
+ cb = kmalloc(sizeof(*cb), gfp);
+ if (!cb) {
+ if (!gfpflags_allow_blocking(gfp))
+ return -ENOMEM;
+
+ return fence_wait(dma, false);
+ }
+
+ cb->fence = kfence_get(fence);
+ kfence_await(fence);
+
+ ret = fence_add_callback(dma, &cb->base, dma_kfence_wake);
+ if (ret == 0) {
+ ret = 1;
+ } else {
+ dma_kfence_wake(dma, &cb->base);
+ if (ret == -ENOENT) /* fence already signaled */
+ ret = 0;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kfence_await_dma_fence);
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 9566a62ad8e3..138b792af0c3 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -543,3 +543,51 @@ unlock_retry:
goto retry;
}
EXPORT_SYMBOL_GPL(reservation_object_test_signaled_rcu);
+
+/**
+ * kfence_add_reservation - set the fence to wait upon a reservation_object
+ * @fence: this kfence
+ * @resv: target reservation_object (collection of DMA fences) to wait upon
+ * @write: Wait for read or read/write access
+ * @gfp: the allowed allocation type
+ *
+ * kfence_add_reservation() causes the @fence to wait upon completion of the
+ * reservation object (a collection of DMA fences), either for read access
+ * or for read/write access.
+ *
+ * Returns 1 if the @fence was successfully to the waitqueues of @resv, 0
+ * if @resev was already signaled (and so not added), or a negative error code.
+ */
+int kfence_await_reservation(struct kfence *fence,
+struct reservation_object *resv,
+bool write,
+gfp_t gfp)
+{
+ struct fence *excl, **shared;
+ unsigned int count, i;
+ int ret;
+
+ ret = reservation_object_get_fences_rcu(resv, &excl, &count, &shared);
+ if (ret)
+ return ret;
+
+ if (write) {
+ for (i = 0; i < count; i++) {
+ ret |= kfence_await_dma_fence(fence, shared[i], gfp);
+ if (ret < 0)
+ goto out;
+ }
+ }
+
+ if (excl)
+ ret |= kfence_await_dma_fence(fence, excl, gfp);
+
+out:
+ fence_put(excl);
+ for (i = 0; i &
[PATCH v2 1/7] kfence: Introduce kfence, a N:M completion mechanism
Completions are a simple synchronization mechanism, suitable for 1:M
barriers where many waiters maybe waiting for a single event. However,
some event driven mechanisms require a graph of events where one event
may depend upon several earlier events. The kfence extends the struct
completion to be able to asynchronously wait upon several event sources,
including completions and other fences forming the basis on which an
acyclic dependency graph can be built. Most often this is used to create
a set of interdependent tasks that can be run concurrently but yet
serialised where necessary. For example, the kernel init sequence has
many tasks that could be run in parallel so long as their dependencies
on previous tasks have been completed. Similarly we have the problem of
assigning interdependent tasks to multiple hardware execution engines,
be they used for rendering or for display. kfences provides a building
block which can be used for determining an order in which tasks can
execute.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/kfence.h| 64
kernel/Makefile | 2 +-
kernel/kfence.c | 431 +++
lib/Kconfig.debug | 23 ++
lib/Makefile | 1 +
lib/test-kfence.c | 536 ++
tools/testing/selftests/lib/kfence.sh | 10 +
7 files changed, 1066 insertions(+), 1 deletion(-)
create mode 100644 include/linux/kfence.h
create mode 100644 kernel/kfence.c
create mode 100644 lib/test-kfence.c
create mode 100755 tools/testing/selftests/lib/kfence.sh
diff --git a/include/linux/kfence.h b/include/linux/kfence.h
new file mode 100644
index ..6e32385b3b8c
--- /dev/null
+++ b/include/linux/kfence.h
@@ -0,0 +1,64 @@
+/*
+ * kfence.h - library routines for N:M synchronisation points
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * This file is released under the GPLv2.
+ *
+ */
+
+#ifndef _KFENCE_H_
+#define _KFENCE_H_
+
+#include
+#include
+#include /* for NOTIFY_DONE */
+#include
+
+struct completion;
+
+struct kfence {
+ wait_queue_head_t wait;
+ unsigned long flags;
+ struct kref kref;
+ atomic_t pending;
+};
+
+#define KFENCE_CHECKED_BIT 0 /* used internally for DAG checking */
+#define KFENCE_PRIVATE_BIT 1 /* available for use by owner */
+#define KFENCE_MASK(~3)
+
+#define __kfence_call __aligned(4)
+typedef int (*kfence_notify_t)(struct kfence *);
+
+void kfence_init(struct kfence *fence, kfence_notify_t fn);
+
+struct kfence *kfence_get(struct kfence *fence);
+void kfence_put(struct kfence *fence);
+
+void kfence_await(struct kfence *fence);
+int kfence_await_kfence(struct kfence *fence,
+ struct kfence *after,
+ gfp_t gfp);
+int kfence_await_completion(struct kfence *fence,
+ struct completion *x,
+ gfp_t gfp);
+void kfence_complete(struct kfence *fence);
+void kfence_wake_up_all(struct kfence *fence);
+void kfence_wait(struct kfence *fence);
+
+/**
+ * kfence_done - report when the fence has been passed
+ * @fence: the kfence to query
+ *
+ * kfence_done() reports true when the fence is no longer waiting for any
+ * events and has completed its fence-complete notification.
+ *
+ * Returns true when the fence has been passed, false otherwise.
+ */
+static inline bool kfence_done(const struct kfence *fence)
+{
+ return atomic_read(&fence->pending) < 0;
+}
+
+#endif /* _KFENCE_H_ */
diff --git a/kernel/Makefile b/kernel/Makefile
index e2ec54e2b952..ff11f31b7ec9 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -9,7 +9,7 @@ obj-y = fork.o exec_domain.o panic.o \
extable.o params.o \
kthread.o sys_ni.o nsproxy.o \
notifier.o ksysfs.o cred.o reboot.o \
- async.o range.o smpboot.o
+ async.o kfence.o range.o smpboot.o
obj-$(CONFIG_MULTIUSER) += groups.o
diff --git a/kernel/kfence.c b/kernel/kfence.c
new file mode 100644
index ..693af9da545a
--- /dev/null
+++ b/kernel/kfence.c
@@ -0,0 +1,431 @@
+/*
+ * (C) Copyright 2016 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of
[PATCH v2 2/7] kfence: Wrap hrtimer to provide a time source for a kfence
A common requirement when scheduling a task is that it should be not be
begun until a certain point in time is passed (e.g.
queue_delayed_work()). kfence_await_hrtimer() causes the kfence to
asynchronously wait until after the appropriate time before being woken.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/kfence.h | 5 +
kernel/kfence.c| 58 ++
lib/test-kfence.c | 44 ++
3 files changed, 107 insertions(+)
diff --git a/include/linux/kfence.h b/include/linux/kfence.h
index 6e32385b3b8c..76a2f95dfb70 100644
--- a/include/linux/kfence.h
+++ b/include/linux/kfence.h
@@ -16,6 +16,7 @@
#include
struct completion;
+enum hrtimer_mode;
struct kfence {
wait_queue_head_t wait;
@@ -43,6 +44,10 @@ int kfence_await_kfence(struct kfence *fence,
int kfence_await_completion(struct kfence *fence,
struct completion *x,
gfp_t gfp);
+int kfence_await_hrtimer(struct kfence *fence,
+clockid_t clock, enum hrtimer_mode mode,
+ktime_t delay, u64 slack,
+gfp_t gfp);
void kfence_complete(struct kfence *fence);
void kfence_wake_up_all(struct kfence *fence);
void kfence_wait(struct kfence *fence);
diff --git a/kernel/kfence.c b/kernel/kfence.c
index 693af9da545a..59c27910a749 100644
--- a/kernel/kfence.c
+++ b/kernel/kfence.c
@@ -48,6 +48,9 @@
* - kfence_await_completion(): the kfence asynchronously waits upon a
* completion
*
+ * - kfence_await_hrtimer(): the kfence asynchronously wait for an expiration
+ * of a timer
+ *
* A kfence is initialised using kfence_init(), and starts off awaiting an
* event. Once you have finished setting up the fence, including adding
* all of its asynchronous waits, call kfence_complete().
@@ -429,3 +432,58 @@ int kfence_await_completion(struct kfence *fence,
return pending;
}
EXPORT_SYMBOL_GPL(kfence_await_completion);
+
+struct timer_cb {
+ struct hrtimer timer;
+ struct kfence *fence;
+};
+
+static enum hrtimer_restart
+timer_kfence_wake(struct hrtimer *timer)
+{
+ struct timer_cb *cb = container_of(timer, typeof(*cb), timer);
+
+ kfence_complete(cb->fence);
+ kfence_put(cb->fence);
+ kfree(cb);
+
+ return HRTIMER_NORESTART;
+}
+
+/**
+ * kfence_await_hrtimer - set the fence to wait for a period of time
+ * @fence: this kfence
+ * @clock: which clock to program
+ * @mode: delay given as relative or absolute
+ * @delay: how long or until what time to wait
+ * @slack: how much slack that may be applied to the delay
+ *
+ * kfence_await_hrtimer() causes the @fence to wait for a a period of time, or
+ * until a certain point in time. It is a convenience wrapper around
+ * hrtimer_start_range_ns(). For more details on @clock, @mode, @delay and
+ * @slack please consult the hrtimer documentation.
+ *
+ * Returns 1 if the delay was sucessfuly added to the @fence, or a negative
+ * error code on failure.
+ */
+int kfence_await_hrtimer(struct kfence *fence,
+clockid_t clock, enum hrtimer_mode mode,
+ktime_t delay, u64 slack,
+gfp_t gfp)
+{
+ struct timer_cb *cb;
+
+ cb = kmalloc(sizeof(*cb), gfp);
+ if (!cb)
+ return -ENOMEM;
+
+ cb->fence = kfence_get(fence);
+ kfence_await(fence);
+
+ hrtimer_init(&cb->timer, clock, mode);
+ cb->timer.function = timer_kfence_wake;
+
+ hrtimer_start_range_ns(&cb->timer, delay, slack, mode);
+ return 1;
+}
+EXPORT_SYMBOL_GPL(kfence_await_hrtimer);
diff --git a/lib/test-kfence.c b/lib/test-kfence.c
index b40719fce967..1b0853fda7c3 100644
--- a/lib/test-kfence.c
+++ b/lib/test-kfence.c
@@ -352,6 +352,44 @@ static int __init test_completion(void)
return 0;
}
+static int __init test_delay(void)
+{
+ struct kfence *fence;
+ ktime_t delay;
+ int ret;
+
+ /* Test use of a hrtimer as an event source for kfences */
+ pr_debug("%s\n", __func__);
+
+ fence = alloc_kfence();
+ if (!fence)
+ return -ENOMEM;
+
+ delay = ktime_get();
+
+ ret = kfence_await_hrtimer(fence, CLOCK_MONOTONIC, HRTIMER_MODE_REL,
+ ms_to_ktime(1), 1 <<
[PATCH v2 7/7] async: Introduce a dependency resolver for parallel execution
A challenge in driver initialisation is the coordination of many small
sometimes independent, sometimes interdependent tasks. We would like to
schedule the independent tasks for execution in parallel across as many
cores as possible for rapid initialisation, and then schedule all the
dependent tasks once they have completed, again running as many of those
in parallel as is possible.
Resolving the interdependencies by hand is time consuming and error
prone. Instead, we want to declare what dependencies a particular task
has, and what that task provides, and let a runtime dependency solver
work out what tasks to run and when, and which in parallel. To this end,
we introduce the struct async_dependency_graph building upon the kfence
and async_work from the previous patches to allow for the runtime
computation of the topological task ordering.
The graph is constructed with async_dependency_graph_build(), which
takes the task, its dependencies and what it provides, and builds the
graph of kfences required for ordering. Additional kfences can be
inserted through async_dependency_depends() and
async_dependency_provides() for manual control of the execution order,
and async_dependency_get() retrieves a kfence for inspection or waiting
upon.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/async.h | 38 +++
kernel/async.c | 250
lib/Kconfig.debug | 12 +
lib/Makefile | 1 +
lib/test-async-dependency-graph.c | 316 +
.../selftests/lib/async-dependency-graph.sh| 10 +
6 files changed, 627 insertions(+)
create mode 100644 lib/test-async-dependency-graph.c
create mode 100755 tools/testing/selftests/lib/async-dependency-graph.sh
diff --git a/include/linux/async.h b/include/linux/async.h
index de44306f8cb7..0a0040d3fc01 100644
--- a/include/linux/async.h
+++ b/include/linux/async.h
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
typedef u64 async_cookie_t;
typedef void (*async_func_t) (void *data, async_cookie_t cookie);
@@ -101,4 +102,41 @@ extern async_cookie_t queue_async_work(struct async_domain
*domain,
gfp_t gfp);
extern async_cookie_t schedule_async_work(struct async_work *work);
+/* Build a graph of work based on dependencies generated by keywords.
+ * The graph must be acyclic. Can be used to both generate a topological
+ * ordering of tasks, and to execute independent chains of tasks in
+ * parallel.
+ */
+
+struct async_dependency_graph {
+ struct rb_root root;
+ struct list_head list;
+};
+
+#define ASYNC_DEPENDENCY_GRAPH_INIT(_name) { \
+ .root = RB_ROOT,\
+ .list = LIST_HEAD_INIT(_name.list), \
+}
+
+#define ASYNC_DEPENDENCY_GRAPH(_name) \
+ struct async_dependency_graph _name = ASYNC_DEPENDENCY_GRAPH_INIT(_name)
+
+extern int async_dependency_graph_build(struct async_dependency_graph *adg,
+ async_func_t fn, void *data,
+ const char *depends,
+ const char *provides);
+
+extern int async_dependency_depends(struct async_dependency_graph *adg,
+ struct kfence *fence,
+ const char *depends);
+
+extern int async_dependency_provides(struct async_dependency_graph *adg,
+struct kfence *fence,
+const char *provides);
+
+extern struct kfence *async_dependency_get(struct async_dependency_graph *adg,
+ const char *name);
+
+extern void async_dependency_graph_execute(struct async_dependency_graph *adg);
+
#endif
diff --git a/kernel/async.c b/kernel/async.c
index 5cfa398a19b2..ac12566f2e0b 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -447,3 +447,253 @@ void init_async_domain(struct async_domain *domain, bool
registered)
domain->registered = registered;
}
EXPORT_SYMBOL_GPL(init_async_domain);
+
+struct async_dependency {
+ struct kfence fence;
+ struct rb_node node;
+ struct list_head link;
+ char name[0];
+};
+
+static struct async_depende
[PATCH v2 6/7] async: Add execution barriers
A frequent mode of operation is fanning out N tasks to execute in
parallel, collating results, fanning out M tasks, rinse and repeat. This
is also common to the notion of the async/sync kernel domain split.
A barrier provides a mechanism by which all work queued after the
barrier must wait (i.e. not be scheduled) until all work queued before the
barrier is completed.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/async.h | 4 +++
kernel/async.c| 72 +++
2 files changed, 76 insertions(+)
diff --git a/include/linux/async.h b/include/linux/async.h
index e7d7289a9889..de44306f8cb7 100644
--- a/include/linux/async.h
+++ b/include/linux/async.h
@@ -26,6 +26,7 @@ struct async_work {
struct async_domain {
struct list_head pending;
+ struct kfence *barrier;
unsigned registered:1;
};
@@ -59,6 +60,9 @@ extern void async_synchronize_cookie(async_cookie_t cookie);
extern void async_synchronize_cookie_domain(async_cookie_t cookie,
struct async_domain *domain);
+extern void async_barrier(void);
+extern void async_barrier_domain(struct async_domain *domain);
+
extern bool current_is_async(void);
extern struct async_work *
diff --git a/kernel/async.c b/kernel/async.c
index 0d695919a60d..5cfa398a19b2 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -154,6 +154,15 @@ struct async_work *async_work_create(async_func_t func,
void *data, gfp_t gfp)
}
EXPORT_SYMBOL_GPL(async_work_create);
+static void async_barrier_delete(struct async_domain *domain)
+{
+ if (!domain->barrier)
+ return;
+
+ kfence_put(domain->barrier);
+ domain->barrier = NULL;
+}
+
async_cookie_t queue_async_work(struct async_domain *domain,
struct async_work *work,
gfp_t gfp)
@@ -174,6 +183,10 @@ async_cookie_t queue_async_work(struct async_domain
*domain,
async_pending_count++;
spin_unlock_irqrestore(&async_lock, flags);
+ if (domain->barrier &&
+ !kfence_await_kfence(&entry->base.fence, domain->barrier, gfp))
+ async_barrier_delete(domain);
+
/* mark that this task has queued an async job, used by module init */
current->flags |= PF_USED_ASYNC;
@@ -241,6 +254,63 @@ async_cookie_t async_schedule_domain(async_func_t func,
void *data,
}
EXPORT_SYMBOL_GPL(async_schedule_domain);
+static struct kfence *__async_barrier_create(struct async_domain *domain)
+{
+ struct kfence *fence;
+ struct async_entry *entry;
+ unsigned long flags;
+ int ret;
+
+ fence = kmalloc(sizeof(*fence), GFP_KERNEL);
+ if (!fence)
+ goto out_sync;
+
+ kfence_init(fence, NULL);
+
+ ret = 0;
+ spin_lock_irqsave(&async_lock, flags);
+ list_for_each_entry(entry, &domain->pending, pending_link[0]) {
+ ret |= kfence_await_kfence(fence,
+ &entry->base.fence,
+ GFP_ATOMIC);
+ if (ret < 0)
+ break;
+ }
+ spin_unlock_irqrestore(&async_lock, flags);
+ if (ret <= 0)
+ goto out_put;
+
+ if (domain->barrier)
+ kfence_await_kfence(fence, domain->barrier, GFP_KERNEL);
+
+ kfence_complete(fence);
+ return fence;
+
+out_put:
+ kfence_complete(fence);
+ kfence_put(fence);
+out_sync:
+ async_synchronize_full_domain(domain);
+ return NULL;
+}
+
+void async_barrier(void)
+{
+ async_barrier_domain(&async_dfl_domain);
+}
+EXPORT_SYMBOL_GPL(async_barrier);
+
+void async_barrier_domain(struct async_domain *domain)
+{
+ struct kfence *barrier = __async_barrier_create(domain);
+
+ if (domain->barrier)
+ kfence_put(domain->barrier);
+
+ domain->barrier = barrier;
+}
+EXPORT_SYMBOL_GPL(async_barrier_domain);
+
/**
* async_synchronize_full - synchronize all asynchronous function calls
*
@@ -264,6 +334,8 @@ EXPORT_SYMBOL_GPL(async_synchronize_full);
void async_unregister_domain(struct async_domain *domain)
{
WARN_ON(!list_empty(&domain->pending));
+
+ async_barrier_delete(domain);
domain->registered = 0;
}
EXPORT_SYMB
[PATCH v2 5/7] async: Add support for explicit fine-grained barriers
The current async-domain model supports running a multitude of
independent tasks with a coarse synchronisation point. This is
sufficient for its original purpose of allowing independent drivers to
run concurrently during various phases (booting, early resume, late
resume etc), and keep the asynchronous domain out of the synchronous
kernel domains. However, for greater exploitation, drivers themselves
want to schedule multiple tasks within a phase (or between phases) and
control the order of execution within those tasks relative to each
other. To enable this, we extend the synchronisation scheme based upon
kfences and back every task with one. Any task may now wait upon the
kfence before being scheduled, and equally the kfence may be used to
wait on the task itself (rather than waiting on the cookie for all
previous tasks to be completed).
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/async.h | 60 -
kernel/async.c | 234 --
lib/test-async-domain.c | 324 +++-
3 files changed, 515 insertions(+), 103 deletions(-)
diff --git a/include/linux/async.h b/include/linux/async.h
index 6b0226bdaadc..e7d7289a9889 100644
--- a/include/linux/async.h
+++ b/include/linux/async.h
@@ -13,38 +13,88 @@
#define __ASYNC_H__
#include
+#include
#include
typedef u64 async_cookie_t;
typedef void (*async_func_t) (void *data, async_cookie_t cookie);
+
+struct async_work {
+ struct kfence fence;
+ /* private */
+};
+
struct async_domain {
struct list_head pending;
unsigned registered:1;
};
+#define ASYNC_DOMAIN_INIT(_name, _r) { \
+ .pending = LIST_HEAD_INIT(_name.pending), \
+ .registered = _r\
+}
+
/*
* domain participates in global async_synchronize_full
*/
#define ASYNC_DOMAIN(_name) \
- struct async_domain _name = { .pending = LIST_HEAD_INIT(_name.pending),
\
- .registered = 1 }
+ struct async_domain _name = ASYNC_DOMAIN_INIT(_name, 1)
/*
* domain is free to go out of scope as soon as all pending work is
* complete, this domain does not participate in async_synchronize_full
*/
#define ASYNC_DOMAIN_EXCLUSIVE(_name) \
- struct async_domain _name = { .pending = LIST_HEAD_INIT(_name.pending),
\
- .registered = 0 }
+ struct async_domain _name = ASYNC_DOMAIN_INIT(_name, 0)
+
+extern void init_async_domain(struct async_domain *domain, bool registered);
extern async_cookie_t async_schedule(async_func_t func, void *data);
extern async_cookie_t async_schedule_domain(async_func_t func, void *data,
struct async_domain *domain);
-void async_unregister_domain(struct async_domain *domain);
+extern void async_unregister_domain(struct async_domain *domain);
extern void async_synchronize_full(void);
extern void async_synchronize_full_domain(struct async_domain *domain);
extern void async_synchronize_cookie(async_cookie_t cookie);
extern void async_synchronize_cookie_domain(async_cookie_t cookie,
struct async_domain *domain);
+
extern bool current_is_async(void);
+
+extern struct async_work *
+async_work_create(async_func_t func, void *data, gfp_t gfp);
+
+static inline struct async_work *async_work_get(struct async_work *work)
+{
+ kfence_get(&work->fence);
+ return work;
+}
+
+static inline int
+async_work_after(struct async_work *work, struct kfence *fence)
+{
+ return kfence_await_kfence(&work->fence, fence, GFP_KERNEL);
+}
+
+static inline int
+async_work_before(struct async_work *work, struct kfence *fence)
+{
+ return kfence_await_kfence(fence, &work->fence, GFP_KERNEL);
+}
+
+static inline void async_work_wait(struct async_work *work)
+{
+ kfence_wait(&work->fence);
+}
+
+static inline void async_work_put(struct async_work *work)
+{
+ kfence_put(&work->fence);
+}
+
+extern async_cookie_t queue_async_work(struct async_domain *domain,
+ struct async_work *work,
+ gfp_t gfp);
+extern async_cookie_t schedule_async_work(struct async_work *work);
+
#endif
diff --git a/kernel/async.c b/kernel/async.c
index d2
Re: [PATCH 2/9] async: Introduce kfence, a N:M completion mechanism
On Wed, Jul 13, 2016 at 11:38:52AM +0200, Peter Zijlstra wrote: > 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 > > #include > > +#include > > #include > > #include > > #include > > So why does this live in async.c? It got its own header, why not also > give it its own .c file? I started in kernel/async (since my first goal was fine-grained async_work serialisation). It is still in kernel/async.c as the embedded fence inside the async_work needs a return code to integrate. I should have done that before posting... > Also, I'm not a particular fan of the k* naming, but I see 'fence' is > already taken. Agreed, I really want to rename the dma-buf fence to struct dma_fence - we would need to do that whilst it dma-buf fencing is still in its infancy. > > +/** > > + * 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. Ok, I was just copying the style from Documentation/scheduler/completion.txt > Also, what !? I don't get what this function does. Hmm. Something more like: "To check the state of a kfence without changing it in any way, call kfence_pending(), which returns true if the kfence is still waiting for its event sources to be signaled." s/signaled/completed/ depending on kfence_signal() vs kfence_complete() > > + * > > + * 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? Possibly, but we also have the dma-buf fences to try and be fairly consistent with. struct completion is definitely a closer sibling though. The biggest conceptual change from completions though is that a kfence will be signaled multiple times before it is complete - I think that is a strong argument in favour of using _signal(). > > + * > > + * 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. Ok, will do a spin with completion naming convention and see how that fits in (and complete the extraction to a separate .c) -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] async: Introduce kfence, a N:M completion mechanism
Completions are a simple synchronization mechanism, suitable for 1:M
barriers where many waiters maybe waiting for a single event. In
situations where a single waiter needs to wait for multiple events they
could wait on a list of individual completions. If many waiters need the
same set of completion events, they each need to wait upon their own list.
The kfence abstracts this by itself being able to wait upon many events
before signaling its own completion. A kfence may wait upon completions
or other kfences, so long as there are no cycles in the dependency graph.
The kfence is a one-shot flag, signaling that work has progressed passed
a certain point and the waiters may proceed. Unlike completions, kfences
are expected to live inside more complex graphs and form the basis for
parallel execution of interdependent tasks and so are referenced counted.
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.
kfence_signal()
undoes the earlier pending notification and allows the fence to complete
if all pending events have then been signaled.
kfence_wait()
allows the caller to sleep (uninterruptibly) until the fence is complete.
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.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/kfence.h| 41 +++
kernel/async.c| 358 +
lib/Kconfig.debug | 23 ++
lib/Makefile | 1 +
lib/test-kfence.c | 475 ++
tools/testing/selftests/lib/kfence.sh | 10 +
6 files changed, 908 insertions(+)
create mode 100644 include/linux/kfence.h
create mode 100644 lib/test-kfence.c
create mode 100755 tools/testing/selftests/lib/kfence.sh
diff --git a/include/linux/kfence.h b/include/linux/kfence.h
new file mode 100644
index ..82eba8aacd02
--- /dev/null
+++ b/include/linux/kfence.h
@@ -0,0 +1,41 @@
+/*
+ * kfence.h - library routines for N:M synchronisation points
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * This file is released under the GPLv2.
+ *
+ */
+
+#ifndef _KFENCE_H_
+#define _KFENCE_H_
+
+#include
+#include
+#include
+
+struct completion;
+
+struct kfence {
+ wait_queue_head_t wait;
+ unsigned long flags;
+ struct kref kref;
+ atomic_t pending;
+};
+
+extern struct kfence *kfence_create(gfp_t gfp);
+extern struct kfence *kfence_get(struct kfence *fence);
+extern int kfence_add(struct kfence *fence, struct kfence *after, gfp_t gfp);
+extern int kfence_add_completion(struct kfence *fence,
+struct completion *x,
+gfp_t gfp);
+extern void kfence_pending(struct kfence *fence);
+extern void kfence_signal(struct kfence *fence);
+extern void kfence_wait(struct kfence *fence);
+static inline bool kfence_complete(struct kfence *fence)
+{
+ return atomic_read(&fence->pending) < 0;
+}
+extern void kfence_put(struct kfence *fence);
+
+#endif /* _KFENCE_H_ */
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
#include
+#include
#include
#include
#include
@@ -82,6 +83,363 @@ static DECLARE_WAIT_QUEUE_HEAD(async_done);
static atomic_t entry_count;
+/**
+ * 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.
[PATCH 7/9] async: Add support for explicit fine-grained barriers
The current async-domain model supports running a multitude of
independent tasks with a coarse synchronisation point. This is
sufficient for its original purpose of allowing independent drivers to
run concurrently during various phases (booting, early resume, late
resume etc), and keep the asynchronous domain out of the synchronous
kernel domains. However, for greater exploitation, drivers themselves
want to schedule multiple tasks within a phase (or between phases) and
control the order of execution within those tasks relative to each
other. To enable this, we extend the synchronisation scheme based upon
kfences and back every task with one. Any task may now wait upon the
kfence before being scheduled, and equally the kfence may be used to
wait on the task itself (rather than waiting on the cookie for all
previous tasks to be completed).
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/async.h | 60 +-
kernel/async.c | 244 +
lib/test-async-domain.c | 311 +++-
3 files changed, 505 insertions(+), 110 deletions(-)
diff --git a/include/linux/async.h b/include/linux/async.h
index 6b0226bdaadc..45d6c8323b60 100644
--- a/include/linux/async.h
+++ b/include/linux/async.h
@@ -13,38 +13,88 @@
#define __ASYNC_H__
#include
+#include
#include
typedef u64 async_cookie_t;
typedef void (*async_func_t) (void *data, async_cookie_t cookie);
+
+struct async_work {
+ struct kfence fence;
+ /* private */
+};
+
struct async_domain {
struct list_head pending;
unsigned registered:1;
};
+#define ASYNC_DOMAIN_INIT(_name, _r) { \
+ .pending = LIST_HEAD_INIT(_name.pending), \
+ .registered = _r\
+}
+
/*
* domain participates in global async_synchronize_full
*/
#define ASYNC_DOMAIN(_name) \
- struct async_domain _name = { .pending = LIST_HEAD_INIT(_name.pending),
\
- .registered = 1 }
+ struct async_domain _name = ASYNC_DOMAIN_INIT(_name, 1)
/*
* domain is free to go out of scope as soon as all pending work is
* complete, this domain does not participate in async_synchronize_full
*/
#define ASYNC_DOMAIN_EXCLUSIVE(_name) \
- struct async_domain _name = { .pending = LIST_HEAD_INIT(_name.pending),
\
- .registered = 0 }
+ struct async_domain _name = ASYNC_DOMAIN_INIT(_name, 0)
+
+extern void init_async_domain(struct async_domain *domain, bool registered);
extern async_cookie_t async_schedule(async_func_t func, void *data);
extern async_cookie_t async_schedule_domain(async_func_t func, void *data,
struct async_domain *domain);
-void async_unregister_domain(struct async_domain *domain);
+extern void async_unregister_domain(struct async_domain *domain);
extern void async_synchronize_full(void);
extern void async_synchronize_full_domain(struct async_domain *domain);
extern void async_synchronize_cookie(async_cookie_t cookie);
extern void async_synchronize_cookie_domain(async_cookie_t cookie,
struct async_domain *domain);
+
extern bool current_is_async(void);
+
+extern struct async_work *
+async_work_create(async_func_t func, void *data, gfp_t gfp);
+
+static inline struct async_work *async_work_get(struct async_work *work)
+{
+ kfence_get(&work->fence);
+ return work;
+}
+
+static inline int
+async_work_after(struct async_work *work, struct kfence *fence)
+{
+ return kfence_add(&work->fence, fence, GFP_KERNEL);
+}
+
+static inline int
+async_work_before(struct async_work *work, struct kfence *fence)
+{
+ return kfence_add(fence, &work->fence, GFP_KERNEL);
+}
+
+static inline void async_work_wait(struct async_work *work)
+{
+ kfence_wait(&work->fence);
+}
+
+static inline void async_work_put(struct async_work *work)
+{
+ kfence_put(&work->fence);
+}
+
+extern async_cookie_t queue_async_work(struct async_domain *domain,
+ struct async_work *work,
+ gfp_t gfp);
+extern async_cookie_t schedule_async_work(struct async_work *work);
+
#endif
diff --git a/kernel/async.c b/kernel/async.c
index 1fa1f39b5a74..8300
[PATCH 6/9] async: Add a convenience wrapper for waiting on implicit dma-buf
dma-buf implicitly track their (DMA) rendering using a
reservation_object, which tracks ether the last write (in an exclusive
fence) or the current renders (with a set of shared fences). To wait
upon a reservation object in conjunction with other sources,
kfence_add_reservation() extracts the DMA fences from the object and
adds the individual waits for the kfence.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/kfence.h | 5
kernel/async.c | 65 ++
2 files changed, 65 insertions(+), 5 deletions(-)
diff --git a/include/linux/kfence.h b/include/linux/kfence.h
index 1abec5e6b23c..2f01eb052e4d 100644
--- a/include/linux/kfence.h
+++ b/include/linux/kfence.h
@@ -17,6 +17,7 @@
struct completion;
struct fence;
enum hrtimer_mode;
+struct reservation_object;
struct kfence {
wait_queue_head_t wait;
@@ -34,6 +35,10 @@ extern int kfence_add_completion(struct kfence *fence,
extern int kfence_add_dma(struct kfence *fence,
struct fence *dma,
gfp_t gfp);
+extern int kfence_add_reservation(struct kfence *fence,
+ struct reservation_object *resv,
+ bool write,
+ gfp_t gfp);
extern int kfence_add_delay(struct kfence *fence,
clockid_t clock, enum hrtimer_mode mode,
ktime_t delay, u64 slack,
diff --git a/kernel/async.c b/kernel/async.c
index 5d02445e36b7..1fa1f39b5a74 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -54,9 +54,10 @@ asynchronous and synchronous parts of the kernel.
#include
#include
#include
-#include
+#include
#include
#include
+#include
#include
#include "workqueue_internal.h"
@@ -123,11 +124,17 @@ static atomic_t entry_count;
* allowing multiple pending / signals to be sent before the kfence is
* complete.
*
- * kfence_add() / kfence_add_completion() / kfence_add_dma()
+ * kfence_add() / kfence_add_completion()
+ * / kfence_add_dma()
+ *
+ * sets the kfence to wait upon another fence or completion respectively. To
+ * wait upon DMA activity, either use
*
- * sets the kfence to wait upon another fence, completion, or DMA fence
- * respectively. To set the fence to wait for at least a certain period of
- * time, or until after a certain point in time, use
+ * kfence_add_dma() or kfence_add_reservation()
+ *
+ * depending on the manner of DMA activity tracking. To set the fence to wait
+ * for at least a certain period of time, or until after a certain point in
+ * time, use
*
* kfence_add_timer()
*
@@ -547,6 +554,54 @@ int kfence_add_dma(struct kfence *fence, struct fence
*dma, gfp_t gfp)
EXPORT_SYMBOL_GPL(kfence_add_dma);
/**
+ * kfence_add_reservation - set the fence to wait upon a reservation_object
+ * @fence: this kfence
+ * @resv: target reservation_object (collection of DMA fences) to wait upon
+ * @write: Wait for read or read/write access
+ * @gfp: the allowed allocation type
+ *
+ * kfence_add_reservation() causes the @fence to wait upon completion of the
+ * reservation object (a collection of DMA fences), either for read access
+ * or for read/write access.
+ *
+ * Returns 1 if the @fence was successfully to the waitqueues of @resv, 0
+ * if @resev was already signaled (and so not added), or a negative error code.
+ */
+int kfence_add_reservation(struct kfence *fence,
+ struct reservation_object *resv,
+ bool write,
+ gfp_t gfp)
+{
+ struct fence *excl, **shared;
+ unsigned count, i;
+ int ret;
+
+ ret = reservation_object_get_fences_rcu(resv, &excl, &count, &shared);
+ if (ret)
+ return ret;
+
+ if (write) {
+ for (i = 0; i < count; i++) {
+ ret |= kfence_add_dma(fence, shared[i], gfp);
+ if (ret < 0)
+ goto out;
+ }
+ }
+
+ if (excl)
+ ret |= kfence_add_dma(fence, excl, gfp);
+
+out:
+ fence_put(excl);
+ for (i = 0; i < count; i++)
+ fence_put(shared[i]);
+ kfree(shared);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kfence_add_reservation);
+
+/**
* kfence_add_delay - set the fence to wait for a peri
[PATCH 8/9] async: Add execution barriers
A frequent mode of operation is fanning out N tasks to execute in
parallel, collating results, fanning out M tasks, rinse and repeat. This
is also common to the notion of the async/sync kernel domain split.
A barrier provides a mechanism by which all work queued after the
barrier must wait (i.e. not be scheduled) until all work queued before the
barrier is completed.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/async.h | 4
kernel/async.c| 60 +++
2 files changed, 64 insertions(+)
diff --git a/include/linux/async.h b/include/linux/async.h
index 45d6c8323b60..64a090e3f24f 100644
--- a/include/linux/async.h
+++ b/include/linux/async.h
@@ -26,6 +26,7 @@ struct async_work {
struct async_domain {
struct list_head pending;
+ struct kfence *barrier;
unsigned registered:1;
};
@@ -59,6 +60,9 @@ extern void async_synchronize_cookie(async_cookie_t cookie);
extern void async_synchronize_cookie_domain(async_cookie_t cookie,
struct async_domain *domain);
+extern void async_barrier(void);
+extern void async_barrier_domain(struct async_domain *domain);
+
extern bool current_is_async(void);
extern struct async_work *
diff --git a/kernel/async.c b/kernel/async.c
index 83007c39a113..a22945f4b4c4 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -724,6 +724,12 @@ struct async_work *async_work_create(async_func_t func,
void *data, gfp_t gfp)
}
EXPORT_SYMBOL_GPL(async_work_create);
+static void async_barrier_delete(struct async_domain *domain)
+{
+ kfence_put(domain->barrier);
+ domain->barrier = NULL;
+}
+
async_cookie_t queue_async_work(struct async_domain *domain,
struct async_work *work,
gfp_t gfp)
@@ -744,6 +750,9 @@ async_cookie_t queue_async_work(struct async_domain *domain,
async_pending_count++;
spin_unlock_irqrestore(&async_lock, flags);
+ if (!kfence_add(&entry->base.fence, domain->barrier, gfp))
+ async_barrier_delete(domain);
+
/* mark that this task has queued an async job, used by module init */
current->flags |= PF_USED_ASYNC;
@@ -811,6 +820,55 @@ async_cookie_t async_schedule_domain(async_func_t func,
void *data,
}
EXPORT_SYMBOL_GPL(async_schedule_domain);
+static struct kfence * __async_barrier_create(struct async_domain *domain)
+{
+ struct kfence *fence;
+ struct async_entry *entry;
+ unsigned long flags;
+ int ret;
+
+ fence = kfence_create(GFP_KERNEL);
+ if (!fence)
+ goto out_sync;
+
+ ret = 0;
+ spin_lock_irqsave(&async_lock, flags);
+ list_for_each_entry(entry, &domain->pending, pending_link[0]) {
+ ret |= kfence_add(fence, &entry->base.fence, GFP_ATOMIC);
+ if (ret < 0)
+ break;
+ }
+ spin_unlock_irqrestore(&async_lock, flags);
+ if (ret <= 0)
+ goto out_put;
+
+ kfence_add(fence, domain->barrier, GFP_KERNEL);
+ kfence_signal(fence);
+ return fence;
+
+out_put:
+ kfence_signal(fence);
+ kfence_put(fence);
+out_sync:
+ async_synchronize_full_domain(domain);
+ return NULL;
+}
+
+void async_barrier(void)
+{
+ async_barrier_domain(&async_dfl_domain);
+}
+EXPORT_SYMBOL_GPL(async_barrier);
+
+void async_barrier_domain(struct async_domain *domain)
+{
+ struct kfence *barrier = __async_barrier_create(domain);
+
+ kfence_put(domain->barrier);
+ domain->barrier = barrier;
+}
+EXPORT_SYMBOL_GPL(async_barrier_domain);
+
/**
* async_synchronize_full - synchronize all asynchronous function calls
*
@@ -834,6 +892,8 @@ EXPORT_SYMBOL_GPL(async_synchronize_full);
void async_unregister_domain(struct async_domain *domain)
{
WARN_ON(!list_empty(&domain->pending));
+
+ async_barrier_delete(domain);
domain->registered = 0;
}
EXPORT_SYMBOL_GPL(async_unregister_domain);
--
2.8.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/9] async: Wrap hrtimer to provide a time source for a kfence
kfence_add_delay() is a convenience wrapper around
hrtimer_start_range_ns() to provide a time source for a kfence graph.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/kfence.h | 5 +
kernel/async.c | 60 +-
lib/test-kfence.c | 44
3 files changed, 108 insertions(+), 1 deletion(-)
diff --git a/include/linux/kfence.h b/include/linux/kfence.h
index d71f30c626ae..1abec5e6b23c 100644
--- a/include/linux/kfence.h
+++ b/include/linux/kfence.h
@@ -16,6 +16,7 @@
struct completion;
struct fence;
+enum hrtimer_mode;
struct kfence {
wait_queue_head_t wait;
@@ -33,6 +34,10 @@ extern int kfence_add_completion(struct kfence *fence,
extern int kfence_add_dma(struct kfence *fence,
struct fence *dma,
gfp_t gfp);
+extern int kfence_add_delay(struct kfence *fence,
+ clockid_t clock, enum hrtimer_mode mode,
+ ktime_t delay, u64 slack,
+ gfp_t gfp);
extern void kfence_pending(struct kfence *fence);
extern void kfence_signal(struct kfence *fence);
extern void kfence_wait(struct kfence *fence);
diff --git a/kernel/async.c b/kernel/async.c
index 01552d23a916..5d02445e36b7 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -126,7 +126,10 @@ static atomic_t entry_count;
* kfence_add() / kfence_add_completion() / kfence_add_dma()
*
* sets the kfence to wait upon another fence, completion, or DMA fence
- * respectively.
+ * respectively. To set the fence to wait for at least a certain period of
+ * time, or until after a certain point in time, use
+ *
+ * kfence_add_timer()
*
* Unlike completions, kfences are expected to live inside more complex graphs
* and form the basis for parallel execution of interdependent and so are
@@ -543,6 +546,61 @@ int kfence_add_dma(struct kfence *fence, struct fence
*dma, gfp_t gfp)
}
EXPORT_SYMBOL_GPL(kfence_add_dma);
+/**
+ * kfence_add_delay - set the fence to wait for a period of time
+ * @fence: this kfence
+ * @clock: which clock to program
+ * @mode: delay given as relative or absolute
+ * @delay: how long or until what time to wait
+ * @slack: how much slack that may be applied to the delay
+ *
+ * kfence_add_delay() causes the @fence to wait for a a period of time, or
+ * until a certain point in time. It is a convenience wrapper around
+ * hrtimer_start_range_ns(). For more details on @clock, @mode, @delay and
+ * @slack please consult the hrtimer documentation.
+ *
+ * Returns 1 if the delay was sucessfuly added to the @fence, or a negative
+ * error code on failure.
+ */
+struct timer_cb {
+ struct hrtimer timer;
+ struct kfence *fence;
+};
+
+static enum hrtimer_restart
+timer_kfence_wake(struct hrtimer *timer)
+{
+ struct timer_cb *cb = container_of(timer, typeof(*cb), timer);
+
+ kfence_signal(cb->fence);
+ kfence_put(cb->fence);
+ kfree(cb);
+
+ return HRTIMER_NORESTART;
+}
+
+int kfence_add_delay(struct kfence *fence,
+clockid_t clock, enum hrtimer_mode mode,
+ktime_t delay, u64 slack,
+gfp_t gfp)
+{
+ struct timer_cb *cb;
+
+ cb = kmalloc(sizeof(*cb), gfp);
+ if (!cb)
+ return -ENOMEM;
+
+ cb->fence = kfence_get(fence);
+ kfence_pending(fence);
+
+ hrtimer_init(&cb->timer, clock, mode);
+ cb->timer.function = timer_kfence_wake;
+
+ hrtimer_start_range_ns(&cb->timer, delay, slack, mode);
+ return 1;
+}
+EXPORT_SYMBOL_GPL(kfence_add_delay);
+
static async_cookie_t lowest_in_progress(struct async_domain *domain)
{
struct list_head *pending;
diff --git a/lib/test-kfence.c b/lib/test-kfence.c
index 0604a27f68b2..782a2cca19c5 100644
--- a/lib/test-kfence.c
+++ b/lib/test-kfence.c
@@ -341,6 +341,44 @@ static int __init test_completion(void)
return 0;
}
+static int __init test_delay(void)
+{
+ struct kfence *fence;
+ ktime_t delay;
+ int ret;
+
+ /* Test use of a hrtimer as an event source for kfences */
+ pr_debug("%s\n", __func__);
+
+ fence = kfence_create(GFP_KERNEL);
+ if (!fence)
+ return -ENOMEM;
+
+ delay = ktime_get();
+
+ ret = kfence_add_delay(fence, CLOCK_MONOTONIC,
[PATCH 4/9] async: Extend kfences for listening on DMA fences
dma-buf provides an interfaces for receiving notifications from DMA
hardware. kfence provides a useful interface for collecting such fences
and combining them with other events.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/kfence.h | 4
kernel/async.c | 63 --
2 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/include/linux/kfence.h b/include/linux/kfence.h
index 82096bfafaa1..d71f30c626ae 100644
--- a/include/linux/kfence.h
+++ b/include/linux/kfence.h
@@ -15,6 +15,7 @@
#include
struct completion;
+struct fence;
struct kfence {
wait_queue_head_t wait;
@@ -29,6 +30,9 @@ extern int kfence_add(struct kfence *fence, struct kfence
*after, gfp_t gfp);
extern int kfence_add_completion(struct kfence *fence,
struct completion *x,
gfp_t gfp);
+extern int kfence_add_dma(struct kfence *fence,
+ struct fence *dma,
+ gfp_t gfp);
extern void kfence_pending(struct kfence *fence);
extern void kfence_signal(struct kfence *fence);
extern void kfence_wait(struct kfence *fence);
diff --git a/kernel/async.c b/kernel/async.c
index db22b890711e..01552d23a916 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -50,6 +50,7 @@ asynchronous and synchronous parts of the kernel.
#include
#include
+#include
#include
#include
#include
@@ -122,9 +123,10 @@ static atomic_t entry_count;
* allowing multiple pending / signals to be sent before the kfence is
* complete.
*
- * kfence_add() / kfence_add_completion()
+ * kfence_add() / kfence_add_completion() / kfence_add_dma()
*
- * sets the kfence to wait upon another fence, or completion respectively.
+ * sets the kfence to wait upon another fence, completion, or DMA fence
+ * respectively.
*
* Unlike completions, kfences are expected to live inside more complex graphs
* and form the basis for parallel execution of interdependent and so are
@@ -484,6 +486,63 @@ int kfence_add_completion(struct kfence *fence, struct
completion *x, gfp_t gfp)
}
EXPORT_SYMBOL_GPL(kfence_add_completion);
+struct dma_fence_cb {
+ struct fence_cb base;
+ struct kfence *fence;
+};
+
+static void dma_kfence_wake(struct fence *dma, struct fence_cb *data)
+{
+ struct dma_fence_cb *cb = container_of(data, typeof(*cb), base);
+ kfence_signal(cb->fence);
+ kfence_put(cb->fence);
+ kfree(cb);
+}
+
+/**
+ * kfence_add_dma - set the fence to wait upon a DMA fence
+ * @fence: this kfence
+ * @dma: target DMA fence to wait upon
+ * @gfp: the allowed allocation type
+ *
+ * kfence_add_dma() causes the @fence to wait upon completion of a DMA fence.
+ *
+ * Returns 1 if the @fence was successfully to the waitqueue of @dma, 0
+ * if @dma was already signaled (and so not added), or a negative error code.
+ */
+int kfence_add_dma(struct kfence *fence, struct fence *dma, gfp_t gfp)
+{
+ struct dma_fence_cb *cb;
+ int ret;
+
+ if (!dma || fence_is_signaled(dma))
+ return 0;
+
+ cb = kmalloc(sizeof(*cb), gfp);
+ if (!cb) {
+ if (!gfpflags_allow_blocking(gfp))
+ return -ENOMEM;
+
+ return fence_wait(dma, false);
+ }
+
+ cb->fence = kfence_get(fence);
+ kfence_pending(fence);
+
+ ret = fence_add_callback(dma, &cb->base, dma_kfence_wake);
+ if (ret == 0) {
+ /* signal fence add and is pending */
+ ret = 1;
+ } else {
+ dma_kfence_wake(dma, &cb->base);
+ if (ret == -ENOENT) /* fence already signaled */
+ ret = 0;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kfence_add_dma);
+
static async_cookie_t lowest_in_progress(struct async_domain *domain)
{
struct list_head *pending;
--
2.8.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] async: Introduce a dependency resolver for parallel execution
A challenge in driver initialisation is the coordination of many small
sometimes independent, sometimes interdependent tasks. We would like to
schedule the independent tasks for execution in parallel across as many
cores as possible for rapid initialisation, and then schedule all the
dependent tasks once they have completed, again running as many of those
in parallel as is possible.
Resolving the interdependencies by hand is time consuming and error
prone. Instead, we want to declare what dependencies a particular task
has, and what that task provides, and let a runtime dependency solver
work out what tasks to run and when, and which in parallel. To this end,
we introduce the struct async_dependency_graph building upon the kfence
and async_work from the previous patches to allow for the runtime
computation of the topological task ordering.
The graph is constructed with async_dependency_graph_build(), which
takes the task, its dependencies and what it provides, and builds the
graph of kfences required for ordering. Additional kfences can be
inserted through async_dependency_depends() and
async_dependency_provides() for manual control of the execution order,
and async_dependency_get() retrieves a kfence for inspection or waiting
upon.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/async.h | 37 +++
kernel/async.c | 250
lib/Kconfig.debug | 12 +
lib/Makefile | 1 +
lib/test-async-dependency-graph.c | 317 +
.../selftests/lib/async-dependency-graph.sh| 10 +
6 files changed, 627 insertions(+)
create mode 100644 lib/test-async-dependency-graph.c
create mode 100755 tools/testing/selftests/lib/async-dependency-graph.sh
diff --git a/include/linux/async.h b/include/linux/async.h
index 64a090e3f24f..c9cadb383813 100644
--- a/include/linux/async.h
+++ b/include/linux/async.h
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
typedef u64 async_cookie_t;
typedef void (*async_func_t) (void *data, async_cookie_t cookie);
@@ -101,4 +102,40 @@ extern async_cookie_t queue_async_work(struct async_domain
*domain,
gfp_t gfp);
extern async_cookie_t schedule_async_work(struct async_work *work);
+/* Build a graph of work based on dependencies generated by keywords.
+ * The graph must be acyclic. Can be used to both generate a topological
+ * ordering of tasks, and to execute independent chains of tasks in
+ * parallel.
+ */
+
+struct async_dependency_graph {
+ struct rb_root root;
+ struct list_head list;
+};
+
+#define ASYNC_DEPENDENCY_GRAPH_INIT(_name) { \
+ .root = RB_ROOT,\
+ .list = LIST_HEAD_INIT(_name.list), \
+}
+#define ASYNC_DEPENDENCY_GRAPH(_name) \
+ struct async_dependency_graph _name = ASYNC_DEPENDENCY_GRAPH_INIT(_name)
+
+extern int async_dependency_graph_build(struct async_dependency_graph *adg,
+ async_func_t fn, void *data,
+ const char *depends,
+ const char *provides);
+
+extern int async_dependency_depends(struct async_dependency_graph *adg,
+ struct kfence *fence,
+ const char *depends);
+
+extern int async_dependency_provides(struct async_dependency_graph *adg,
+struct kfence *fence,
+const char *provides);
+
+extern struct kfence *async_dependency_get(struct async_dependency_graph *adg,
+ const char *name);
+
+extern void async_dependency_graph_execute(struct async_dependency_graph *adg);
+
#endif
diff --git a/kernel/async.c b/kernel/async.c
index a22945f4b4c4..8330d719074b 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -1005,3 +1005,253 @@ void init_async_domain(struct async_domain *domain,
bool registered)
domain->registered = registered;
}
EXPORT_SYMBOL_GPL(init_async_domain);
+
+struct async_dependency {
+ struct kfence fence;
+ struct rb_node node;
+ struct list_head link;
+ char name[0];
+};
+
+static struct async_depende
[PATCH 3/9] async: Extend kfence to allow struct embedding
Provide a kfence_init() function for use for embedding the kfence into a
parent structure. kfence_init() takes an optional function pointer argument
should the caller wish to be notified when the kfence is complete. This is
useful for allowing the kfences to drive other state machinery.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Shuah Khan
Cc: Tejun Heo
Cc: Daniel Vetter
Cc: Andrew Morton
Cc: Ingo Molnar
Cc: Kees Cook
Cc: Thomas Gleixner
Cc: "Paul E. McKenney"
Cc: Dan Williams
Cc: Andrey Ryabinin
Cc: Davidlohr Bueso
Cc: Nikolay Aleksandrov
Cc: "David S. Miller"
Cc: "Peter Zijlstra (Intel)"
Cc: Rasmus Villemoes
Cc: Andy Shevchenko
Cc: Dmitry Vyukov
Cc: Alexander Potapenko
Cc: linux-ker...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
include/linux/kfence.h | 5
kernel/async.c | 52
lib/test-kfence.c | 64 +++---
3 files changed, 109 insertions(+), 12 deletions(-)
diff --git a/include/linux/kfence.h b/include/linux/kfence.h
index 82eba8aacd02..82096bfafaa1 100644
--- a/include/linux/kfence.h
+++ b/include/linux/kfence.h
@@ -38,4 +38,9 @@ static inline bool kfence_complete(struct kfence *fence)
}
extern void kfence_put(struct kfence *fence);
+typedef void (*kfence_notify_t)(struct kfence *);
+#define __kfence_call __attribute__((aligned(4)))
+
+extern void kfence_init(struct kfence *fence, kfence_notify_t fn);
+
#endif /* _KFENCE_H_ */
diff --git a/kernel/async.c b/kernel/async.c
index d0bcb7cc4884..db22b890711e 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -134,7 +134,17 @@ static atomic_t entry_count;
*
* 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.
+ * its event sources. To embed a kfence within another struct, use
+ *
+ * kfence_init()
+ *
+ * This can also be used to receive a callback when the kfence is completed
+ * (pending signal count hits 0). The callback is also called when the fence
+ * is released (the reference count hits 0, and the fence will be complete).
+ * Note that since the kfence is embedded inside another, its initial reference
+ * must not be dropped unless a callback is provided, or the kfence is the
+ * first member in the parent, and the parent was allocated by kmalloc (i.e.
+ * valid to be freed by kfree()).
*
* Use
*
@@ -151,7 +161,11 @@ static void kfence_free(struct kref *kref)
WARN_ON(atomic_read(&fence->pending) > 0);
- kfree(fence);
+ if (fence->flags) {
+ kfence_notify_t fn = (kfence_notify_t)fence->flags;
+ fn(fence);
+ } else
+ kfree(fence);
}
/**
@@ -203,6 +217,11 @@ static void __kfence_signal(struct kfence *fence,
if (!atomic_dec_and_test(&fence->pending))
return;
+ if (fence->flags) {
+ kfence_notify_t fn = (kfence_notify_t)fence->flags;
+ fn(fence);
+ }
+
atomic_dec(&fence->pending);
__kfence_wake_up_all(fence, continuation);
}
@@ -240,7 +259,7 @@ void kfence_wait(struct kfence *fence)
}
EXPORT_SYMBOL_GPL(kfence_wait);
-static void kfence_init(struct kfence *fence)
+static void __kfence_init(struct kfence *fence)
{
init_waitqueue_head(&fence->wait);
kref_init(&fence->kref);
@@ -266,11 +285,36 @@ struct kfence *kfence_create(gfp_t gfp)
if (!fence)
return NULL;
- kfence_init(fence);
+ __kfence_init(fence);
return fence;
}
EXPORT_SYMBOL_GPL(kfence_create);
+/**
+ * kfence_init - initialize a fence for use embedded within a struct
+ * @fence: this kfence
+ * @fn: a callback function for when the fence is complete, and when the
+ * fence is released
+ *
+ * This function initialises the @fence for use embedded within a parent
+ * structure. The optional @fn hook is called when the fence is completed
+ * (when all of its events sources have signaled their completion, i.e.
+ * pending signal count hits 0, fence_complete() however reports false as the
+ * fence is not considered complete until after the callback returns) and when
+ * the fence is subsequently released (the reference count hits 0,
+ * fence_complete() is then true). Note carefully that @fn will be called from
+ * atomic context. Also the lower 2 bits of the function pointer are used for
+ * storing flags, so the function must be aligned to at least 4 bytes - use
+ * __kfence_call as a function attribute to ensure correct alignment.
+ */
+void kfence_init(struct kfence *fence, kfence_notify_t fn)
+{
+ __kfence_init(fence);
+ BUG_ON((unsigned long)fn & KFENCE_CHECKED_BIT);
+ fence->flags = (unsigned long)fn;
+}
+EXPORT_SYMBOL_GPL(k
[PATCH] dma-buf: Wait on the reservation object when sync'ing before CPU access
Rendering operations to the dma-buf are tracked implicitly via the
reservation_object (dmabuf->resv). This is used to allow poll() to
wait upon outstanding rendering (or just query the current status of
rendering). The dma-buf sync ioctl allows userspace to prepare the
dma-buf for CPU access, which should include waiting upon rendering.
(Some drivers may need to do more work to ensure that the dma-buf mmap
is coherent as well as complete.)
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Daniel Vetter
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Cc: linux-ker...@vger.kernel.org
---
I'm wondering whether it makes sense just to always do the wait first.
It is one of the first operations every driver has to make. A driver
that wants to implement it differently (e.g. they can special case
native waits) will still require a wait on the reservation object to
finish external rendering.
-Chris
---
drivers/dma-buf/dma-buf.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ddaee60ae52a..123f14b8e882 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment
*attach,
}
EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment);
+static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
+ enum dma_data_direction direction)
+{
+ bool write = (direction == DMA_BIDIRECTIONAL ||
+ direction == DMA_TO_DEVICE);
+ struct reservation_object *resv = dma_buf->resv;
+ long ret;
+
+ /* Wait on any implicit rendering fences */
+ ret = reservation_object_wait_timeout_rcu(resv, write, true,
+ MAX_SCHEDULE_TIMEOUT);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
/**
* dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from
the
@@ -607,6 +623,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
if (dmabuf->ops->begin_cpu_access)
ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
+ else
+ ret = __dma_buf_begin_cpu_access(dmabuf, direction);
return ret;
}
--
2.8.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dma-buf: Release module reference on creation failure
If we fail to create the anon file, we need to remember to release the
module reference on the owner.
Signed-off-by: Chris Wilson
Cc: Sumit Semwal
Cc: Daniel Vetter
Cc: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/dma-buf/dma-buf.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 4a2c07ee6677..6f0f0b10a241 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -333,6 +333,7 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
struct reservation_object *resv = exp_info->resv;
struct file *file;
size_t alloc_size = sizeof(struct dma_buf);
+ int ret;
if (!exp_info->resv)
alloc_size += sizeof(struct reservation_object);
@@ -356,8 +357,8 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
dmabuf = kzalloc(alloc_size, GFP_KERNEL);
if (!dmabuf) {
- module_put(exp_info->owner);
- return ERR_PTR(-ENOMEM);
+ ret = -ENOMEM;
+ goto free_module;
}
dmabuf->priv = exp_info->priv;
@@ -378,8 +379,8 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf,
exp_info->flags);
if (IS_ERR(file)) {
- kfree(dmabuf);
- return ERR_CAST(file);
+ ret = PTR_ERR(file);
+ goto free_dmabuf;
}
file->f_mode |= FMODE_LSEEK;
@@ -393,6 +394,12 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
mutex_unlock(&db_list.lock);
return dmabuf;
+
+free_dmabuf:
+ kfree(dmabuf);
+free_module:
+ module_put(exp_info->owner);
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(dma_buf_export);
--
2.8.0.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma-buf: Update docs for SYNC ioctl
On Wed, Mar 23, 2016 at 04:32:59PM +0100, David Herrmann wrote: > Hi > > On Wed, Mar 23, 2016 at 12:56 PM, Chris Wilson > wrote: > > On Wed, Mar 23, 2016 at 12:30:42PM +0100, David Herrmann wrote: > >> My question was rather about why we do this? Semantics for EINTR are > >> well defined, and with SA_RESTART (default on linux) user-space can > >> ignore it. However, looping on EAGAIN is very uncommon, and it is not > >> at all clear why it is needed? > >> > >> Returning an error to user-space makes sense if user-space has a > >> reason to react to it. I fail to see how EAGAIN on a cache-flush/sync > >> operation helps user-space at all? As someone without insight into the > >> driver implementation, it is hard to tell why.. Any hints? > > > > The reason we return EAGAIN is to workaround a deadlock we face when > > blocking on the GPU holding the struct_mutex (inside the client's > > process), but the GPU is dead. As our locking is very, very coarse we > > cannot restart the GPU without acquiring the struct_mutex being held by > > the client so we wake the client up and tell them the resource they are > > waiting on (the flush of the object from the GPU into the CPU domain) is > > temporarily unavailable. If they try to immediately wait upon the ioctl > > again, they are blocked waiting for the reset to occur before they may > > complete their flush. There are a few other possible deadlocks that are > > also avoided with EAGAIN (again, the issue is more or less the lack of > > fine grained locking). > > ...so you hijacked EAGAIN for all DRM ioctls just for a driver > workaround? No, we utilized the fact that EAGAIN was already enshrined by libdrm as the defacto mechanism for repeating the ioctl in order to repeat the ioctl for a driver workaround. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma-buf: Update docs for SYNC ioctl
On Wed, Mar 23, 2016 at 12:30:42PM +0100, David Herrmann wrote: > My question was rather about why we do this? Semantics for EINTR are > well defined, and with SA_RESTART (default on linux) user-space can > ignore it. However, looping on EAGAIN is very uncommon, and it is not > at all clear why it is needed? > > Returning an error to user-space makes sense if user-space has a > reason to react to it. I fail to see how EAGAIN on a cache-flush/sync > operation helps user-space at all? As someone without insight into the > driver implementation, it is hard to tell why.. Any hints? The reason we return EAGAIN is to workaround a deadlock we face when blocking on the GPU holding the struct_mutex (inside the client's process), but the GPU is dead. As our locking is very, very coarse we cannot restart the GPU without acquiring the struct_mutex being held by the client so we wake the client up and tell them the resource they are waiting on (the flush of the object from the GPU into the CPU domain) is temporarily unavailable. If they try to immediately wait upon the ioctl again, they are blocked waiting for the reset to occur before they may complete their flush. There are a few other possible deadlocks that are also avoided with EAGAIN (again, the issue is more or less the lack of fine grained locking). -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dma-buf,drm,ion: Propagate error code from dma_buf_start_cpu_access()
Drivers, especially i915.ko, can fail during the initial migration of a
dma-buf for CPU access. However, the error code from the driver was not
being propagated back to ioctl and so userspace was blissfully ignorant
of the failure. Rendering corruption ensues.
Whilst fixing the ioctl to return the error code from
dma_buf_start_cpu_access(), also do the same for
dma_buf_end_cpu_access(). For most drivers, dma_buf_end_cpu_access()
cannot fail. i915.ko however, as most drivers would, wants to avoid being
uninterruptible (as would be required to guarrantee no failure when
flushing the buffer to the device). As userspace already has to handle
errors from the SYNC_IOCTL, take advantage of this to be able to restart
the syscall across signals.
This fixes a coherency issue for i915.ko as well as reducing the
uninterruptible hold upon its BKL, the struct_mutex.
Fixes commit c11e391da2a8fe973c3c2398452000bed505851e
Author: Daniel Vetter
Date: Thu Feb 11 20:04:51 2016 -0200
dma-buf: Add ioctls to allow userspace to flush
Testcase: igt/gem_concurrent_blit/*dmabuf*interruptible
Testcase: igt/prime_mmap_coherency/ioctl-errors
Signed-off-by: Chris Wilson
Cc: Tiago Vignatti
Cc: Stéphane Marchesin
Cc: David Herrmann
Cc: Sumit Semwal
Cc: Daniel Vetter
CC: linux-media@vger.kernel.org
Cc: dri-de...@lists.freedesktop.org
Cc: linaro-mm-...@lists.linaro.org
Cc: intel-...@lists.freedesktop.org
Cc: de...@driverdev.osuosl.org
---
drivers/dma-buf/dma-buf.c | 17 +++--
drivers/gpu/drm/i915/i915_gem_dmabuf.c| 15 +--
drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 5 +++--
drivers/gpu/drm/udl/udl_fb.c | 4 ++--
drivers/staging/android/ion/ion.c | 6 --
include/linux/dma-buf.h | 6 +++---
6 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 9810d1df0691..774a60f4309a 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -259,6 +259,7 @@ static long dma_buf_ioctl(struct file *file,
struct dma_buf *dmabuf;
struct dma_buf_sync sync;
enum dma_data_direction direction;
+ int ret;
dmabuf = file->private_data;
@@ -285,11 +286,11 @@ static long dma_buf_ioctl(struct file *file,
}
if (sync.flags & DMA_BUF_SYNC_END)
- dma_buf_end_cpu_access(dmabuf, direction);
+ ret = dma_buf_end_cpu_access(dmabuf, direction);
else
- dma_buf_begin_cpu_access(dmabuf, direction);
+ ret = dma_buf_begin_cpu_access(dmabuf, direction);
- return 0;
+ return ret;
default:
return -ENOTTY;
}
@@ -613,13 +614,17 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
*
* This call must always succeed.
*/
-void dma_buf_end_cpu_access(struct dma_buf *dmabuf,
- enum dma_data_direction direction)
+int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
+ enum dma_data_direction direction)
{
+ int ret = 0;
+
WARN_ON(!dmabuf);
if (dmabuf->ops->end_cpu_access)
- dmabuf->ops->end_cpu_access(dmabuf, direction);
+ ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access);
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 1f3eef6fb345..0506016e18e0 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -228,25 +228,20 @@ static int i915_gem_begin_cpu_access(struct dma_buf
*dma_buf, enum dma_data_dire
return ret;
}
-static void i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum
dma_data_direction direction)
+static int i915_gem_end_cpu_access(struct dma_buf *dma_buf, enum
dma_data_direction direction)
{
struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
struct drm_device *dev = obj->base.dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- bool was_interruptible;
int ret;
- mutex_lock(&dev->struct_mutex);
- was_interruptible = dev_priv->mm.interruptible;
- dev_priv->mm.interruptible = false;
+ ret = i915_mutex_lock_interruptible(dev);
+ if (ret)
+ return ret;
ret = i915_gem_object_set_to_gtt_domain(obj, false);
-
- dev_priv->mm.interruptible = was_interruptible;
mutex_unlock(&dev->struct_mutex);
- if (unlikely(ret))
- DRM_ERROR("unable to flush buffer following CPU access;
rendering may be corrupt\n");
+ return ret;
}
static const struct dma_buf_ops i915_dmabuf_ops = {
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
Re: [Linaro-mm-sig] [PATCH 2/3] dma-buf: add support for kernel cpu access
On Thu, 1 Mar 2012 16:36:00 +0100, Daniel Vetter wrote: > Big differences to other contenders in the field (like ion) is > that this also supports highmem, so we have to split up the cpu > access from the kernel side into a prepare and a kmap step. > > Prepare is allowed to fail and should do everything required so that > the kmap calls can succeed (like swapin/backing storage allocation, > flushing, ...). > > More in-depth explanations will follow in the follow-up documentation > patch. > > Changes in v2: > > - Clear up begin_cpu_access confusion noticed by Sumit Semwal. > - Don't automatically fallback from the _atomic variants to the > non-atomic variants. The _atomic callbacks are not allowed to > sleep, so we want exporters to make this decision explicit. The > function signatures are explicit, so simpler exporters can still > use the same function for both. > - Make the unmap functions optional. Simpler exporters with permanent > mappings don't need to do anything at unmap time. Are we going to have to have a dma_buf->ops->begin_async_access(&me, dir) variant for coherency control of rendering with an imported dma_buf? There is also no concurrency control here between multiple importers doing simultaneous begin_cpu_access(). I presume that is going to be a common function across all exporters so the midlayer might offer a semaphore as a library function and then the dma_buf->ops->begin_cpu_access() becomes mandatory as at a minimum it has to point to the default implementation. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel Display and Video API Consolidation mini-summit at ELC 2012 - Notes
On Wed, 22 Feb 2012 17:24:24 +0100, Daniel Vetter wrote: > On Wed, Feb 22, 2012 at 04:03:21PM +, James Simmons wrote: > > Fbcon scrolling at be painful at HD or better modes. Fbcon needs 3 > > possible accels; copyarea, imageblit, and fillrect. The first two could be > > hooked from the TTM layer. Its something I plan to experiment to see if > > its worth it. > > Let's bite into this ;-) I know that fbcon scrolling totally sucks on big > screens, but I also think it's a total waste of time to fix this. Imo > fbcon has 2 use-cases: > - display an OOSP. > - allow me to run fsck (or any other desaster-recovery stuff). 3. Show panics. Ensuring that nothing prevents the switch to fbcon and displaying the panic message is the reason why we haven't felt inclined to accelerate fbcon - it just gets messy for no real gain. For example: https://bugs.freedesktop.org/attachment.cgi?id=48933 which doesn't handle flushing of pending updates via the GPU when writing with the CPU during interrupts (i.e. a panic). -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
