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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Am 31.08.21 um 07:47 schrieb Pan, Xinhui:

在 2021/8/31 13:38,“Pan, Xinhui”<Xinhui.Pan@xxxxxxx> 写入:

在 2021/8/31 12:03,“Grodzovsky, Andrey”<Andrey.Grodzovsky@xxxxxxx> 写入: 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@xxxxxxx>
         > ---
         >   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

Also we can do something like below which is simpler.

Yeah, I think that would be better. We could also change the "if (r) { if (r != -ERESTARTSYS) {" into just "if (r != -ERESTARTSYS)" then.

But good catch anyway.

Regards,
Christian.

@@ -355,7 +355,6 @@ 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);
                 }
-               return r;
         }
if (flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID) { >
         >          r = drm_gem_handle_create(filp, gobj, &handle);
         >          /* drop reference from allocate - handle holds it now */
         > --
         > 2.25.1




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux