[PATCH 0/3] dma-buf changes for ttm and amdgpu

[PATCH 0/3] dma-buf changes for ttm and amdgpu

[Alex Deucher]

We have some changes in ttm and amdgpu that depend on these patches.
Sumit, can you pull these in via dma-buf or should I roll them up
through my amdgpu tree?

Christian König (3):
  dma-buf: dma_fence_put is NULL safe
  dma-buf: add reservation_object_copy_fences
  dma-buf: fix reservation_object_wait_timeout_rcu to wait correctly v2

 drivers/dma-buf/reservation.c | 97 +++++++++++++++++++++++++++++++++----------
 include/linux/reservation.h   |  3 ++
 2 files changed, 78 insertions(+), 22 deletions(-)

-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/2] dma-buf: add reservation_object_copy_fences (v2)

[Alex Deucher]

From: Christian König <christian.koenig@amd.com>

Allows us to copy all the fences in a reservation object to another one.

v2: handle NULL src_list

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/dma-buf/reservation.c | 60 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/reservation.h   |  3 +++
 2 files changed, 63 insertions(+)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 87f8f57..d4881f9 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -262,6 +262,66 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,
 EXPORT_SYMBOL(reservation_object_add_excl_fence);
 
 /**
+* reservation_object_copy_fences - Copy all fences from src to dst.
+* @dst: the destination reservation object
+* @src: the source reservation object
+*
+* Copy all fences from src to dst. Both src->lock as well as dst-lock must be
+* held.
+*/
+int reservation_object_copy_fences(struct reservation_object *dst,
+				   struct reservation_object *src)
+{
+	struct reservation_object_list *src_list, *dst_list;
+	struct dma_fence *old, *new;
+	size_t size;
+	unsigned i;
+
+	src_list = reservation_object_get_list(src);
+
+	if (src_list) {
+		size = offsetof(typeof(*src_list),
+				shared[src_list->shared_count]);
+		dst_list = kmalloc(size, GFP_KERNEL);
+		if (!dst_list)
+			return -ENOMEM;
+
+		dst_list->shared_count = src_list->shared_count;
+		dst_list->shared_max = src_list->shared_count;
+		for (i = 0; i < src_list->shared_count; ++i)
+			dst_list->shared[i] =
+				dma_fence_get(src_list->shared[i]);
+	} else {
+		dst_list = NULL;
+	}
+
+	kfree(dst->staged);
+	dst->staged = NULL;
+
+	src_list = reservation_object_get_list(dst);
+
+	old = reservation_object_get_excl(dst);
+	new = reservation_object_get_excl(src);
+
+	dma_fence_get(new);
+
+	preempt_disable();
+	write_seqcount_begin(&dst->seq);
+	/* write_seqcount_begin provides the necessary memory barrier */
+	RCU_INIT_POINTER(dst->fence_excl, new);
+	RCU_INIT_POINTER(dst->fence, dst_list);
+	write_seqcount_end(&dst->seq);
+	preempt_enable();
+
+	if (src_list)
+		kfree_rcu(src_list, rcu);
+	dma_fence_put(old);
+
+	return 0;
+}
+EXPORT_SYMBOL(reservation_object_copy_fences);
+
+/**
  * reservation_object_get_fences_rcu - Get an object's shared and exclusive
  * fences without update side lock held
  * @obj: the reservation object
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index 156cfd3..21fc84d 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -254,6 +254,9 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
 				      unsigned *pshared_count,
 				      struct dma_fence ***pshared);
 
+int reservation_object_copy_fences(struct reservation_object *dst,
+				   struct reservation_object *src);
+
 long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
 					 bool wait_all, bool intr,
 					 unsigned long timeout);
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] dma-buf: fix reservation_object_wait_timeout_rcu to wait correctly v2

[Alex Deucher]

From: Christian König <christian.koenig@amd.com>

With hardware resets in mind it is possible that all shared fences are
signaled, but the exlusive isn't. Fix waiting for everything in this situation.

v2: make sure we always wait for the exclusive fence

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/dma-buf/reservation.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index d4881f9..dec3a81 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -431,12 +431,25 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
 	long ret = timeout ? timeout : 1;
 
 retry:
-	fence = NULL;
 	shared_count = 0;
 	seq = read_seqcount_begin(&obj->seq);
 	rcu_read_lock();
 
-	if (wait_all) {
+	fence = rcu_dereference(obj->fence_excl);
+	if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
+		if (!dma_fence_get_rcu(fence))
+			goto unlock_retry;
+
+		if (dma_fence_is_signaled(fence)) {
+			dma_fence_put(fence);
+			fence = NULL;
+		}
+
+	} else {
+		fence = NULL;
+	}
+
+	if (!fence && wait_all) {
 		struct reservation_object_list *fobj =
 						rcu_dereference(obj->fence);
 
@@ -463,22 +476,6 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
 		}
 	}
 
-	if (!shared_count) {
-		struct dma_fence *fence_excl = rcu_dereference(obj->fence_excl);
-
-		if (fence_excl &&
-		    !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
-			      &fence_excl->flags)) {
-			if (!dma_fence_get_rcu(fence_excl))
-				goto unlock_retry;
-
-			if (dma_fence_is_signaled(fence_excl))
-				dma_fence_put(fence_excl);
-			else
-				fence = fence_excl;
-		}
-	}
-
 	rcu_read_unlock();
 	if (fence) {
 		if (read_seqcount_retry(&obj->seq, seq)) {
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] dma-buf: fix reservation_object_wait_timeout_rcu to wait correctly v2

[Chris Wilson]

Quoting Alex Deucher (2017-08-10 18:01:49)
> From: Christian König <christian.koenig@amd.com>
> 
> With hardware resets in mind it is possible that all shared fences are
> signaled, but the exlusive isn't. Fix waiting for everything in this situation.

I'm still puzzling over this one.

Setting an exclusive fence will clear all shared, so we must have added
the shared fences after the exclusive fence. But those shared fences must
follow the exclusive fence, you should not be able to reorder the shared
fences ahead of the exclusive. If you have completed shared fences, but
incomplete exclusive, doesn't that imply you have an ordering issue?
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] dma-buf: fix reservation_object_wait_timeout_rcu to wait correctly v2

[Christian König]

Am 10.08.2017 um 19:11 schrieb Chris Wilson:
> Quoting Alex Deucher (2017-08-10 18:01:49)
>> From: Christian König <christian.koenig@amd.com>
>>
>> With hardware resets in mind it is possible that all shared fences are
>> signaled, but the exlusive isn't. Fix waiting for everything in this situation.
> I'm still puzzling over this one.
>
> Setting an exclusive fence will clear all shared, so we must have added
> the shared fences after the exclusive fence. But those shared fences must
> follow the exclusive fence, you should not be able to reorder the shared
> fences ahead of the exclusive. If you have completed shared fences, but
> incomplete exclusive, doesn't that imply you have an ordering issue?

No, that is not an ordering issue.

The problem happens when the shared fences are "aborted" because of a 
GPU reset.

See the exclusive fence is often a DMA moving bytes on behalf of the 
kernel while the shared fences are the actual command submission from 
user space.

What can happen is that the userspace process is killed and/or it's 
scheduled command submission aborted for other reasons.

In this case the shared fences get an error code, are set to the 
signaled state and the job they represent is never executed.

Regards,
Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] dma-buf: fix reservation_object_wait_timeout_rcu to wait correctly v2

[Chris Wilson]

Quoting Christian König (2017-08-10 19:19:52)
> Am 10.08.2017 um 19:11 schrieb Chris Wilson:
> > Quoting Alex Deucher (2017-08-10 18:01:49)
> >> From: Christian König <christian.koenig@amd.com>
> >>
> >> With hardware resets in mind it is possible that all shared fences are
> >> signaled, but the exlusive isn't. Fix waiting for everything in this situation.
> > I'm still puzzling over this one.
> >
> > Setting an exclusive fence will clear all shared, so we must have added
> > the shared fences after the exclusive fence. But those shared fences must
> > follow the exclusive fence, you should not be able to reorder the shared
> > fences ahead of the exclusive. If you have completed shared fences, but
> > incomplete exclusive, doesn't that imply you have an ordering issue?
> 
> No, that is not an ordering issue.
> 
> The problem happens when the shared fences are "aborted" because of a 
> GPU reset.
> 
> See the exclusive fence is often a DMA moving bytes on behalf of the 
> kernel while the shared fences are the actual command submission from 
> user space.
> 
> What can happen is that the userspace process is killed and/or it's 
> scheduled command submission aborted for other reasons.
> 
> In this case the shared fences get an error code, are set to the 
> signaled state and the job they represent is never executed.

I didn't allow the GPU reset to cause fences to appear to complete out
of order, just rewrote them so they would be skipped (with the error
status flagged) if the error was terminal but still update the hw
semaphore and breadcrumbs (i.e. the user portion was cancel, but our
bookkeeping was left intact).
-Chris
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] dma-buf: fix reservation_object_wait_timeout_rcu to wait correctly v2

[Christian König]

Am 10.08.2017 um 20:50 schrieb Chris Wilson:
> Quoting Christian König (2017-08-10 19:19:52)
>> Am 10.08.2017 um 19:11 schrieb Chris Wilson:
>>> Quoting Alex Deucher (2017-08-10 18:01:49)
>>>> From: Christian König <christian.koenig@amd.com>
>>>>
>>>> With hardware resets in mind it is possible that all shared fences are
>>>> signaled, but the exlusive isn't. Fix waiting for everything in this situation.
>>> I'm still puzzling over this one.
>>>
>>> Setting an exclusive fence will clear all shared, so we must have added
>>> the shared fences after the exclusive fence. But those shared fences must
>>> follow the exclusive fence, you should not be able to reorder the shared
>>> fences ahead of the exclusive. If you have completed shared fences, but
>>> incomplete exclusive, doesn't that imply you have an ordering issue?
>> No, that is not an ordering issue.
>>
>> The problem happens when the shared fences are "aborted" because of a
>> GPU reset.
>>
>> See the exclusive fence is often a DMA moving bytes on behalf of the
>> kernel while the shared fences are the actual command submission from
>> user space.
>>
>> What can happen is that the userspace process is killed and/or it's
>> scheduled command submission aborted for other reasons.
>>
>> In this case the shared fences get an error code, are set to the
>> signaled state and the job they represent is never executed.
> I didn't allow the GPU reset to cause fences to appear to complete out
> of order, just rewrote them so they would be skipped (with the error
> status flagged) if the error was terminal but still update the hw
> semaphore and breadcrumbs (i.e. the user portion was cancel, but our
> bookkeeping was left intact).

Well again, they don't signal out of order. The exclusive fence and the 
shared fence which is aborted belong to completely independent contexts 
(and can even belong to completely independent hardware).

It's just that the job which should trigger the shared fence never gets 
executed.

Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 0/3] dma-buf changes for ttm and amdgpu

[Sumit Semwal]

Hi Alex,

On 10 August 2017 at 22:31, Alex Deucher <alexdeucher@gmail.com> wrote:
> We have some changes in ttm and amdgpu that depend on these patches.
> Sumit, can you pull these in via dma-buf or should I roll them up
> through my amdgpu tree?
>
> Christian König (3):
>   dma-buf: dma_fence_put is NULL safe
>   dma-buf: add reservation_object_copy_fences
>   dma-buf: fix reservation_object_wait_timeout_rcu to wait correctly v2
>
Looks like my reply on your last set of patches didn't reach you?

Anyways, I'm on a short vacation, so please feel free to take this via
drm-misc, with my
Acked-by: Sumit Semwal <sumit.semwal@linaro.org>

>  drivers/dma-buf/reservation.c | 97 +++++++++++++++++++++++++++++++++----------
>  include/linux/reservation.h   |  3 ++
>  2 files changed, 78 insertions(+), 22 deletions(-)
>
> --
> 2.5.5
>

Best,
Sumit.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 0/3] dma-buf changes for ttm and amdgpu

[Deucher, Alexander]

> -----Original Message-----
> From: Sumit Semwal [mailto:sumit.semwal@linaro.org]
> Sent: Friday, August 11, 2017 11:52 AM
> To: Alex Deucher
> Cc: amd-gfx list; DRI mailing list; Koenig, Christian; Deucher, Alexander
> Subject: Re: [PATCH 0/3] dma-buf changes for ttm and amdgpu
> 
> Hi Alex,
> 
> On 10 August 2017 at 22:31, Alex Deucher <alexdeucher@gmail.com> wrote:
> > We have some changes in ttm and amdgpu that depend on these patches.
> > Sumit, can you pull these in via dma-buf or should I roll them up
> > through my amdgpu tree?
> >
> > Christian König (3):
> >   dma-buf: dma_fence_put is NULL safe
> >   dma-buf: add reservation_object_copy_fences
> >   dma-buf: fix reservation_object_wait_timeout_rcu to wait correctly v2
> >
> Looks like my reply on your last set of patches didn't reach you?
> 
> Anyways, I'm on a short vacation, so please feel free to take this via
> drm-misc, with my
> Acked-by: Sumit Semwal <sumit.semwal@linaro.org>
> 

I got your last ack, but I ended up re-sending the last two patches because the second one had a bug.

Thanks,

Alex

> >  drivers/dma-buf/reservation.c | 97
> +++++++++++++++++++++++++++++++++----------
> >  include/linux/reservation.h   |  3 ++
> >  2 files changed, 78 insertions(+), 22 deletions(-)
> >
> > --
> > 2.5.5
> >
> 
> Best,
> Sumit.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx