Re: [PATCH 1/7] drm/ttm: call ttm_bo_swapout directly when ttm shrink

["Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Am 20.12.2017 um 11:34 schrieb Roger He:
> then remove superfluous functions

We need a better commit message. Something like:

Remove the extra indirection, cause we have only one implementation anyway.

>
> Change-Id: Iea020f0e30a239e0265e7a1500168c7d7f819bd9
> Signed-off-by: Roger He <Hongbo.He@amd.com>

With the commit message fixed Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c     | 21 +++---------
>   drivers/gpu/drm/ttm/ttm_memory.c | 12 ++-----
>   include/drm/ttm/ttm_bo_api.h     |  1 +
>   include/drm/ttm/ttm_bo_driver.h  |  1 -
>   include/drm/ttm/ttm_memory.h     | 69 +---------------------------------------
>   5 files changed, 9 insertions(+), 95 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 60bb5c1..fa57aa8 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -42,7 +42,6 @@
>   #include <linux/atomic.h>
>   #include <linux/reservation.h>
>   
> -static int ttm_bo_swapout(struct ttm_mem_shrink *shrink);
>   static void ttm_bo_global_kobj_release(struct kobject *kobj);
>   
>   static struct attribute ttm_bo_count = {
> @@ -1454,7 +1453,6 @@ static void ttm_bo_global_kobj_release(struct kobject *kobj)
>   	struct ttm_bo_global *glob =
>   		container_of(kobj, struct ttm_bo_global, kobj);
>   
> -	ttm_mem_unregister_shrink(glob->mem_glob, &glob->shrink);
>   	__free_page(glob->dummy_read_page);
>   	kfree(glob);
>   }
> @@ -1479,6 +1477,7 @@ int ttm_bo_global_init(struct drm_global_reference *ref)
>   	mutex_init(&glob->device_list_mutex);
>   	spin_lock_init(&glob->lru_lock);
>   	glob->mem_glob = bo_ref->mem_glob;
> +	glob->mem_glob->bo_glob = glob;
>   	glob->dummy_read_page = alloc_page(__GFP_ZERO | GFP_DMA32);
>   
>   	if (unlikely(glob->dummy_read_page == NULL)) {
> @@ -1489,14 +1488,6 @@ int ttm_bo_global_init(struct drm_global_reference *ref)
>   	for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
>   		INIT_LIST_HEAD(&glob->swap_lru[i]);
>   	INIT_LIST_HEAD(&glob->device_list);
> -
> -	ttm_mem_init_shrink(&glob->shrink, ttm_bo_swapout);
> -	ret = ttm_mem_register_shrink(glob->mem_glob, &glob->shrink);
> -	if (unlikely(ret != 0)) {
> -		pr_err("Could not register buffer object swapout\n");
> -		goto out_no_shrink;
> -	}
> -
>   	atomic_set(&glob->bo_count, 0);
>   
>   	ret = kobject_init_and_add(
> @@ -1504,8 +1495,6 @@ int ttm_bo_global_init(struct drm_global_reference *ref)
>   	if (unlikely(ret != 0))
>   		kobject_put(&glob->kobj);
>   	return ret;
> -out_no_shrink:
> -	__free_page(glob->dummy_read_page);
>   out_no_drp:
>   	kfree(glob);
>   	return ret;
> @@ -1688,11 +1677,8 @@ EXPORT_SYMBOL(ttm_bo_synccpu_write_release);
>    * A buffer object shrink method that tries to swap out the first
>    * buffer object on the bo_global::swap_lru list.
>    */
> -
> -static int ttm_bo_swapout(struct ttm_mem_shrink *shrink)
> +int ttm_bo_swapout(struct ttm_bo_global *glob)
>   {
> -	struct ttm_bo_global *glob =
> -	    container_of(shrink, struct ttm_bo_global, shrink);
>   	struct ttm_buffer_object *bo;
>   	int ret = -EBUSY;
>   	unsigned i;
> @@ -1774,10 +1760,11 @@ static int ttm_bo_swapout(struct ttm_mem_shrink *shrink)
>   	kref_put(&bo->list_kref, ttm_bo_release_list);
>   	return ret;
>   }
> +EXPORT_SYMBOL(ttm_bo_swapout);
>   
>   void ttm_bo_swapout_all(struct ttm_bo_device *bdev)
>   {
> -	while (ttm_bo_swapout(&bdev->glob->shrink) == 0)
> +	while (ttm_bo_swapout(bdev->glob) == 0)
>   		;
>   }
>   EXPORT_SYMBOL(ttm_bo_swapout_all);
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
> index e963749..9130bdf 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -214,26 +214,20 @@ static void ttm_shrink(struct ttm_mem_global *glob, bool from_wq,
>   		       uint64_t extra)
>   {
>   	int ret;
> -	struct ttm_mem_shrink *shrink;
>   
>   	spin_lock(&glob->lock);
> -	if (glob->shrink == NULL)
> -		goto out;
>   
>   	while (ttm_zones_above_swap_target(glob, from_wq, extra)) {
> -		shrink = glob->shrink;
>   		spin_unlock(&glob->lock);
> -		ret = shrink->do_shrink(shrink);
> +		ret = ttm_bo_swapout(glob->bo_glob);
>   		spin_lock(&glob->lock);
>   		if (unlikely(ret != 0))
> -			goto out;
> +			break;
>   	}
> -out:
> +
>   	spin_unlock(&glob->lock);
>   }
>   
> -
> -
>   static void ttm_shrink_work(struct work_struct *work)
>   {
>   	struct ttm_mem_global *glob =
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index c126330..24a8db7 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -752,6 +752,7 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
>   		  const char __user *wbuf, char __user *rbuf,
>   		  size_t count, loff_t *f_pos, bool write);
>   
> +int ttm_bo_swapout(struct ttm_bo_global *glob);
>   void ttm_bo_swapout_all(struct ttm_bo_device *bdev);
>   int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo);
>   #endif
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 5115718..934fecf 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -522,7 +522,6 @@ struct ttm_bo_global {
>   	struct kobject kobj;
>   	struct ttm_mem_global *mem_glob;
>   	struct page *dummy_read_page;
> -	struct ttm_mem_shrink shrink;
>   	struct mutex device_list_mutex;
>   	spinlock_t lru_lock;
>   
> diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h
> index 2c1e359..85f3ad6 100644
> --- a/include/drm/ttm/ttm_memory.h
> +++ b/include/drm/ttm/ttm_memory.h
> @@ -37,20 +37,6 @@
>   #include <linux/mm.h>
>   
>   /**
> - * struct ttm_mem_shrink - callback to shrink TTM memory usage.
> - *
> - * @do_shrink: The callback function.
> - *
> - * Arguments to the do_shrink functions are intended to be passed using
> - * inheritance. That is, the argument class derives from struct ttm_mem_shrink,
> - * and can be accessed using container_of().
> - */
> -
> -struct ttm_mem_shrink {
> -	int (*do_shrink) (struct ttm_mem_shrink *);
> -};
> -
> -/**
>    * struct ttm_mem_global - Global memory accounting structure.
>    *
>    * @shrink: A single callback to shrink TTM memory usage. Extend this
> @@ -76,7 +62,7 @@ struct ttm_mem_shrink {
>   struct ttm_mem_zone;
>   struct ttm_mem_global {
>   	struct kobject kobj;
> -	struct ttm_mem_shrink *shrink;
> +	struct ttm_bo_global *bo_glob;
>   	struct workqueue_struct *swap_queue;
>   	struct work_struct work;
>   	spinlock_t lock;
> @@ -90,59 +76,6 @@ struct ttm_mem_global {
>   #endif
>   };
>   
> -/**
> - * ttm_mem_init_shrink - initialize a struct ttm_mem_shrink object
> - *
> - * @shrink: The object to initialize.
> - * @func: The callback function.
> - */
> -
> -static inline void ttm_mem_init_shrink(struct ttm_mem_shrink *shrink,
> -				       int (*func) (struct ttm_mem_shrink *))
> -{
> -	shrink->do_shrink = func;
> -}
> -
> -/**
> - * ttm_mem_register_shrink - register a struct ttm_mem_shrink object.
> - *
> - * @glob: The struct ttm_mem_global object to register with.
> - * @shrink: An initialized struct ttm_mem_shrink object to register.
> - *
> - * Returns:
> - * -EBUSY: There's already a callback registered. (May change).
> - */
> -
> -static inline int ttm_mem_register_shrink(struct ttm_mem_global *glob,
> -					  struct ttm_mem_shrink *shrink)
> -{
> -	spin_lock(&glob->lock);
> -	if (glob->shrink != NULL) {
> -		spin_unlock(&glob->lock);
> -		return -EBUSY;
> -	}
> -	glob->shrink = shrink;
> -	spin_unlock(&glob->lock);
> -	return 0;
> -}
> -
> -/**
> - * ttm_mem_unregister_shrink - unregister a struct ttm_mem_shrink object.
> - *
> - * @glob: The struct ttm_mem_global object to unregister from.
> - * @shrink: A previously registert struct ttm_mem_shrink object.
> - *
> - */
> -
> -static inline void ttm_mem_unregister_shrink(struct ttm_mem_global *glob,
> -					     struct ttm_mem_shrink *shrink)
> -{
> -	spin_lock(&glob->lock);
> -	BUG_ON(glob->shrink != shrink);
> -	glob->shrink = NULL;
> -	spin_unlock(&glob->lock);
> -}
> -
>   extern int ttm_mem_global_init(struct ttm_mem_global *glob);
>   extern void ttm_mem_global_release(struct ttm_mem_global *glob);
>   extern int ttm_mem_global_alloc(struct ttm_mem_global *glob, uint64_t memory,

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx