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.koe...@amd.com> >>> >>> Stop requiring that the src reservation object is locked for this operation. >>> >>> Signed-off-by: Christian König <christian.koe...@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); /** _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel