v2: still check context first to avoid warning from dma_fence_is_later apply this fix in add_shared_replace as well
v3: use a bool falg to indict if fence is need to insert to new slot and ignore it if it is an eld fence compared with the one with the same context in old->shared
Change-Id: If6a979ba9fd6c923b82212f35f07a9ff31c86767 Signed-off-by: Monk Liu Monk.Liu@amd.com --- drivers/dma-buf/reservation.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index 314eb10..a7d0598 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -118,7 +118,8 @@ reservation_object_add_shared_inplace(struct reservation_object *obj, old_fence = rcu_dereference_protected(fobj->shared[i], reservation_object_held(obj));
- if (old_fence->context == fence->context) { + if (old_fence->context == fence->context && + dma_fence_is_later(fence, old_fence)) { /* memory barrier is added by write_seqcount_begin */ RCU_INIT_POINTER(fobj->shared[i], fence); write_seqcount_end(&obj->seq); @@ -158,6 +159,7 @@ reservation_object_add_shared_replace(struct reservation_object *obj, struct dma_fence *fence) { unsigned i, j, k; + bool wrong_fence = false;
dma_fence_get(fence);
@@ -179,15 +181,29 @@ reservation_object_add_shared_replace(struct reservation_object *obj, check = rcu_dereference_protected(old->shared[i], reservation_object_held(obj));
- if (check->context == fence->context || - dma_fence_is_signaled(check)) + if (dma_fence_is_signaled(check)) { + /* put check to tail of fobj if signaled */ RCU_INIT_POINTER(fobj->shared[--k], check); - else + } else if (check->context == fence->context) { + if (dma_fence_is_later(fence, check)) { + /* put check to tail of fobj if it is deprecated */ + RCU_INIT_POINTER(fobj->shared[--k], check); + } else { + /* this is a wrong operation that add an eld fence */ + wrong_fence = true; + RCU_INIT_POINTER(fobj->shared[j++], check); + } + } else { + /* add fence to new slot */ RCU_INIT_POINTER(fobj->shared[j++], check); + } } + fobj->shared_count = j; - RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); - fobj->shared_count++; + if (!wrong_fence) { + RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); + fobj->shared_count++; + }
done: preempt_disable();
Patch looks ok to me, Reviewed-by: Chunming Zhou david1.zhou@amd.com, and it’s better to get others RB as well.
Regards, David Zhou -----Original Message----- From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Monk Liu Sent: Tuesday, March 06, 2018 3:01 PM To: dri-devel@freedesktop.org Cc: Liu, Monk Monk.Liu@amd.com Subject: [PATCH] dma-buf/reservation: should keep later one in add fence(v3)
v2: still check context first to avoid warning from dma_fence_is_later apply this fix in add_shared_replace as well
v3: use a bool falg to indict if fence is need to insert to new slot and ignore it if it is an eld fence compared with the one with the same context in old->shared
Change-Id: If6a979ba9fd6c923b82212f35f07a9ff31c86767 Signed-off-by: Monk Liu Monk.Liu@amd.com --- drivers/dma-buf/reservation.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index 314eb10..a7d0598 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -118,7 +118,8 @@ reservation_object_add_shared_inplace(struct reservation_object *obj, old_fence = rcu_dereference_protected(fobj->shared[i], reservation_object_held(obj));
- if (old_fence->context == fence->context) { + if (old_fence->context == fence->context && + dma_fence_is_later(fence, old_fence)) { /* memory barrier is added by write_seqcount_begin */ RCU_INIT_POINTER(fobj->shared[i], fence); write_seqcount_end(&obj->seq); @@ -158,6 +159,7 @@ reservation_object_add_shared_replace(struct reservation_object *obj, struct dma_fence *fence) { unsigned i, j, k; + bool wrong_fence = false;
dma_fence_get(fence);
@@ -179,15 +181,29 @@ reservation_object_add_shared_replace(struct reservation_object *obj, check = rcu_dereference_protected(old->shared[i], reservation_object_held(obj));
- if (check->context == fence->context || - dma_fence_is_signaled(check)) + if (dma_fence_is_signaled(check)) { + /* put check to tail of fobj if signaled */ RCU_INIT_POINTER(fobj->shared[--k], check); - else + } else if (check->context == fence->context) { + if (dma_fence_is_later(fence, check)) { + /* put check to tail of fobj if it is deprecated */ + RCU_INIT_POINTER(fobj->shared[--k], check); + } else { + /* this is a wrong operation that add an eld fence */ + wrong_fence = true; + RCU_INIT_POINTER(fobj->shared[j++], check); + } + } else { + /* add fence to new slot */ RCU_INIT_POINTER(fobj->shared[j++], check); + } } + fobj->shared_count = j; - RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); - fobj->shared_count++; + if (!wrong_fence) { + RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); + fobj->shared_count++; + }
done: preempt_disable(); -- 2.7.4
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
And still a NAK, the prerequisite for adding a shared fence is that it is later than any existing fence in that context.
In other words reservation_object_add_shared_fence() is always called with a new fence, never with some old one.
So the whole checking here is completely superfluous, Christian.
Am 06.03.2018 um 08:00 schrieb Monk Liu:
v2: still check context first to avoid warning from dma_fence_is_later apply this fix in add_shared_replace as well
v3: use a bool falg to indict if fence is need to insert to new slot and ignore it if it is an eld fence compared with the one with the same context in old->shared
Change-Id: If6a979ba9fd6c923b82212f35f07a9ff31c86767 Signed-off-by: Monk Liu Monk.Liu@amd.com
drivers/dma-buf/reservation.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index 314eb10..a7d0598 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -118,7 +118,8 @@ reservation_object_add_shared_inplace(struct reservation_object *obj, old_fence = rcu_dereference_protected(fobj->shared[i], reservation_object_held(obj));
if (old_fence->context == fence->context) {
if (old_fence->context == fence->context &&dma_fence_is_later(fence, old_fence)) { /* memory barrier is added by write_seqcount_begin */ RCU_INIT_POINTER(fobj->shared[i], fence); write_seqcount_end(&obj->seq);@@ -158,6 +159,7 @@ reservation_object_add_shared_replace(struct reservation_object *obj, struct dma_fence *fence) { unsigned i, j, k;
bool wrong_fence = false;
dma_fence_get(fence);
@@ -179,15 +181,29 @@ reservation_object_add_shared_replace(struct reservation_object *obj, check = rcu_dereference_protected(old->shared[i], reservation_object_held(obj));
if (check->context == fence->context ||dma_fence_is_signaled(check))
if (dma_fence_is_signaled(check)) {/* put check to tail of fobj if signaled */ RCU_INIT_POINTER(fobj->shared[--k], check);
else
} else if (check->context == fence->context) {if (dma_fence_is_later(fence, check)) {/* put check to tail of fobj if it is deprecated */RCU_INIT_POINTER(fobj->shared[--k], check);} else {/* this is a wrong operation that add an eld fence */wrong_fence = true;RCU_INIT_POINTER(fobj->shared[j++], check);}} else {/* add fence to new slot */ RCU_INIT_POINTER(fobj->shared[j++], check);}- fobj->shared_count = j;
- RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
- fobj->shared_count++;
if (!wrong_fence) {
RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);fobj->shared_count++;}
done: preempt_disable();
dri-devel@lists.freedesktop.org
-
Christian König -
Monk Liu -
Zhou, David(ChunMing)