On Tue, 2022-03-29 at 13:02 +0200, Christian König wrote:
> ⚠ External Email
>
> It's the only driver using this.
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
Looks good. A small suggestion underneath.
Reviewed-by: Zack Rusin <zackr@xxxxxxxxxx>
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 9 +--------
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 11 ++++++++++-
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
> b/drivers/gpu/drm/ttm/ttm_bo.c
> index e5fd0f2c0299..7598d59423bf 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -44,12 +44,6 @@
>
> #include "ttm_module.h"
>
> -/* default destructor */
> -static void ttm_bo_default_destroy(struct ttm_buffer_object *bo)
> -{
> - kfree(bo);
> -}
> -
> static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
> struct ttm_placement
> *placement)
> {
> @@ -938,8 +932,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
> bool locked;
> int ret;
>
> - bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
> -
> + bo->destroy = destroy;
> kref_init(&bo->kref);
> INIT_LIST_HEAD(&bo->ddestroy);
> bo->bdev = bdev;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 31aecc46624b..60dcc6a75248 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -378,6 +378,12 @@ void vmw_bo_bo_free(struct ttm_buffer_object
> *bo)
> kfree(vmw_bo);
> }
>
> +/* default destructor */
> +static void vmw_bo_default_destroy(struct ttm_buffer_object *bo)
> +{
> + kfree(bo);
> +}
> +
> /**
> * vmw_bo_create_kernel - Create a pinned BO for internal kernel
> use.
> *
> @@ -410,7 +416,7 @@ int vmw_bo_create_kernel(struct vmw_private
> *dev_priv, unsigned long size,
>
> ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size,
> ttm_bo_type_kernel, placement, 0,
> - &ctx, NULL, NULL, NULL);
> + &ctx, NULL, NULL,
> vmw_bo_default_destroy);
> if (unlikely(ret))
> goto error_free;
>
> @@ -439,6 +445,9 @@ int vmw_bo_create(struct vmw_private *vmw,
> return -ENOMEM;
> }
>
> + if (!bo_free)
> + bo_free = vmw_bo_default_destroy;
> +
If you could change this to just BUG_ON(!bo_free) that'd be great.
bo_free == NULL should never happen here.
z
