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,
