[PATCH 1/7] drm/ttm: call ttm_bo_swapout directly when ttm shrink
Thomas Hellstrom
thomas at shipmail.org
Thu Dec 21 07:34:10 UTC 2017
Roger,
5 out of 7 patches in this series are completely lacking a commit log
message, and thats not OK. Really.
https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html#describe-your-changes
I'll review these, but IIRC the no_wait in the memory accounting code is
different in that it doesn't allow sleeping,
whereas in the bo code we had originally had no_wait_gpu and also
no_wait. (IIRC Maarten or Jerome removed the latter due to lack of
users.) Seems like these patches confuse the to. At the very least that
requires some form of motivation.
Also I wonder what testing is being performed on these changes prior to
submission?
We have pretty high number of critical deployments out there, that we
need to support.
/Thomas
On 12/20/2017 11:34 AM, Roger He wrote:
> then remove superfluous functions
>
> Change-Id: Iea020f0e30a239e0265e7a1500168c7d7f819bd9
> Signed-off-by: Roger He <Hongbo.He at amd.com>
> ---
> 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,
More information about the amd-gfx
mailing list