[PATCH 1/2] dma-buf: make reservation_object_copy_fences rcu save
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Mon Sep 11 13:56:59 UTC 2017
Op 11-09-17 om 14:53 schreef Christian König:
> Am 10.09.2017 um 09:30 schrieb Maarten Lankhorst:
>> Op 04-09-17 om 21:02 schreef Christian König:
>>> From: Christian König <christian.koenig at amd.com>
>>>
>>> Stop requiring that the src reservation object is locked for this operation.
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>> drivers/dma-buf/reservation.c | 56 ++++++++++++++++++++++++++++++++-----------
>>> 1 file changed, 42 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
>>> index dec3a81..b44d9d7 100644
>>> --- a/drivers/dma-buf/reservation.c
>>> +++ b/drivers/dma-buf/reservation.c
>>> @@ -266,8 +266,7 @@ EXPORT_SYMBOL(reservation_object_add_excl_fence);
>>> * @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.
>>> +* Copy all fences from src to dst. dst-lock must be held.
>>> */
>>> int reservation_object_copy_fences(struct reservation_object *dst,
>>> struct reservation_object *src)
>> Could this be implemented using reservation_object_get_fences_rcu? You're essentially duplicating its functionality.
>
> I've considered this as well, but reservation_object_get_fences_rcu() returns an array and here we need an reservation_object_list.
Doesn't seem too hard, below is the result from fiddling with the allocation function..
reservation_object_list is just a list of fences with some junk in front.
Here you go, but only tested with the compiler. O:)
-----8<-----
drivers/dma-buf/reservation.c | 192 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------
1 file changed, 109 insertions(+), 83 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index dec3a815455d..72ee91f34132 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -261,87 +261,43 @@ 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)
+static bool realloc_shared(void **ptr, struct dma_fence ***pshared, unsigned shared_max, bool alloc_list, bool unlocked)
{
- 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;
+ size_t sz;
+ void *newptr;
- src_list = reservation_object_get_list(dst);
+ if (alloc_list)
+ sz = offsetof(struct reservation_object_list, shared[shared_max]);
+ else
+ sz = shared_max * sizeof(struct dma_fence *);
- old = reservation_object_get_excl(dst);
- new = reservation_object_get_excl(src);
+ newptr = krealloc(*ptr, sz, unlocked ? GFP_KERNEL : GFP_NOWAIT | __GFP_NOWARN);
+ if (!newptr)
+ return false;
- dma_fence_get(new);
+ *ptr = newptr;
+ if (alloc_list) {
+ struct reservation_object_list *fobj = newptr;
- 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);
+ fobj->shared_max = shared_max;
+ *pshared = fobj->shared;
+ } else
+ *pshared = newptr;
- return 0;
+ return true;
}
-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
- * @pfence_excl: the returned exclusive fence (or NULL)
- * @pshared_count: the number of shared fences returned
- * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
- * the required size, and must be freed by caller)
- *
- * RETURNS
- * Zero or -errno
- */
-int reservation_object_get_fences_rcu(struct reservation_object *obj,
+static int __reservation_object_get_fences_rcu(struct reservation_object *obj,
struct dma_fence **pfence_excl,
unsigned *pshared_count,
- struct dma_fence ***pshared)
+ struct dma_fence ***pshared,
+ struct reservation_object_list **list_shared)
{
struct dma_fence **shared = NULL;
struct dma_fence *fence_excl;
unsigned int shared_count;
int ret = 1;
+ void *ptr = NULL;
do {
struct reservation_object_list *fobj;
@@ -359,23 +315,16 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
fobj = rcu_dereference(obj->fence);
if (fobj) {
- struct dma_fence **nshared;
- size_t sz = sizeof(*shared) * fobj->shared_max;
-
- nshared = krealloc(shared, sz,
- GFP_NOWAIT | __GFP_NOWARN);
- if (!nshared) {
+ if (!realloc_shared(&ptr, &shared, fobj->shared_max, list_shared, false)) {
rcu_read_unlock();
- nshared = krealloc(shared, sz, GFP_KERNEL);
- if (nshared) {
- shared = nshared;
- continue;
+ if (!realloc_shared(&ptr, &shared, fobj->shared_max, list_shared, true)) {
+ ret = -ENOMEM;
+ break;
}
- ret = -ENOMEM;
- break;
+ /* need to acquire rcu lock again and retry */
+ continue;
}
- shared = nshared;
shared_count = fobj->shared_count;
for (i = 0; i < shared_count; ++i) {
@@ -398,16 +347,93 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
} while (ret);
if (!shared_count) {
- kfree(shared);
+ kfree(ptr);
+ ptr = NULL;
shared = NULL;
}
- *pshared_count = shared_count;
- *pshared = shared;
- *pfence_excl = fence_excl;
+ if (!list_shared) {
+ *pshared_count = shared_count;
+ *pshared = shared;
+ } else {
+ *list_shared = ptr;
+ if (ptr)
+ (*list_shared)->shared_count = shared_count;
+ }
+ *pfence_excl = fence_excl;
return ret;
}
+
+
+/**
+* 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. dst-lock must be held.
+*/
+int reservation_object_copy_fences(struct reservation_object *dst,
+ struct reservation_object *src)
+{
+ struct reservation_object_list *old_list, *dst_list;
+ struct dma_fence *excl, *old;
+ unsigned i;
+ int ret;
+
+ ret = __reservation_object_get_fences_rcu(src, &excl, NULL,
+ NULL, &dst_list);
+ if (ret)
+ return ret;
+
+ kfree(dst->staged);
+ dst->staged = NULL;
+
+ old = rcu_dereference_protected(dst->fence_excl,
+ reservation_object_held(dst));
+
+ old_list = rcu_dereference_protected(dst->fence,
+ reservation_object_held(dst));
+
+ preempt_disable();
+ write_seqcount_begin(&dst->seq);
+ /* write_seqcount_begin provides the necessary memory barrier */
+ RCU_INIT_POINTER(dst->fence_excl, excl);
+ RCU_INIT_POINTER(dst->fence, dst_list);
+ write_seqcount_end(&dst->seq);
+ preempt_enable();
+
+ if (old_list) {
+ for (i = 0; i < old_list->shared_count; i++)
+ dma_fence_put(old_list->shared[i]);
+
+ kfree_rcu(old_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
+ * @pfence_excl: the returned exclusive fence (or NULL)
+ * @pshared_count: the number of shared fences returned
+ * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
+ * the required size, and must be freed by caller)
+ *
+ * RETURNS
+ * Zero or -errno
+ */
+int reservation_object_get_fences_rcu(struct reservation_object *obj,
+ struct dma_fence **pfence_excl,
+ unsigned *pshared_count,
+ struct dma_fence ***pshared)
+{
+ return __reservation_object_get_fences_rcu(obj, pfence_excl, pshared_count, pshared, NULL);
+}
EXPORT_SYMBOL_GPL(reservation_object_get_fences_rcu);
/**
More information about the amd-gfx
mailing list