dma-fence: fix dma_fence_get_rcu_safe

Submitted by Christian König on Sept. 4, 2017, 1:16 p.m.

Details

Message ID 1504530994-2464-1-git-send-email-deathsimple@vodafone.de
State New
Headers show
Series "dma-fence: fix dma_fence_get_rcu_safe" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Christian König Sept. 4, 2017, 1:16 p.m.
From: Christian König <christian.koenig@amd.com>

The logic is buggy and unnecessary complex. When dma_fence_get_rcu() fails to
acquire a reference it doesn't necessary mean that there is no fence at all.

It usually mean that the fence was replaced by a new one and in this situation
we certainly want to have the new one as result and *NOT* NULL.

Signed-off-by: Christian König <christian.koenig@amd.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-sig@lists.linaro.org
---
 include/linux/dma-fence.h | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

Patch hide | download patch | download mbox

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index a5195a7..37f3d67 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -246,27 +246,8 @@  dma_fence_get_rcu_safe(struct dma_fence * __rcu *fencep)
 		struct dma_fence *fence;
 
 		fence = rcu_dereference(*fencep);
-		if (!fence || !dma_fence_get_rcu(fence))
-			return NULL;
-
-		/* The atomic_inc_not_zero() inside dma_fence_get_rcu()
-		 * provides a full memory barrier upon success (such as now).
-		 * This is paired with the write barrier from assigning
-		 * to the __rcu protected fence pointer so that if that
-		 * pointer still matches the current fence, we know we
-		 * have successfully acquire a reference to it. If it no
-		 * longer matches, we are holding a reference to some other
-		 * reallocated pointer. This is possible if the allocator
-		 * is using a freelist like SLAB_TYPESAFE_BY_RCU where the
-		 * fence remains valid for the RCU grace period, but it
-		 * may be reallocated. When using such allocators, we are
-		 * responsible for ensuring the reference we get is to
-		 * the right fence, as below.
-		 */
-		if (fence == rcu_access_pointer(*fencep))
-			return rcu_pointer_handoff(fence);
-
-		dma_fence_put(fence);
+		if (!fence || dma_fence_get_rcu(fence))
+			return fence;
 	} while (1);
 }
 

Comments

I really wonder what's wrong with my mail client, but it looks like this 
patch never made it at least to dri-devel.

Forwarding manually now,
Christian.

Am 04.09.2017 um 15:16 schrieb Christian König:
> From: Christian König <christian.koenig@amd.com>
>
> The logic is buggy and unnecessary complex. When dma_fence_get_rcu() fails to
> acquire a reference it doesn't necessary mean that there is no fence at all.
>
> It usually mean that the fence was replaced by a new one and in this situation
> we certainly want to have the new one as result and *NOT* NULL.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: linux-media@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linaro-mm-sig@lists.linaro.org
> ---
>   include/linux/dma-fence.h | 23 ++---------------------
>   1 file changed, 2 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index a5195a7..37f3d67 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -246,27 +246,8 @@ dma_fence_get_rcu_safe(struct dma_fence * __rcu *fencep)
>   		struct dma_fence *fence;
>   
>   		fence = rcu_dereference(*fencep);
> -		if (!fence || !dma_fence_get_rcu(fence))
> -			return NULL;
> -
> -		/* The atomic_inc_not_zero() inside dma_fence_get_rcu()
> -		 * provides a full memory barrier upon success (such as now).
> -		 * This is paired with the write barrier from assigning
> -		 * to the __rcu protected fence pointer so that if that
> -		 * pointer still matches the current fence, we know we
> -		 * have successfully acquire a reference to it. If it no
> -		 * longer matches, we are holding a reference to some other
> -		 * reallocated pointer. This is possible if the allocator
> -		 * is using a freelist like SLAB_TYPESAFE_BY_RCU where the
> -		 * fence remains valid for the RCU grace period, but it
> -		 * may be reallocated. When using such allocators, we are
> -		 * responsible for ensuring the reference we get is to
> -		 * the right fence, as below.
> -		 */
> -		if (fence == rcu_access_pointer(*fencep))
> -			return rcu_pointer_handoff(fence);
> -
> -		dma_fence_put(fence);
> +		if (!fence || dma_fence_get_rcu(fence))
> +			return fence;
>   	} while (1);
>   }
>