[PATCH] drm/amdgpu: Fix a deadlock if previous GEM object allocation fails

[Pan, Xinhui]

在 2021/8/31 12:03，“Grodzovsky, Andrey”<Andrey.Grodzovsky at amd.com> 写入:

    
    On 2021-08-30 11:24 p.m., Pan, Xinhui wrote:
    > [AMD Official Use Only]
    >
    > [AMD Official Use Only]
    >
    > Unreserve root BO before return otherwise next allocation got deadlock.
    >
    > Signed-off-by: xinhui pan <xinhui.pan at amd.com>
    > ---
    >   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 11 +++++------
    >   1 file changed, 5 insertions(+), 6 deletions(-)
    >
    > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
    > index 85b292ed5c43..c9db7d2c5816 100644
    > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
    > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
    > @@ -355,19 +355,18 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
    >                          DRM_DEBUG("Failed to allocate GEM object (%llu, %d, %llu, %d)\n",
    >                                    size, initial_domain, args->in.alignment, r);
    >                  }
    > +
    > +               if (flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID)
    > +                       amdgpu_bo_unreserve(vm->root.bo);
    >                  return r;
    >          }
    >
    >          if (flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID) {
    > -               if (!r) {
    > -                       struct amdgpu_bo *abo = gem_to_amdgpu_bo(gobj);
    > +               struct amdgpu_bo *abo = gem_to_amdgpu_bo(gobj);
    >
    > -                       abo->parent = amdgpu_bo_ref(vm->root.bo);
    > -               }
    > +               abo->parent = amdgpu_bo_ref(vm->root.bo);
    >                  amdgpu_bo_unreserve(vm->root.bo);
    >          }
    > -       if (r)
    > -               return r;
    
    
    The above early return seems to be needed for -ERESTARTSYS case.
    
    Andrey

There are two returns. ERESTARTSYS and other error after retry are already handled by the first return in if {}. So the second return is not needed.

Thanks
xinhui
    
    
    >
    >          r = drm_gem_handle_create(filp, gobj, &handle);
    >          /* drop reference from allocate - handle holds it now */
    > --
    > 2.25.1