[PATCH libdrm] amdgpu: add a function to create amdgpu bo internally

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

 



> 
> 
> > +       r = amdgpu_bo_create(dev, alloc_buffer->alloc_size, args.out.handle,
> > +                            buf_handle);
> >          if (r)
> > -               amdgpu_bo_free(bo);
> > -       else
> > -               *buf_handle = bo;
> > +               goto out;
> > [yuq] the handle is not freed.
> 
> Do you mean to free buf_handle?
> in the bo_create, the failure means bo is failed to create where there is no bo
> created actually.
> 
> [yuq] I mean the args.out.handle.

(the network is so poor, recently...)
That looks an old bug, I could fix it later.
Thanks.

Regards,
Jerry

> 
> Regards,
> Qiang
> 
> >
> > +       pthread_mutex_lock(&dev->bo_table_mutex);
> > +       r = handle_table_insert(&dev->bo_handles, (*buf_handle)->handle,
> > +                               *buf_handle);
> > +       pthread_mutex_unlock(&dev->bo_table_mutex);
> > +       if (r)
> > +               amdgpu_bo_free(*buf_handle);
> > +out:
> >          return r;
> >   }
> >
> > @@ -256,7 +266,9 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
> >   {
> >          struct drm_gem_open open_arg = {};
> >          struct amdgpu_bo *bo = NULL;
> > -       int r;
> > +       uint32_t handle = 0, flink_name = 0;
> > +       uint64_t alloc_size = 0;
> > +       int r = 0;
> >          int dma_fd;
> >          uint64_t dma_buf_size = 0;
> >
> > @@ -266,22 +278,18 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
> >
> >          /* Convert a DMA buf handle to a KMS handle now. */
> >          if (type == amdgpu_bo_handle_type_dma_buf_fd) {
> > -               uint32_t handle;
> >                  off_t size;
> >
> >                  /* Get a KMS handle. */
> >                  r = drmPrimeFDToHandle(dev->fd, shared_handle, &handle);
> > -               if (r) {
> > -                       pthread_mutex_unlock(&dev->bo_table_mutex);
> > -                       return r;
> > -               }
> > +               if (r)
> > +                       goto unlock;
> >
> >                  /* Query the buffer size. */
> >                  size = lseek(shared_handle, 0, SEEK_END);
> >                  if (size == (off_t)-1) {
> > -                       pthread_mutex_unlock(&dev->bo_table_mutex);
> > -                       amdgpu_close_kms_handle(dev, handle);
> > -                       return -errno;
> > +                       r = -errno;
> > +                       goto unlock;
> >                  }
> >                  lseek(shared_handle, 0, SEEK_SET);
> >
> > @@ -320,58 +328,32 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
> >                  return 0;
> >          }
> >
> > -       bo = calloc(1, sizeof(struct amdgpu_bo));
> > -       if (!bo) {
> > -               pthread_mutex_unlock(&dev->bo_table_mutex);
> > -               if (type == amdgpu_bo_handle_type_dma_buf_fd) {
> > -                       amdgpu_close_kms_handle(dev, shared_handle);
> > -               }
> > -               return -ENOMEM;
> > -       }
> > -
> >          /* Open the handle. */
> >          switch (type) {
> >          case amdgpu_bo_handle_type_gem_flink_name:
> >                  open_arg.name = shared_handle;
> >                  r = drmIoctl(dev->flink_fd, DRM_IOCTL_GEM_OPEN, &open_arg);
> > -               if (r) {
> > -                       free(bo);
> > -                       pthread_mutex_unlock(&dev->bo_table_mutex);
> > -                       return r;
> > -               }
> > +               if (r)
> > +                       goto unlock;
> >
> > -               bo->handle = open_arg.handle;
> > +               flink_name = shared_handle;
> > +               handle = open_arg.handle;
> > +               alloc_size = open_arg.size;
> >                  if (dev->flink_fd != dev->fd) {
> > -                       r = drmPrimeHandleToFD(dev->flink_fd, bo->handle,
> DRM_CLOEXEC, &dma_fd);
> > -                       if (r) {
> > -                               free(bo);
> > -                               pthread_mutex_unlock(&dev->bo_table_mutex);
> > -                               return r;
> > -                       }
> > -                       r = drmPrimeFDToHandle(dev->fd, dma_fd, &bo->handle );
> > -
> > +                       r = drmPrimeHandleToFD(dev->flink_fd, handle,
> > +                                              DRM_CLOEXEC, &dma_fd);
> > +                       if (r)
> > +                               goto unlock;
> > +                       r = drmPrimeFDToHandle(dev->fd, dma_fd,
> > + &handle);
> >                          close(dma_fd);
> > -
> > -                       if (r) {
> > -                               free(bo);
> > -                               pthread_mutex_unlock(&dev->bo_table_mutex);
> > -                               return r;
> > -                       }
> > -               }
> > -               bo->flink_name = shared_handle;
> > -               bo->alloc_size = open_arg.size;
> > -               r = handle_table_insert(&dev->bo_flink_names, shared_handle,
> > -                                       bo);
> > -               if (r) {
> > -                       pthread_mutex_unlock(&dev->bo_table_mutex);
> > -                       amdgpu_bo_free(bo);
> > -                       return r;
> > +                       if (r)
> > +                               goto unlock;
> >                  }
> >                  break;
> >
> >          case amdgpu_bo_handle_type_dma_buf_fd:
> > -               bo->handle = shared_handle;
> > -               bo->alloc_size = dma_buf_size;
> > +               handle = shared_handle;
> > +               alloc_size = dma_buf_size;
> >                  break;
> >
> >          case amdgpu_bo_handle_type_kms:
> > @@ -380,16 +362,28 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
> >          }
> >
> >          /* Initialize it. */
> > -       atomic_set(&bo->refcount, 1);
> > -       bo->dev = dev;
> > -       pthread_mutex_init(&bo->cpu_access_mutex, NULL);
> > -
> > -       handle_table_insert(&dev->bo_handles, bo->handle, bo);
> > -       pthread_mutex_unlock(&dev->bo_table_mutex);
> > +       amdgpu_bo_create(dev, alloc_size, handle, &bo);
> > [yuq] no check return value.
> 
> good catch, will update that, thanks.
> 
> Regards,
> Jerry
> 
> >
> > +       if (flink_name) {
> > +               r = handle_table_insert(&dev->bo_flink_names, flink_name,
> > +                                       bo);
> > +               if (r) {
> > +                       amdgpu_bo_free(bo);
> > +                       goto unlock;
> > +               }
> > +       }
> > +       r = handle_table_insert(&dev->bo_handles, bo->handle, bo);
> > +       if (r) {
> > +               amdgpu_bo_free(bo);
> > +               goto unlock;
> > +       }
> >
> >          output->buf_handle = bo;
> >          output->alloc_size = bo->alloc_size;
> > -       return 0;
> > +unlock:
> > +       pthread_mutex_unlock(&dev->bo_table_mutex);
> > +       if (r && type == amdgpu_bo_handle_type_dma_buf_fd && handle)
> > +               amdgpu_close_kms_handle(dev, handle);
> > +       return r;
> >   }
> >
> >   int amdgpu_bo_free(amdgpu_bo_handle buf_handle) @@ -574,7 +568,6 @@
> > int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
> >                                      amdgpu_bo_handle *buf_handle)
> >   {
> >          int r;
> > -       struct amdgpu_bo *bo;
> >          struct drm_amdgpu_gem_userptr args;
> >
> >          args.addr = (uintptr_t)cpu;
> > @@ -584,28 +577,19 @@ int
> amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
> >          r = drmCommandWriteRead(dev->fd, DRM_AMDGPU_GEM_USERPTR,
> >                                  &args, sizeof(args));
> >          if (r)
> > -               return r;
> > -
> > -       bo = calloc(1, sizeof(struct amdgpu_bo));
> > -       if (!bo)
> > -               return -ENOMEM;
> > -
> > -       atomic_set(&bo->refcount, 1);
> > -       bo->dev = dev;
> > -       bo->alloc_size = size;
> > -       bo->handle = args.handle;
> > -
> > -       pthread_mutex_lock(&bo->dev->bo_table_mutex);
> > -       r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
> > -       pthread_mutex_unlock(&bo->dev->bo_table_mutex);
> > -
> > -       pthread_mutex_init(&bo->cpu_access_mutex, NULL);
> > +               goto out;
> >
> > +       r = amdgpu_bo_create(dev, size, args.handle, buf_handle);
> >          if (r)
> > -               amdgpu_bo_free(bo);
> > -       else
> > -               *buf_handle = bo;
> > +               goto out;
> > [yuq] handle is not freed.
> >
> > +       pthread_mutex_lock(&dev->bo_table_mutex);
> > +       r = handle_table_insert(&dev->bo_handles, (*buf_handle)->handle,
> > +                               *buf_handle);
> > +       pthread_mutex_unlock(&dev->bo_table_mutex);
> > +       if (r)
> > +               amdgpu_bo_free(*buf_handle);
> > +out:
> >          return r;
> >   }
> >
> > --
> > 1.9.1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

  Powered by Linux