Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function

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

 



On 15.02.2017 04:16, zhoucm1 wrote:


On 2017年02月14日 18:37, Nicolai Hähnle wrote:
From: Nicolai Hähnle <nicolai.haehnle@xxxxxxx>

Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.

Signed-off-by: Nicolai Hähnle <nicolai.haehnle@xxxxxxx>
---
  drivers/gpu/drm/ttm/ttm_bo.c | 62
+++++++++++++++++++++++++++++---------------
  include/drm/ttm/ttm_bo_api.h | 45 ++++++++++++++++++++++++++++++++
  2 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
  }
  EXPORT_SYMBOL(ttm_bo_validate);
  -int ttm_bo_init(struct ttm_bo_device *bdev,
-        struct ttm_buffer_object *bo,
-        unsigned long size,
-        enum ttm_bo_type type,
-        struct ttm_placement *placement,
-        uint32_t page_alignment,
-        bool interruptible,
-        struct file *persistent_swap_storage,
-        size_t acc_size,
-        struct sg_table *sg,
-        struct reservation_object *resv,
-        void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+            struct ttm_buffer_object *bo,
+            unsigned long size,
+            enum ttm_bo_type type,
+            uint32_t page_alignment,
+            struct file *persistent_swap_storage,
+            size_t acc_size,
+            struct sg_table *sg,
+            struct reservation_object *resv,
+            void (*destroy) (struct ttm_buffer_object *))
  {
      int ret = 0;
      unsigned long num_pages;
      struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
-    bool locked;
        ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
      if (ret) {
          pr_err("Out of kernel memory\n");
-        if (destroy)
-            (*destroy)(bo);
-        else
-            kfree(bo);
          return -ENOMEM;
      }
        num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
      if (num_pages == 0) {
          pr_err("Illegal buffer object size\n");
-        if (destroy)
-            (*destroy)(bo);
-        else
-            kfree(bo);
          ttm_mem_global_free(mem_glob, acc_size);
          return -EINVAL;
      }
@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
          ret = drm_vma_offset_add(&bdev->vma_manager, &bo->vma_node,
                       bo->mem.num_pages);
if (ret && !resv), we should call
reservation_object_fini(&bo->ttm_resv), right?

FWIW, you were right about this (and also mutex_destroy needs to be called for wu_mutex, etc.). But I'm following Christian's suggestion of having the caller use ttm_bo_unref for error cleanup, so all this error cleanup needn't be duplicated.

Cheers,
Nicola
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux