2019-11-06 11:37:45

by Pan Bian

[permalink] [raw]
Subject: [PATCH v2] drm/amdgpu: fix double reference dropping

The reference to object fence is dropped at the end of the loop.
However, it is dropped again outside the loop. The reference can be
dropped immediately after calling dma_fence_wait() in the loop and
thus the dropping operation outside the loop can be removed.

Signed-off-by: Pan Bian <[email protected]>
---
v2: fix the bug in a more concise way
---
drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
index 649e68c4479b..d1495e1c9289 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -33,7 +33,7 @@ static int amdgpu_benchmark_do_move(struct amdgpu_device *adev, unsigned size,
{
unsigned long start_jiffies;
unsigned long end_jiffies;
- struct dma_fence *fence = NULL;
+ struct dma_fence *fence;
int i, r;

start_jiffies = jiffies;
@@ -44,16 +44,14 @@ static int amdgpu_benchmark_do_move(struct amdgpu_device *adev, unsigned size,
if (r)
goto exit_do_move;
r = dma_fence_wait(fence, false);
+ dma_fence_put(fence);
if (r)
goto exit_do_move;
- dma_fence_put(fence);
}
end_jiffies = jiffies;
r = jiffies_to_msecs(end_jiffies - start_jiffies);

exit_do_move:
- if (fence)
- dma_fence_put(fence);
return r;
}

--
2.7.4


2019-11-06 12:50:22

by Christian König

[permalink] [raw]
Subject: Re: [PATCH v2] drm/amdgpu: fix double reference dropping

Am 06.11.19 um 12:35 schrieb Pan Bian:
> The reference to object fence is dropped at the end of the loop.
> However, it is dropped again outside the loop. The reference can be
> dropped immediately after calling dma_fence_wait() in the loop and
> thus the dropping operation outside the loop can be removed.
>
> Signed-off-by: Pan Bian <[email protected]>

Reviewed-by: Christian König <[email protected]>

> ---
> v2: fix the bug in a more concise way
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> index 649e68c4479b..d1495e1c9289 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> @@ -33,7 +33,7 @@ static int amdgpu_benchmark_do_move(struct amdgpu_device *adev, unsigned size,
> {
> unsigned long start_jiffies;
> unsigned long end_jiffies;
> - struct dma_fence *fence = NULL;
> + struct dma_fence *fence;
> int i, r;
>
> start_jiffies = jiffies;
> @@ -44,16 +44,14 @@ static int amdgpu_benchmark_do_move(struct amdgpu_device *adev, unsigned size,
> if (r)
> goto exit_do_move;
> r = dma_fence_wait(fence, false);
> + dma_fence_put(fence);
> if (r)
> goto exit_do_move;
> - dma_fence_put(fence);
> }
> end_jiffies = jiffies;
> r = jiffies_to_msecs(end_jiffies - start_jiffies);
>
> exit_do_move:
> - if (fence)
> - dma_fence_put(fence);
> return r;
> }
>

2019-11-06 17:26:53

by Alex Deucher

[permalink] [raw]
Subject: Re: [PATCH v2] drm/amdgpu: fix double reference dropping

On Wed, Nov 6, 2019 at 7:48 AM Koenig, Christian
<[email protected]> wrote:
>
> Am 06.11.19 um 12:35 schrieb Pan Bian:
> > The reference to object fence is dropped at the end of the loop.
> > However, it is dropped again outside the loop. The reference can be
> > dropped immediately after calling dma_fence_wait() in the loop and
> > thus the dropping operation outside the loop can be removed.
> >
> > Signed-off-by: Pan Bian <[email protected]>
>
> Reviewed-by: Christian König <[email protected]>
>

Applied. thanks!

Alex

> > ---
> > v2: fix the bug in a more concise way
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> > index 649e68c4479b..d1495e1c9289 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> > @@ -33,7 +33,7 @@ static int amdgpu_benchmark_do_move(struct amdgpu_device *adev, unsigned size,
> > {
> > unsigned long start_jiffies;
> > unsigned long end_jiffies;
> > - struct dma_fence *fence = NULL;
> > + struct dma_fence *fence;
> > int i, r;
> >
> > start_jiffies = jiffies;
> > @@ -44,16 +44,14 @@ static int amdgpu_benchmark_do_move(struct amdgpu_device *adev, unsigned size,
> > if (r)
> > goto exit_do_move;
> > r = dma_fence_wait(fence, false);
> > + dma_fence_put(fence);
> > if (r)
> > goto exit_do_move;
> > - dma_fence_put(fence);
> > }
> > end_jiffies = jiffies;
> > r = jiffies_to_msecs(end_jiffies - start_jiffies);
> >
> > exit_do_move:
> > - if (fence)
> > - dma_fence_put(fence);
> > return r;
> > }
> >
>
> _______________________________________________
> amd-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx