Re: [PATCH 1/6] dma-buf: remove shared fence staging in reservation object
Am 09.08.2018 um 14:08 schrieb Chris Wilson:
Quoting Christian König (2018-08-09 12:37:08)
void reservation_object_add_shared_fence(struct reservation_object *obj,
struct dma_fence *fence)
{
- struct reservation_object_list *old, *fobj = obj->staged;
+ struct reservation_object_list *fobj;
+ unsigned int i;
- old = reservation_object_get_list(obj);
- obj->staged = NULL;
+ dma_fence_get(fence);
+
+ fobj = reservation_object_get_list(obj);
- if (!fobj) {
- BUG_ON(old->shared_count >= old->shared_max);
- reservation_object_add_shared_inplace(obj, old, fence);
- } else
- reservation_object_add_shared_replace(obj, old, fobj, fence);
+ preempt_disable();
+ write_seqcount_begin(>seq);
+
+ for (i = 0; i < fobj->shared_count; ++i) {
+ struct dma_fence *old_fence;
+
+ old_fence = rcu_dereference_protected(fobj->shared[i],
+
reservation_object_held(obj));
+ if (old_fence->context == fence->context ||
+ dma_fence_is_signaled(old_fence)) {
Are you happy with the possibility that the shared[] may contain two
fences belonging to the same context? That was a sticking point earlier.
Yeah, that is fixed by now. I've removed the dependency on this in our
VM handling code quite a while ago.
+ /* memory barrier is added by write_seqcount_begin */
+ RCU_INIT_POINTER(fobj->shared[i], fence);
+ write_seqcount_end(>seq);
+ preempt_enable();
+ dma_fence_put(old_fence);
You can rearrange this to have a single exit.
Good point, going to rearrange the code.
Christian.
for (i = 0; i < fobj->shared_count; ++i) {
struct dma_fence *old_fence;
old_fence = rcu_dereference_protected(fobj->shared[i],
reservation_object_held(obj));
if (old_fence->context == fence->context ||
dma_fence_is_signaled(old_fence)) {
dma_fence_put(old_fence);
goto replace;
}
}
fobj->shared_count++;
replace:
/*
* memory barrier is added by write_seqcount_begin,
* fobj->shared_count is protected by this lock too
*/
RCU_INIT_POINTER(fobj->shared[i], fence);
write_seqcount_end(>seq);
preempt_enable();
}
-Chris
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/6] dma-buf: remove shared fence staging in reservation object
Quoting Christian König (2018-08-09 12:37:08)
> void reservation_object_add_shared_fence(struct reservation_object *obj,
> struct dma_fence *fence)
> {
> - struct reservation_object_list *old, *fobj = obj->staged;
> + struct reservation_object_list *fobj;
> + unsigned int i;
>
> - old = reservation_object_get_list(obj);
> - obj->staged = NULL;
> + dma_fence_get(fence);
> +
> + fobj = reservation_object_get_list(obj);
>
> - if (!fobj) {
> - BUG_ON(old->shared_count >= old->shared_max);
> - reservation_object_add_shared_inplace(obj, old, fence);
> - } else
> - reservation_object_add_shared_replace(obj, old, fobj, fence);
> + preempt_disable();
> + write_seqcount_begin(>seq);
> +
> + for (i = 0; i < fobj->shared_count; ++i) {
> + struct dma_fence *old_fence;
> +
> + old_fence = rcu_dereference_protected(fobj->shared[i],
> +
> reservation_object_held(obj));
> + if (old_fence->context == fence->context ||
> + dma_fence_is_signaled(old_fence)) {
Are you happy with the possibility that the shared[] may contain two
fences belonging to the same context? That was a sticking point earlier.
> + /* memory barrier is added by write_seqcount_begin */
> + RCU_INIT_POINTER(fobj->shared[i], fence);
> + write_seqcount_end(>seq);
> + preempt_enable();
> + dma_fence_put(old_fence);
You can rearrange this to have a single exit.
for (i = 0; i < fobj->shared_count; ++i) {
struct dma_fence *old_fence;
old_fence = rcu_dereference_protected(fobj->shared[i],
reservation_object_held(obj));
if (old_fence->context == fence->context ||
dma_fence_is_signaled(old_fence)) {
dma_fence_put(old_fence);
goto replace;
}
}
fobj->shared_count++;
replace:
/*
* memory barrier is added by write_seqcount_begin,
* fobj->shared_count is protected by this lock too
*/
RCU_INIT_POINTER(fobj->shared[i], fence);
write_seqcount_end(>seq);
preempt_enable();
}
-Chris
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/6] dma-buf: remove shared fence staging in reservation object
No need for that any more. Just replace the list when there isn't enough
room any more for at least one additional fence.
Signed-off-by: Christian König
---
drivers/dma-buf/reservation.c | 180 ++
include/linux/reservation.h | 4 -
2 files changed, 60 insertions(+), 124 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 314eb1071cce..1f0c61b540ba 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -68,104 +68,23 @@ EXPORT_SYMBOL(reservation_seqcount_string);
*/
int reservation_object_reserve_shared(struct reservation_object *obj)
{
- struct reservation_object_list *fobj, *old;
- u32 max;
+ struct reservation_object_list *old, *new;
+ unsigned int i, j, k, max;
old = reservation_object_get_list(obj);
if (old && old->shared_max) {
- if (old->shared_count < old->shared_max) {
- /* perform an in-place update */
- kfree(obj->staged);
- obj->staged = NULL;
+ if (old->shared_count < old->shared_max)
return 0;
- } else
+ else
max = old->shared_max * 2;
- } else
- max = 4;
-
- /*
-* resize obj->staged or allocate if it doesn't exist,
-* noop if already correct size
-*/
- fobj = krealloc(obj->staged, offsetof(typeof(*fobj), shared[max]),
- GFP_KERNEL);
- if (!fobj)
- return -ENOMEM;
-
- obj->staged = fobj;
- fobj->shared_max = max;
- return 0;
-}
-EXPORT_SYMBOL(reservation_object_reserve_shared);
-
-static void
-reservation_object_add_shared_inplace(struct reservation_object *obj,
- struct reservation_object_list *fobj,
- struct dma_fence *fence)
-{
- struct dma_fence *signaled = NULL;
- u32 i, signaled_idx;
-
- dma_fence_get(fence);
-
- preempt_disable();
- write_seqcount_begin(>seq);
-
- for (i = 0; i < fobj->shared_count; ++i) {
- struct dma_fence *old_fence;
-
- old_fence = rcu_dereference_protected(fobj->shared[i],
- reservation_object_held(obj));
-
- if (old_fence->context == fence->context) {
- /* memory barrier is added by write_seqcount_begin */
- RCU_INIT_POINTER(fobj->shared[i], fence);
- write_seqcount_end(>seq);
- preempt_enable();
-
- dma_fence_put(old_fence);
- return;
- }
-
- if (!signaled && dma_fence_is_signaled(old_fence)) {
- signaled = old_fence;
- signaled_idx = i;
- }
- }
-
- /*
-* memory barrier is added by write_seqcount_begin,
-* fobj->shared_count is protected by this lock too
-*/
- if (signaled) {
- RCU_INIT_POINTER(fobj->shared[signaled_idx], fence);
} else {
- RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
- fobj->shared_count++;
+ max = 4;
}
- write_seqcount_end(>seq);
- preempt_enable();
-
- dma_fence_put(signaled);
-}
-
-static void
-reservation_object_add_shared_replace(struct reservation_object *obj,
- struct reservation_object_list *old,
- struct reservation_object_list *fobj,
- struct dma_fence *fence)
-{
- unsigned i, j, k;
-
- dma_fence_get(fence);
-
- if (!old) {
- RCU_INIT_POINTER(fobj->shared[0], fence);
- fobj->shared_count = 1;
- goto done;
- }
+ new = kmalloc(offsetof(typeof(*new), shared[max]), GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
/*
* no need to bump fence refcounts, rcu_read access
@@ -173,46 +92,45 @@ reservation_object_add_shared_replace(struct
reservation_object *obj,
* references from the old struct are carried over to
* the new.
*/
- for (i = 0, j = 0, k = fobj->shared_max; i < old->shared_count; ++i) {
- struct dma_fence *check;
-
- check = rcu_dereference_protected(old->shared[i],
- reservation_object_held(obj));
+ for (i = 0, j = 0, k = max; i < (old ? old->shared_count : 0); ++i) {
+ struct dma_fence *fence;
- if (check->context == fence->context ||
- dma_fence_is_signaled(check))
- RCU_INIT_POINTER(fobj->shared[--k], check);
+ fence =
