diff mbox series

[2/2] dma-buf: Relax the write-seqlock for reallocating the shared fence list

Message ID 20190712080314.21018-2-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [1/2] dma-buf: Expand reservation_list to fill allocation | expand

Commit Message

Chris Wilson July 12, 2019, 8:03 a.m. UTC
As the set of shared fences is not being changed during reallocation of
the reservation list, we can skip updating the write_seqlock.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/reservation.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Daniel Vetter July 16, 2019, 9:21 a.m. UTC | #1
On Fri, Jul 12, 2019 at 09:03:14AM +0100, Chris Wilson wrote:
> As the set of shared fences is not being changed during reallocation of
> the reservation list, we can skip updating the write_seqlock.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Christian König <christian.koenig@amd.com>

sounds legit.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

More seriously, I think I convinced myself that we cant see a mess of old
and new fence arrays anywhere, even without the seqlock retry, so I think
we should be all good.
-Daniel

> ---
>  drivers/dma-buf/reservation.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 80ecc1283d15..c71b85c8c159 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -157,15 +157,15 @@ int reservation_object_reserve_shared(struct reservation_object *obj,
>  		(ksize(new) - offsetof(typeof(*new), shared)) /
>  		sizeof(*new->shared);
>  
> -	preempt_disable();
> -	write_seqcount_begin(&obj->seq);
>  	/*
> -	 * RCU_INIT_POINTER can be used here,
> -	 * seqcount provides the necessary barriers
> +	 * We are not changing the effective set of fences here so can
> +	 * merely update the pointer to the new array; both existing
> +	 * readers and new readers will see exactly the same set of
> +	 * active (unsignaled) shared fences. Individual fences and the
> +	 * old array are protected by RCU and so will not vanish under
> +	 * the gaze of the rcu_read_lock() readers.
>  	 */
> -	RCU_INIT_POINTER(obj->fence, new);
> -	write_seqcount_end(&obj->seq);
> -	preempt_enable();
> +	rcu_assign_pointer(obj->fence, new);
>  
>  	if (!old)
>  		return 0;
> -- 
> 2.22.0
>
Chris Wilson July 16, 2019, 8:05 p.m. UTC | #2
Quoting Daniel Vetter (2019-07-16 10:21:54)
> On Fri, Jul 12, 2019 at 09:03:14AM +0100, Chris Wilson wrote:
> > As the set of shared fences is not being changed during reallocation of
> > the reservation list, we can skip updating the write_seqlock.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Christian König <christian.koenig@amd.com>
> 
> sounds legit.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> More seriously, I think I convinced myself that we cant see a mess of old
> and new fence arrays anywhere, even without the seqlock retry, so I think
> we should be all good.

Aye, the view remains consistent which is key. Thanks for the review,
pushed to drm-misc-next.
-Chris
diff mbox series

Patch

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 80ecc1283d15..c71b85c8c159 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -157,15 +157,15 @@  int reservation_object_reserve_shared(struct reservation_object *obj,
 		(ksize(new) - offsetof(typeof(*new), shared)) /
 		sizeof(*new->shared);
 
-	preempt_disable();
-	write_seqcount_begin(&obj->seq);
 	/*
-	 * RCU_INIT_POINTER can be used here,
-	 * seqcount provides the necessary barriers
+	 * We are not changing the effective set of fences here so can
+	 * merely update the pointer to the new array; both existing
+	 * readers and new readers will see exactly the same set of
+	 * active (unsignaled) shared fences. Individual fences and the
+	 * old array are protected by RCU and so will not vanish under
+	 * the gaze of the rcu_read_lock() readers.
 	 */
-	RCU_INIT_POINTER(obj->fence, new);
-	write_seqcount_end(&obj->seq);
-	preempt_enable();
+	rcu_assign_pointer(obj->fence, new);
 
 	if (!old)
 		return 0;