mutex_destroy in amdgpu_ctx_fini()

mutex_destroy in amdgpu_ctx_fini()

[StDenis, Tom]

In this block of code I'm wondering if the mutex_destroy should be hoisted out of the inner loop?

static void amdgpu_ctx_fini(struct amdgpu_ctx *ctx)
{
        struct amdgpu_device *adev = ctx->adev;
        unsigned i, j;


        if (!adev)
                return;


        for (i = 0; i < AMDGPU_MAX_RINGS; ++i)
                for (j = 0; j < amdgpu_sched_jobs; ++j) {
                        fence_put(ctx->rings[i].fences[j]);
                        mutex_destroy(&ctx->rings[i].sem_lock);
                }


Cheers,
Tom
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: mutex_destroy in amdgpu_ctx_fini()

[Christian König]

At least of hand that strongly looks like a bug.

Probably doesn't do much harm cause mutex_destroy() is mostly for 
lockdep annotation, but still should be fixed.

Christian.

Am 29.09.2016 um 13:18 schrieb StDenis, Tom:
> In this block of code I'm wondering if the mutex_destroy should be hoisted out of the inner loop?
>
> static void amdgpu_ctx_fini(struct amdgpu_ctx *ctx)
> {
>          struct amdgpu_device *adev = ctx->adev;
>          unsigned i, j;
>
>
>          if (!adev)
>                  return;
>
>
>          for (i = 0; i < AMDGPU_MAX_RINGS; ++i)
>                  for (j = 0; j < amdgpu_sched_jobs; ++j) {
>                          fence_put(ctx->rings[i].fences[j]);
>                          mutex_destroy(&ctx->rings[i].sem_lock);
>                  }
>
>
> Cheers,
> Tom
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: mutex_destroy in amdgpu_ctx_fini()

[zhoucm1]

Good catch, Could you make a patch?

Thanks,
David Zhou

On 2016年09月29日 19:18, StDenis, Tom wrote:
> In this block of code I'm wondering if the mutex_destroy should be hoisted out of the inner loop?
>
> static void amdgpu_ctx_fini(struct amdgpu_ctx *ctx)
> {
>          struct amdgpu_device *adev = ctx->adev;
>          unsigned i, j;
>
>
>          if (!adev)
>                  return;
>
>
>          for (i = 0; i < AMDGPU_MAX_RINGS; ++i)
>                  for (j = 0; j < amdgpu_sched_jobs; ++j) {
>                          fence_put(ctx->rings[i].fences[j]);
>                          mutex_destroy(&ctx->rings[i].sem_lock);
>                  }
>
>
> Cheers,
> Tom
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx