dma-buf: fix reservation_object_wait_timeout_rcu to wait correctly v2
Commit Message
From: Christian König <christian.koenig@amd.com>
With hardware resets in mind it is possible that all shared fences are
signaled, but the exlusive isn't. Fix waiting for everything in this situation.
v2: make sure we always wait for the exclusive fence
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/dma-buf/reservation.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)
Comments
Ping, what do you guys think of that?
Am 25.07.2017 um 15:35 schrieb Christian König:
> From: Christian König <christian.koenig@amd.com>
>
> With hardware resets in mind it is possible that all shared fences are
> signaled, but the exlusive isn't. Fix waiting for everything in this situation.
>
> v2: make sure we always wait for the exclusive fence
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/dma-buf/reservation.c | 33 +++++++++++++++------------------
> 1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 393817e..9d4316d 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -373,12 +373,25 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
> long ret = timeout ? timeout : 1;
>
> retry:
> - fence = NULL;
> shared_count = 0;
> seq = read_seqcount_begin(&obj->seq);
> rcu_read_lock();
>
> - if (wait_all) {
> + fence = rcu_dereference(obj->fence_excl);
> + if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> + if (!dma_fence_get_rcu(fence))
> + goto unlock_retry;
> +
> + if (dma_fence_is_signaled(fence)) {
> + dma_fence_put(fence);
> + fence = NULL;
> + }
> +
> + } else {
> + fence = NULL;
> + }
> +
> + if (!fence && wait_all) {
> struct reservation_object_list *fobj =
> rcu_dereference(obj->fence);
>
> @@ -405,22 +418,6 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
> }
> }
>
> - if (!shared_count) {
> - struct dma_fence *fence_excl = rcu_dereference(obj->fence_excl);
> -
> - if (fence_excl &&
> - !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> - &fence_excl->flags)) {
> - if (!dma_fence_get_rcu(fence_excl))
> - goto unlock_retry;
> -
> - if (dma_fence_is_signaled(fence_excl))
> - dma_fence_put(fence_excl);
> - else
> - fence = fence_excl;
> - }
> - }
> -
> rcu_read_unlock();
> if (fence) {
> if (read_seqcount_retry(&obj->seq, seq)) {
> -----Original Message-----
> From: Christian König [mailto:deathsimple@vodafone.de]
> Sent: Monday, July 31, 2017 10:13 AM
> To: linux-media@vger.kernel.org; dri-devel@lists.freedesktop.org; linaro-
> mm-sig@lists.linaro.org; Zhou, David(ChunMing); Deucher, Alexander
> Subject: Re: [PATCH] dma-buf: fix reservation_object_wait_timeout_rcu to
> wait correctly v2
>
> Ping, what do you guys think of that?
Seems reasonable to me.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> Am 25.07.2017 um 15:35 schrieb Christian König:
> > From: Christian König <christian.koenig@amd.com>
> >
> > With hardware resets in mind it is possible that all shared fences are
> > signaled, but the exlusive isn't. Fix waiting for everything in this situation.
> >
> > v2: make sure we always wait for the exclusive fence
> >
> > Signed-off-by: Christian König <christian.koenig@amd.com>
> > ---
> > drivers/dma-buf/reservation.c | 33 +++++++++++++++------------------
> > 1 file changed, 15 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> > index 393817e..9d4316d 100644
> > --- a/drivers/dma-buf/reservation.c
> > +++ b/drivers/dma-buf/reservation.c
> > @@ -373,12 +373,25 @@ long reservation_object_wait_timeout_rcu(struct
> reservation_object *obj,
> > long ret = timeout ? timeout : 1;
> >
> > retry:
> > - fence = NULL;
> > shared_count = 0;
> > seq = read_seqcount_begin(&obj->seq);
> > rcu_read_lock();
> >
> > - if (wait_all) {
> > + fence = rcu_dereference(obj->fence_excl);
> > + if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence-
> >flags)) {
> > + if (!dma_fence_get_rcu(fence))
> > + goto unlock_retry;
> > +
> > + if (dma_fence_is_signaled(fence)) {
> > + dma_fence_put(fence);
> > + fence = NULL;
> > + }
> > +
> > + } else {
> > + fence = NULL;
> > + }
> > +
> > + if (!fence && wait_all) {
> > struct reservation_object_list *fobj =
> > rcu_dereference(obj-
> >fence);
> >
> > @@ -405,22 +418,6 @@ long reservation_object_wait_timeout_rcu(struct
> reservation_object *obj,
> > }
> > }
> >
> > - if (!shared_count) {
> > - struct dma_fence *fence_excl = rcu_dereference(obj-
> >fence_excl);
> > -
> > - if (fence_excl &&
> > - !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> > - &fence_excl->flags)) {
> > - if (!dma_fence_get_rcu(fence_excl))
> > - goto unlock_retry;
> > -
> > - if (dma_fence_is_signaled(fence_excl))
> > - dma_fence_put(fence_excl);
> > - else
> > - fence = fence_excl;
> > - }
> > - }
> > -
> > rcu_read_unlock();
> > if (fence) {
> > if (read_seqcount_retry(&obj->seq, seq)) {
>
On 2017?07?31? 23:39, Deucher, Alexander wrote:
>> -----Original Message-----
>> From: Christian König [mailto:deathsimple@vodafone.de]
>> Sent: Monday, July 31, 2017 10:13 AM
>> To: linux-media@vger.kernel.org; dri-devel@lists.freedesktop.org; linaro-
>> mm-sig@lists.linaro.org; Zhou, David(ChunMing); Deucher, Alexander
>> Subject: Re: [PATCH] dma-buf: fix reservation_object_wait_timeout_rcu to
>> wait correctly v2
>>
>> Ping, what do you guys think of that?
> Seems reasonable to me.
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Chunming Zhou <david1.zhou@amd.com> as well.
>
>> Am 25.07.2017 um 15:35 schrieb Christian König:
>>> From: Christian König <christian.koenig@amd.com>
>>>
>>> With hardware resets in mind it is possible that all shared fences are
>>> signaled, but the exlusive isn't. Fix waiting for everything in this situation.
>>>
>>> v2: make sure we always wait for the exclusive fence
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>> drivers/dma-buf/reservation.c | 33 +++++++++++++++------------------
>>> 1 file changed, 15 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
>>> index 393817e..9d4316d 100644
>>> --- a/drivers/dma-buf/reservation.c
>>> +++ b/drivers/dma-buf/reservation.c
>>> @@ -373,12 +373,25 @@ long reservation_object_wait_timeout_rcu(struct
>> reservation_object *obj,
>>> long ret = timeout ? timeout : 1;
>>>
>>> retry:
>>> - fence = NULL;
>>> shared_count = 0;
>>> seq = read_seqcount_begin(&obj->seq);
>>> rcu_read_lock();
>>>
>>> - if (wait_all) {
>>> + fence = rcu_dereference(obj->fence_excl);
>>> + if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence-
>>> flags)) {
>>> + if (!dma_fence_get_rcu(fence))
>>> + goto unlock_retry;
>>> +
>>> + if (dma_fence_is_signaled(fence)) {
>>> + dma_fence_put(fence);
>>> + fence = NULL;
>>> + }
>>> +
>>> + } else {
>>> + fence = NULL;
>>> + }
>>> +
>>> + if (!fence && wait_all) {
>>> struct reservation_object_list *fobj =
>>> rcu_dereference(obj-
>>> fence);
>>>
>>> @@ -405,22 +418,6 @@ long reservation_object_wait_timeout_rcu(struct
>> reservation_object *obj,
>>> }
>>> }
>>>
>>> - if (!shared_count) {
>>> - struct dma_fence *fence_excl = rcu_dereference(obj-
>>> fence_excl);
>>> -
>>> - if (fence_excl &&
>>> - !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>>> - &fence_excl->flags)) {
>>> - if (!dma_fence_get_rcu(fence_excl))
>>> - goto unlock_retry;
>>> -
>>> - if (dma_fence_is_signaled(fence_excl))
>>> - dma_fence_put(fence_excl);
>>> - else
>>> - fence = fence_excl;
>>> - }
>>> - }
>>> -
>>> rcu_read_unlock();
>>> if (fence) {
>>> if (read_seqcount_retry(&obj->seq, seq)) {
@@ -373,12 +373,25 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
long ret = timeout ? timeout : 1;
retry:
- fence = NULL;
shared_count = 0;
seq = read_seqcount_begin(&obj->seq);
rcu_read_lock();
- if (wait_all) {
+ fence = rcu_dereference(obj->fence_excl);
+ if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
+ if (!dma_fence_get_rcu(fence))
+ goto unlock_retry;
+
+ if (dma_fence_is_signaled(fence)) {
+ dma_fence_put(fence);
+ fence = NULL;
+ }
+
+ } else {
+ fence = NULL;
+ }
+
+ if (!fence && wait_all) {
struct reservation_object_list *fobj =
rcu_dereference(obj->fence);
@@ -405,22 +418,6 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj,
}
}
- if (!shared_count) {
- struct dma_fence *fence_excl = rcu_dereference(obj->fence_excl);
-
- if (fence_excl &&
- !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
- &fence_excl->flags)) {
- if (!dma_fence_get_rcu(fence_excl))
- goto unlock_retry;
-
- if (dma_fence_is_signaled(fence_excl))
- dma_fence_put(fence_excl);
- else
- fence = fence_excl;
- }
- }
-
rcu_read_unlock();
if (fence) {
if (read_seqcount_retry(&obj->seq, seq)) {