Am 31.08.21 um 07:47 schrieb Pan, Xinhui:

在 2021/8/31 13:38,“Pan, Xinhui”<[email protected]> 写入:

在 2021/8/31 12:03,“Grodzovsky, Andrey”<[email protected]> 写入: 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 <[email protected]>
         > ---
         >   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

Reply via email to