| Message ID | 20190626150522.11618-8-Kenny.Ho@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | new cgroup controller for gpu/drm subsystem | expand |
On Wed, Jun 26, 2019 at 11:05:18AM -0400, Kenny Ho wrote: > The drm resource being measured is the TTM (Translation Table Manager) > buffers. TTM manages different types of memory that a GPU might access. > These memory types include dedicated Video RAM (VRAM) and host/system > memory accessible through IOMMU (GART/GTT). TTM is currently used by > multiple drm drivers (amd, ast, bochs, cirrus, hisilicon, maga200, > nouveau, qxl, virtio, vmwgfx.) > > drm.memory.stats > A read-only nested-keyed file which exists on all cgroups. > Each entry is keyed by the drm device's major:minor. The > following nested keys are defined. > > ====== ============================================= > system Host/system memory Shouldn't that be covered by gem bo stats already? Also, system memory is definitely something a lot of non-ttm drivers want to be able to track, so that needs to be separate from ttm. > tt Host memory used by the drm device (GTT/GART) > vram Video RAM used by the drm device > priv Other drm device, vendor specific memory So what's "priv". In general I think we need some way to register the different kinds of memory, e.g. stuff not in your list: - multiple kinds of vram (like numa-style gpus) - cma (for all those non-ttm drivers that's a big one, it's like system memory but also totally different) - any carveouts and stuff > ====== ============================================= > > Reading returns the following:: > > 226:0 system=0 tt=0 vram=0 priv=0 > 226:1 system=0 tt=9035776 vram=17768448 priv=16809984 > 226:2 system=0 tt=9035776 vram=17768448 priv=16809984 > > drm.memory.evict.stats > A read-only flat-keyed file which exists on all cgroups. Each > entry is keyed by the drm device's major:minor. > > Total number of evictions. > > Change-Id: Ice2c4cc845051229549bebeb6aa2d7d6153bdf6a > Signed-off-by: Kenny Ho <Kenny.Ho@amd.com> I think with all the ttm refactoring going on I think we need to de-ttm the interface functions here a bit. With Gerd Hoffmans series you can just use a gem_bo pointer here, so what's left to do is have some extracted structure for tracking memory types. I think Brian Welty has some ideas for this, even in patch form. Would be good to keep him on cc at least for the next version. We'd need to explicitly hand in the ttm_mem_reg (or whatever the specific thing is going to be). -Daniel > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 +- > drivers/gpu/drm/ttm/ttm_bo.c | 30 +++++++ > drivers/gpu/drm/ttm/ttm_bo_util.c | 4 + > include/drm/drm_cgroup.h | 19 ++++ > include/drm/ttm/ttm_bo_api.h | 2 + > include/drm/ttm/ttm_bo_driver.h | 8 ++ > include/linux/cgroup_drm.h | 4 + > kernel/cgroup/drm.c | 113 ++++++++++++++++++++++++ > 8 files changed, 182 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index e9ecc3953673..a8dfc78ed45f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -1678,8 +1678,9 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) > mutex_init(&adev->mman.gtt_window_lock); > > /* No others user of address space so set it to 0 */ > - r = ttm_bo_device_init(&adev->mman.bdev, > + r = ttm_bo_device_init_tmp(&adev->mman.bdev, > &amdgpu_bo_driver, > + adev->ddev, > adev->ddev->anon_inode->i_mapping, > adev->need_dma32); > if (r) { > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 2845fceb2fbd..e9f70547f0ad 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -34,6 +34,7 @@ > #include <drm/ttm/ttm_module.h> > #include <drm/ttm/ttm_bo_driver.h> > #include <drm/ttm/ttm_placement.h> > +#include <drm/drm_cgroup.h> > #include <linux/jiffies.h> > #include <linux/slab.h> > #include <linux/sched.h> > @@ -42,6 +43,7 @@ > #include <linux/module.h> > #include <linux/atomic.h> > #include <linux/reservation.h> > +#include <linux/cgroup_drm.h> > > static void ttm_bo_global_kobj_release(struct kobject *kobj); > > @@ -151,6 +153,10 @@ static void ttm_bo_release_list(struct kref *list_kref) > struct ttm_bo_device *bdev = bo->bdev; > size_t acc_size = bo->acc_size; > > + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all > + drmcgrp_unchg_mem(bo); > + put_drmcgrp(bo->drmcgrp); > + > BUG_ON(kref_read(&bo->list_kref)); > BUG_ON(kref_read(&bo->kref)); > BUG_ON(atomic_read(&bo->cpu_writers)); > @@ -353,6 +359,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, > if (bo->mem.mem_type == TTM_PL_SYSTEM) { > if (bdev->driver->move_notify) > bdev->driver->move_notify(bo, evict, mem); > + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all > + drmcgrp_mem_track_move(bo, evict, mem); > bo->mem = *mem; > mem->mm_node = NULL; > goto moved; > @@ -361,6 +369,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, > > if (bdev->driver->move_notify) > bdev->driver->move_notify(bo, evict, mem); > + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all > + drmcgrp_mem_track_move(bo, evict, mem); > > if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) && > !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) > @@ -374,6 +384,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, > if (bdev->driver->move_notify) { > swap(*mem, bo->mem); > bdev->driver->move_notify(bo, false, mem); > + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all > + drmcgrp_mem_track_move(bo, evict, mem); > swap(*mem, bo->mem); > } > > @@ -1275,6 +1287,10 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev, > WARN_ON(!locked); > } > > + bo->drmcgrp = get_drmcgrp(current); > + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all > + drmcgrp_chg_mem(bo); > + > if (likely(!ret)) > ret = ttm_bo_validate(bo, placement, ctx); > > @@ -1666,6 +1682,20 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, > } > EXPORT_SYMBOL(ttm_bo_device_init); > > +/* TODO merge with official function when implementation finalized*/ > +int ttm_bo_device_init_tmp(struct ttm_bo_device *bdev, > + struct ttm_bo_driver *driver, > + struct drm_device *ddev, > + struct address_space *mapping, > + bool need_dma32) > +{ > + int ret = ttm_bo_device_init(bdev, driver, mapping, need_dma32); > + > + bdev->ddev = ddev; > + return ret; > +} > +EXPORT_SYMBOL(ttm_bo_device_init_tmp); > + > /* > * buffer object vm functions. > */ > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c > index 895d77d799e4..4ed7847c21f4 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > @@ -32,6 +32,7 @@ > #include <drm/ttm/ttm_bo_driver.h> > #include <drm/ttm/ttm_placement.h> > #include <drm/drm_vma_manager.h> > +#include <drm/drm_cgroup.h> > #include <linux/io.h> > #include <linux/highmem.h> > #include <linux/wait.h> > @@ -522,6 +523,9 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, > ret = reservation_object_trylock(fbo->base.resv); > WARN_ON(!ret); > > + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all > + drmcgrp_chg_mem(bo); > + > *new_obj = &fbo->base; > return 0; > } > diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h > index 8711b7c5f7bf..48ab5450cf17 100644 > --- a/include/drm/drm_cgroup.h > +++ b/include/drm/drm_cgroup.h > @@ -5,6 +5,7 @@ > #define __DRM_CGROUP_H__ > > #include <linux/cgroup_drm.h> > +#include <drm/ttm/ttm_bo_api.h> > > #ifdef CONFIG_CGROUP_DRM > > @@ -18,6 +19,11 @@ void drmcgrp_unchg_bo_alloc(struct drmcgrp *drmcgrp, struct drm_device *dev, > size_t size); > bool drmcgrp_bo_can_allocate(struct task_struct *task, struct drm_device *dev, > size_t size); > +void drmcgrp_chg_mem(struct ttm_buffer_object *tbo); > +void drmcgrp_unchg_mem(struct ttm_buffer_object *tbo); > +void drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, bool evict, > + struct ttm_mem_reg *new_mem); > + > #else > static inline int drmcgrp_register_device(struct drm_device *device) > { > @@ -50,5 +56,18 @@ static inline bool drmcgrp_bo_can_allocate(struct task_struct *task, > { > return true; > } > + > +static inline void drmcgrp_chg_mem(struct ttm_buffer_object *tbo) > +{ > +} > + > +static inline void drmcgrp_unchg_mem(struct ttm_buffer_object *tbo) > +{ > +} > + > +static inline void drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, > + bool evict, struct ttm_mem_reg *new_mem) > +{ > +} > #endif /* CONFIG_CGROUP_DRM */ > #endif /* __DRM_CGROUP_H__ */ > diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h > index 49d9cdfc58f2..ae1bb6daec81 100644 > --- a/include/drm/ttm/ttm_bo_api.h > +++ b/include/drm/ttm/ttm_bo_api.h > @@ -128,6 +128,7 @@ struct ttm_tt; > * struct ttm_buffer_object > * > * @bdev: Pointer to the buffer object device structure. > + * @drmcgrp: DRM cgroup this object belongs to. > * @type: The bo type. > * @destroy: Destruction function. If NULL, kfree is used. > * @num_pages: Actual number of pages. > @@ -174,6 +175,7 @@ struct ttm_buffer_object { > */ > > struct ttm_bo_device *bdev; > + struct drmcgrp *drmcgrp; > enum ttm_bo_type type; > void (*destroy) (struct ttm_buffer_object *); > unsigned long num_pages; > diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h > index c008346c2401..4cbcb41e5aa9 100644 > --- a/include/drm/ttm/ttm_bo_driver.h > +++ b/include/drm/ttm/ttm_bo_driver.h > @@ -30,6 +30,7 @@ > #ifndef _TTM_BO_DRIVER_H_ > #define _TTM_BO_DRIVER_H_ > > +#include <drm/drm_device.h> > #include <drm/drm_mm.h> > #include <drm/drm_vma_manager.h> > #include <linux/workqueue.h> > @@ -442,6 +443,7 @@ extern struct ttm_bo_global { > * @driver: Pointer to a struct ttm_bo_driver struct setup by the driver. > * @man: An array of mem_type_managers. > * @vma_manager: Address space manager > + * @ddev: Pointer to struct drm_device that this ttm_bo_device belongs to > * lru_lock: Spinlock that protects the buffer+device lru lists and > * ddestroy lists. > * @dev_mapping: A pointer to the struct address_space representing the > @@ -460,6 +462,7 @@ struct ttm_bo_device { > struct ttm_bo_global *glob; > struct ttm_bo_driver *driver; > struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES]; > + struct drm_device *ddev; > > /* > * Protected by internal locks. > @@ -598,6 +601,11 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, > struct address_space *mapping, > bool need_dma32); > > +int ttm_bo_device_init_tmp(struct ttm_bo_device *bdev, > + struct ttm_bo_driver *driver, > + struct drm_device *ddev, > + struct address_space *mapping, > + bool need_dma32); > /** > * ttm_bo_unmap_virtual > * > diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h > index e4400b21ab8e..141bea06f74c 100644 > --- a/include/linux/cgroup_drm.h > +++ b/include/linux/cgroup_drm.h > @@ -9,6 +9,7 @@ > #include <linux/mutex.h> > #include <linux/cgroup.h> > #include <drm/drm_file.h> > +#include <drm/ttm/ttm_placement.h> > > /* limit defined per the way drm_minor_alloc operates */ > #define MAX_DRM_DEV (64 * DRM_MINOR_RENDER) > @@ -22,6 +23,9 @@ struct drmcgrp_device_resource { > size_t bo_limits_peak_allocated; > > s64 bo_stats_count_allocated; > + > + s64 mem_stats[TTM_PL_PRIV+1]; > + s64 mem_stats_evict; > }; > > struct drmcgrp { > diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c > index 9144f93b851f..5aee42a628c1 100644 > --- a/kernel/cgroup/drm.c > +++ b/kernel/cgroup/drm.c > @@ -8,6 +8,8 @@ > #include <linux/mutex.h> > #include <linux/cgroup_drm.h> > #include <linux/kernel.h> > +#include <drm/ttm/ttm_bo_api.h> > +#include <drm/ttm/ttm_bo_driver.h> > #include <drm/drm_device.h> > #include <drm/drm_ioctl.h> > #include <drm/drm_cgroup.h> > @@ -34,6 +36,8 @@ enum drmcgrp_res_type { > DRMCGRP_TYPE_BO_TOTAL, > DRMCGRP_TYPE_BO_PEAK, > DRMCGRP_TYPE_BO_COUNT, > + DRMCGRP_TYPE_MEM, > + DRMCGRP_TYPE_MEM_EVICT, > }; > > enum drmcgrp_file_type { > @@ -42,6 +46,13 @@ enum drmcgrp_file_type { > DRMCGRP_FTYPE_DEFAULT, > }; > > +static char const *ttm_placement_names[] = { > + [TTM_PL_SYSTEM] = "system", > + [TTM_PL_TT] = "tt", > + [TTM_PL_VRAM] = "vram", > + [TTM_PL_PRIV] = "priv", > +}; > + > /* indexed by drm_minor for access speed */ > static struct drmcgrp_device *known_drmcgrp_devs[MAX_DRM_DEV]; > > @@ -134,6 +145,7 @@ drmcgrp_css_alloc(struct cgroup_subsys_state *parent_css) > static inline void drmcgrp_print_stats(struct drmcgrp_device_resource *ddr, > struct seq_file *sf, enum drmcgrp_res_type type) > { > + int i; > if (ddr == NULL) { > seq_puts(sf, "\n"); > return; > @@ -149,6 +161,16 @@ static inline void drmcgrp_print_stats(struct drmcgrp_device_resource *ddr, > case DRMCGRP_TYPE_BO_COUNT: > seq_printf(sf, "%lld\n", ddr->bo_stats_count_allocated); > break; > + case DRMCGRP_TYPE_MEM: > + for (i = 0; i <= TTM_PL_PRIV; i++) { > + seq_printf(sf, "%s=%lld ", ttm_placement_names[i], > + ddr->mem_stats[i]); > + } > + seq_puts(sf, "\n"); > + break; > + case DRMCGRP_TYPE_MEM_EVICT: > + seq_printf(sf, "%lld\n", ddr->mem_stats_evict); > + break; > default: > seq_puts(sf, "\n"); > break; > @@ -406,6 +428,18 @@ struct cftype files[] = { > .private = DRMCG_CTF_PRIV(DRMCGRP_TYPE_BO_COUNT, > DRMCGRP_FTYPE_STATS), > }, > + { > + .name = "memory.stats", > + .seq_show = drmcgrp_bo_show, > + .private = DRMCG_CTF_PRIV(DRMCGRP_TYPE_MEM, > + DRMCGRP_FTYPE_STATS), > + }, > + { > + .name = "memory.evict.stats", > + .seq_show = drmcgrp_bo_show, > + .private = DRMCG_CTF_PRIV(DRMCGRP_TYPE_MEM_EVICT, > + DRMCGRP_FTYPE_STATS), > + }, > { } /* terminate */ > }; > > @@ -555,3 +589,82 @@ void drmcgrp_unchg_bo_alloc(struct drmcgrp *drmcgrp, struct drm_device *dev, > mutex_unlock(&known_drmcgrp_devs[devIdx]->mutex); > } > EXPORT_SYMBOL(drmcgrp_unchg_bo_alloc); > + > +void drmcgrp_chg_mem(struct ttm_buffer_object *tbo) > +{ > + struct drm_device *dev = tbo->bdev->ddev; > + struct drmcgrp *drmcgrp = tbo->drmcgrp; > + int devIdx = dev->primary->index; > + s64 size = (s64)(tbo->mem.size); > + int mem_type = tbo->mem.mem_type; > + struct drmcgrp_device_resource *ddr; > + > + if (drmcgrp == NULL || known_drmcgrp_devs[devIdx] == NULL) > + return; > + > + mem_type = mem_type > TTM_PL_PRIV ? TTM_PL_PRIV : mem_type; > + > + mutex_lock(&known_drmcgrp_devs[devIdx]->mutex); > + for ( ; drmcgrp != NULL; drmcgrp = parent_drmcgrp(drmcgrp)) { > + ddr = drmcgrp->dev_resources[devIdx]; > + ddr->mem_stats[mem_type] += size; > + } > + mutex_unlock(&known_drmcgrp_devs[devIdx]->mutex); > +} > +EXPORT_SYMBOL(drmcgrp_chg_mem); > + > +void drmcgrp_unchg_mem(struct ttm_buffer_object *tbo) > +{ > + struct drm_device *dev = tbo->bdev->ddev; > + struct drmcgrp *drmcgrp = tbo->drmcgrp; > + int devIdx = dev->primary->index; > + s64 size = (s64)(tbo->mem.size); > + int mem_type = tbo->mem.mem_type; > + struct drmcgrp_device_resource *ddr; > + > + if (drmcgrp == NULL || known_drmcgrp_devs[devIdx] == NULL) > + return; > + > + mem_type = mem_type > TTM_PL_PRIV ? TTM_PL_PRIV : mem_type; > + > + mutex_lock(&known_drmcgrp_devs[devIdx]->mutex); > + for ( ; drmcgrp != NULL; drmcgrp = parent_drmcgrp(drmcgrp)) { > + ddr = drmcgrp->dev_resources[devIdx]; > + ddr->mem_stats[mem_type] -= size; > + } > + mutex_unlock(&known_drmcgrp_devs[devIdx]->mutex); > +} > +EXPORT_SYMBOL(drmcgrp_unchg_mem); > + > +void drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, bool evict, > + struct ttm_mem_reg *new_mem) > +{ > + struct drm_device *dev = old_bo->bdev->ddev; > + struct drmcgrp *drmcgrp = old_bo->drmcgrp; > + s64 move_in_bytes = (s64)(old_bo->mem.size); > + int devIdx = dev->primary->index; > + int old_mem_type = old_bo->mem.mem_type; > + int new_mem_type = new_mem->mem_type; > + struct drmcgrp_device_resource *ddr; > + struct drmcgrp_device *known_dev; > + > + known_dev = known_drmcgrp_devs[devIdx]; > + > + if (drmcgrp == NULL || known_dev == NULL) > + return; > + > + old_mem_type = old_mem_type > TTM_PL_PRIV ? TTM_PL_PRIV : old_mem_type; > + new_mem_type = new_mem_type > TTM_PL_PRIV ? TTM_PL_PRIV : new_mem_type; > + > + mutex_lock(&known_dev->mutex); > + for ( ; drmcgrp != NULL; drmcgrp = parent_drmcgrp(drmcgrp)) { > + ddr = drmcgrp->dev_resources[devIdx]; > + ddr->mem_stats[old_mem_type] -= move_in_bytes; > + ddr->mem_stats[new_mem_type] += move_in_bytes; > + > + if (evict) > + ddr->mem_stats_evict++; > + } > + mutex_unlock(&known_dev->mutex); > +} > +EXPORT_SYMBOL(drmcgrp_mem_track_move); > -- > 2.21.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Jun 26, 2019 at 12:12 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Wed, Jun 26, 2019 at 11:05:18AM -0400, Kenny Ho wrote: > > drm.memory.stats > > A read-only nested-keyed file which exists on all cgroups. > > Each entry is keyed by the drm device's major:minor. The > > following nested keys are defined. > > > > ====== ============================================= > > system Host/system memory > > Shouldn't that be covered by gem bo stats already? Also, system memory is > definitely something a lot of non-ttm drivers want to be able to track, so > that needs to be separate from ttm. The gem bo stats covers all of these type. I am treat the gem stats as more of the front end and a hard limit and this set of stats as the backing store which can be of various type. How does non-ttm drivers identify various memory types? > > tt Host memory used by the drm device (GTT/GART) > > vram Video RAM used by the drm device > > priv Other drm device, vendor specific memory > > So what's "priv". In general I think we need some way to register the > different kinds of memory, e.g. stuff not in your list: > > - multiple kinds of vram (like numa-style gpus) > - cma (for all those non-ttm drivers that's a big one, it's like system > memory but also totally different) > - any carveouts and stuff privs are vendor specific, which is why I have truncated it. For example, AMD has AMDGPU_PL_GDS, GWS, OA https://elixir.bootlin.com/linux/v5.2-rc6/source/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h#L30 Since we are using keyed file type, we should be able to support vendor specific memory type but I am not sure if this is acceptable to cgroup upstream. This is why I stick to the 3 memory type that is common across all ttm drivers. > I think with all the ttm refactoring going on I think we need to de-ttm > the interface functions here a bit. With Gerd Hoffmans series you can just > use a gem_bo pointer here, so what's left to do is have some extracted > structure for tracking memory types. I think Brian Welty has some ideas > for this, even in patch form. Would be good to keep him on cc at least for > the next version. We'd need to explicitly hand in the ttm_mem_reg (or > whatever the specific thing is going to be). I assume Gerd Hoffman's series you are referring to is this one? https://www.spinics.net/lists/dri-devel/msg215056.html I can certainly keep an eye out for Gerd's refactoring while refactoring other parts of this RFC. I have added Brian and Gerd to the thread for awareness. Regards, Kenny
On Thu, Jun 27, 2019 at 12:06:13AM -0400, Kenny Ho wrote: > On Wed, Jun 26, 2019 at 12:12 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Wed, Jun 26, 2019 at 11:05:18AM -0400, Kenny Ho wrote: > > > drm.memory.stats > > > A read-only nested-keyed file which exists on all cgroups. > > > Each entry is keyed by the drm device's major:minor. The > > > following nested keys are defined. > > > > > > ====== ============================================= > > > system Host/system memory > > > > Shouldn't that be covered by gem bo stats already? Also, system memory is > > definitely something a lot of non-ttm drivers want to be able to track, so > > that needs to be separate from ttm. > The gem bo stats covers all of these type. I am treat the gem stats > as more of the front end and a hard limit and this set of stats as the > backing store which can be of various type. How does non-ttm drivers > identify various memory types? Not explicitly, they generally just have one. I think i915 currently has two, system and carveout (with vram getting added). > > > tt Host memory used by the drm device (GTT/GART) > > > vram Video RAM used by the drm device > > > priv Other drm device, vendor specific memory > > > > So what's "priv". In general I think we need some way to register the > > different kinds of memory, e.g. stuff not in your list: > > > > - multiple kinds of vram (like numa-style gpus) > > - cma (for all those non-ttm drivers that's a big one, it's like system > > memory but also totally different) > > - any carveouts and stuff > privs are vendor specific, which is why I have truncated it. For > example, AMD has AMDGPU_PL_GDS, GWS, OA > https://elixir.bootlin.com/linux/v5.2-rc6/source/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h#L30 > > Since we are using keyed file type, we should be able to support > vendor specific memory type but I am not sure if this is acceptable to > cgroup upstream. This is why I stick to the 3 memory type that is > common across all ttm drivers. I think we'll need custom memory pools, not just priv, and I guess some naming scheme for them. I think just exposing them as amd-gws, amd-oa, amd-gds would make sense. Another thing I wonder about is multi-gpu cards, with multiple gpus and each their own vram and other device-specific resources. For those we'd have node0.vram and node1.vram too (on top of maybe an overall vram node, not sure). > > I think with all the ttm refactoring going on I think we need to de-ttm > > the interface functions here a bit. With Gerd Hoffmans series you can just > > use a gem_bo pointer here, so what's left to do is have some extracted > > structure for tracking memory types. I think Brian Welty has some ideas > > for this, even in patch form. Would be good to keep him on cc at least for > > the next version. We'd need to explicitly hand in the ttm_mem_reg (or > > whatever the specific thing is going to be). > > I assume Gerd Hoffman's series you are referring to is this one? > https://www.spinics.net/lists/dri-devel/msg215056.html There's a newer one, much more complete, but yes that's the work. > I can certainly keep an eye out for Gerd's refactoring while > refactoring other parts of this RFC. > > I have added Brian and Gerd to the thread for awareness. btw just realized that maybe building the interfaces on top of ttm_mem_reg is maybe not the best. That's what you're using right now, but in a way that's just the ttm internal detail of how the backing storage is allocated. I think the structure we need to abstract away is ttm_mem_type_manager, without any of the actual management details. btw reminds me: I guess it would be good to have a per-type .total read-only exposed, so that userspace has an idea of how much there is? ttm is trying to be agnostic to the allocator that's used to manage a memory type/resource, so doesn't even know that. But I think something we need to expose to admins, otherwise they can't meaningfully set limits. -Daniel
On Thu, Jun 27, 2019 at 2:01 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > btw reminds me: I guess it would be good to have a per-type .total > read-only exposed, so that userspace has an idea of how much there is? > ttm is trying to be agnostic to the allocator that's used to manage a > memory type/resource, so doesn't even know that. But I think something we > need to expose to admins, otherwise they can't meaningfully set limits. I don't think I understand this bit, do you mean total across multiple GPU of the same mem type? Or do you mean the total available per GPU (or something else?) Regards, Kenny
On Thu, Jun 27, 2019 at 04:17:09PM -0400, Kenny Ho wrote: > On Thu, Jun 27, 2019 at 2:01 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > btw reminds me: I guess it would be good to have a per-type .total > > read-only exposed, so that userspace has an idea of how much there is? > > ttm is trying to be agnostic to the allocator that's used to manage a > > memory type/resource, so doesn't even know that. But I think something we > > need to expose to admins, otherwise they can't meaningfully set limits. > > I don't think I understand this bit, do you mean total across multiple > GPU of the same mem type? Or do you mean the total available per GPU > (or something else?) Total for a given type on a given cpu. E.g. maybe you want to give 50% of your vram to one cgroup, and the other 50% to the other cgroup. For that you need to know how much vram you have. And expecting people to lspci and then look at wikipedia for how much vram that chip should have (or something like that) isn't great. Hence 0.vram.total, 0.tt.total, and so on (also for all the other gpu minors ofc). For system memory we probably don't want to provide a total, since that's already a value that's easy to obtain from various sources. -Daniel
On 6/26/2019 11:01 PM, Daniel Vetter wrote: > On Thu, Jun 27, 2019 at 12:06:13AM -0400, Kenny Ho wrote: >> On Wed, Jun 26, 2019 at 12:12 PM Daniel Vetter <daniel@ffwll.ch> wrote: >>> >>> I think with all the ttm refactoring going on I think we need to de-ttm >>> the interface functions here a bit. With Gerd Hoffmans series you can just >>> use a gem_bo pointer here, so what's left to do is have some extracted >>> structure for tracking memory types. I think Brian Welty has some ideas >>> for this, even in patch form. Would be good to keep him on cc at least for >>> the next version. We'd need to explicitly hand in the ttm_mem_reg (or >>> whatever the specific thing is going to be). >> >> I assume Gerd Hoffman's series you are referring to is this one? >> https://www.spinics.net/lists/dri-devel/msg215056.html > > There's a newer one, much more complete, but yes that's the work. > >> I can certainly keep an eye out for Gerd's refactoring while >> refactoring other parts of this RFC. >> >> I have added Brian and Gerd to the thread for awareness. > > btw just realized that maybe building the interfaces on top of ttm_mem_reg > is maybe not the best. That's what you're using right now, but in a way > that's just the ttm internal detail of how the backing storage is > allocated. I think the structure we need to abstract away is > ttm_mem_type_manager, without any of the actual management details. > Any de-ttm refactoring should probably not spam all the cgroups folks. So I removed cgroups list. As Daniel mentioned, some of us are looking at possible refactoring of TTM for reuse in i915 driver. Here is a brief summary of some ideas to be considered: 1) refactor part of ttm_mem_type_manager into a new drm_mem_type_region. Really, should then move the array from ttm_bo_device.man[] into drm_device. Relevant to drm_cgroup, you could then perhaps access these stats through drm_device and don't need the mem_stats array in drmcgrp_device_resource. 1a) doing this right means replacing TTM_PL_XXX memory types with new DRM defines. But could keep the TTM ones as redefinition of (new) DRM ones. Probably those private ones (TTM_PL_PRIV) make this difficult. All of the above could be eventually leveraged by the vram support being implemented now in i915 driver. 2) refactor ttm_mem_reg + ttm_bus_placement into something generic for any GEM object, maybe call it drm_gem_object_placement. ttm_mem_reg could remain as a wrapper for TTM drivers. This hasn't been broadly discussed with intel-gfx folks, so not sure this fits well into i915 or not. Relevant to drm_cgroup, maybe this function: drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, bool evict, struct ttm_mem_reg *new_mem) could potentially become: drmcgrp_mem_track_move(struct drm_gem_object *old_bo, bool evict, struct drm_gem_object_placement *new_place) Though from ttm_mem_reg, you look to only be using mem_type and size. I think Daniel is noting that ttm_mem_reg wasn't truly needed here, so you could just pass in the mem_type and size instead. Would appreciate any feedback (positive or negative) on above.... Perhaps this should move to a new thread? I could send out basic RFC patches for (1) if helpful but as it touches all the TTM drivers, nice to hear some feedback first. Anyway, this doesn't necessarily need to block forward progress on drm_cgroup, as refactoring into common base structures could happen incrementally. Thanks, -Brian
On Fri, Jun 28, 2019 at 3:16 AM Welty, Brian <brian.welty@intel.com> wrote: > On 6/26/2019 11:01 PM, Daniel Vetter wrote: > > On Thu, Jun 27, 2019 at 12:06:13AM -0400, Kenny Ho wrote: > >> On Wed, Jun 26, 2019 at 12:12 PM Daniel Vetter <daniel@ffwll.ch> wrote: > >>> > >>> I think with all the ttm refactoring going on I think we need to de-ttm > >>> the interface functions here a bit. With Gerd Hoffmans series you can just > >>> use a gem_bo pointer here, so what's left to do is have some extracted > >>> structure for tracking memory types. I think Brian Welty has some ideas > >>> for this, even in patch form. Would be good to keep him on cc at least for > >>> the next version. We'd need to explicitly hand in the ttm_mem_reg (or > >>> whatever the specific thing is going to be). > >> > >> I assume Gerd Hoffman's series you are referring to is this one? > >> https://www.spinics.net/lists/dri-devel/msg215056.html > > > > There's a newer one, much more complete, but yes that's the work. > > > >> I can certainly keep an eye out for Gerd's refactoring while > >> refactoring other parts of this RFC. > >> > >> I have added Brian and Gerd to the thread for awareness. > > > > btw just realized that maybe building the interfaces on top of ttm_mem_reg > > is maybe not the best. That's what you're using right now, but in a way > > that's just the ttm internal detail of how the backing storage is > > allocated. I think the structure we need to abstract away is > > ttm_mem_type_manager, without any of the actual management details. > > > > Any de-ttm refactoring should probably not spam all the cgroups folks. > So I removed cgroups list. > > As Daniel mentioned, some of us are looking at possible refactoring of TTM > for reuse in i915 driver. > Here is a brief summary of some ideas to be considered: > > 1) refactor part of ttm_mem_type_manager into a new drm_mem_type_region. > Really, should then move the array from ttm_bo_device.man[] into drm_device. > > Relevant to drm_cgroup, you could then perhaps access these stats through > drm_device and don't need the mem_stats array in drmcgrp_device_resource. > > 1a) doing this right means replacing TTM_PL_XXX memory types with new DRM > defines. But could keep the TTM ones as redefinition of (new) DRM ones. > Probably those private ones (TTM_PL_PRIV) make this difficult. > > All of the above could be eventually leveraged by the vram support being > implemented now in i915 driver. > > 2) refactor ttm_mem_reg + ttm_bus_placement into something generic for > any GEM object, maybe call it drm_gem_object_placement. > ttm_mem_reg could remain as a wrapper for TTM drivers. > This hasn't been broadly discussed with intel-gfx folks, so not sure > this fits well into i915 or not. > > Relevant to drm_cgroup, maybe this function: > drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, bool evict, > struct ttm_mem_reg *new_mem) > could potentially become: > drmcgrp_mem_track_move(struct drm_gem_object *old_bo, bool evict, > struct drm_gem_object_placement *new_place) > > Though from ttm_mem_reg, you look to only be using mem_type and size. > I think Daniel is noting that ttm_mem_reg wasn't truly needed here, so > you could just pass in the mem_type and size instead. Yeah I think the relevant part of your refactoring is creating a more abstract memory type/resource thing (not the individual allocations from it which ttm calls regions and I get confused about that every time). I think that abstraction should also have a field for the total (which I think cgroups also needs, both as read-only information and starting value). ttm would that put somewhere into the ttm_mem_type_manager, i915 would put it somewhere else, cma based drivers could perhaps expose the cma heap like that (if it's exclusive to the gpu at least). > Would appreciate any feedback (positive or negative) on above.... > Perhaps this should move to a new thread? I could send out basic RFC > patches for (1) if helpful but as it touches all the TTM drivers, nice to > hear some feedback first. > Anyway, this doesn't necessarily need to block forward progress on drm_cgroup, > as refactoring into common base structures could happen incrementally. Yeah I think new dri-devel thread with a totally unpolished rfc as draft plus this as the intro would be good. That way we can ground this a bit better in actual code. -Daniel
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index e9ecc3953673..a8dfc78ed45f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1678,8 +1678,9 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) mutex_init(&adev->mman.gtt_window_lock); /* No others user of address space so set it to 0 */ - r = ttm_bo_device_init(&adev->mman.bdev, + r = ttm_bo_device_init_tmp(&adev->mman.bdev, &amdgpu_bo_driver, + adev->ddev, adev->ddev->anon_inode->i_mapping, adev->need_dma32); if (r) { diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 2845fceb2fbd..e9f70547f0ad 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -34,6 +34,7 @@ #include <drm/ttm/ttm_module.h> #include <drm/ttm/ttm_bo_driver.h> #include <drm/ttm/ttm_placement.h> +#include <drm/drm_cgroup.h> #include <linux/jiffies.h> #include <linux/slab.h> #include <linux/sched.h> @@ -42,6 +43,7 @@ #include <linux/module.h> #include <linux/atomic.h> #include <linux/reservation.h> +#include <linux/cgroup_drm.h> static void ttm_bo_global_kobj_release(struct kobject *kobj); @@ -151,6 +153,10 @@ static void ttm_bo_release_list(struct kref *list_kref) struct ttm_bo_device *bdev = bo->bdev; size_t acc_size = bo->acc_size; + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all + drmcgrp_unchg_mem(bo); + put_drmcgrp(bo->drmcgrp); + BUG_ON(kref_read(&bo->list_kref)); BUG_ON(kref_read(&bo->kref)); BUG_ON(atomic_read(&bo->cpu_writers)); @@ -353,6 +359,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (bo->mem.mem_type == TTM_PL_SYSTEM) { if (bdev->driver->move_notify) bdev->driver->move_notify(bo, evict, mem); + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all + drmcgrp_mem_track_move(bo, evict, mem); bo->mem = *mem; mem->mm_node = NULL; goto moved; @@ -361,6 +369,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (bdev->driver->move_notify) bdev->driver->move_notify(bo, evict, mem); + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all + drmcgrp_mem_track_move(bo, evict, mem); if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) && !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) @@ -374,6 +384,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (bdev->driver->move_notify) { swap(*mem, bo->mem); bdev->driver->move_notify(bo, false, mem); + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all + drmcgrp_mem_track_move(bo, evict, mem); swap(*mem, bo->mem); } @@ -1275,6 +1287,10 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev, WARN_ON(!locked); } + bo->drmcgrp = get_drmcgrp(current); + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all + drmcgrp_chg_mem(bo); + if (likely(!ret)) ret = ttm_bo_validate(bo, placement, ctx); @@ -1666,6 +1682,20 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, } EXPORT_SYMBOL(ttm_bo_device_init); +/* TODO merge with official function when implementation finalized*/ +int ttm_bo_device_init_tmp(struct ttm_bo_device *bdev, + struct ttm_bo_driver *driver, + struct drm_device *ddev, + struct address_space *mapping, + bool need_dma32) +{ + int ret = ttm_bo_device_init(bdev, driver, mapping, need_dma32); + + bdev->ddev = ddev; + return ret; +} +EXPORT_SYMBOL(ttm_bo_device_init_tmp); + /* * buffer object vm functions. */ diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 895d77d799e4..4ed7847c21f4 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -32,6 +32,7 @@ #include <drm/ttm/ttm_bo_driver.h> #include <drm/ttm/ttm_placement.h> #include <drm/drm_vma_manager.h> +#include <drm/drm_cgroup.h> #include <linux/io.h> #include <linux/highmem.h> #include <linux/wait.h> @@ -522,6 +523,9 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo, ret = reservation_object_trylock(fbo->base.resv); WARN_ON(!ret); + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for all + drmcgrp_chg_mem(bo); + *new_obj = &fbo->base; return 0; } diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h index 8711b7c5f7bf..48ab5450cf17 100644 --- a/include/drm/drm_cgroup.h +++ b/include/drm/drm_cgroup.h @@ -5,6 +5,7 @@ #define __DRM_CGROUP_H__ #include <linux/cgroup_drm.h> +#include <drm/ttm/ttm_bo_api.h> #ifdef CONFIG_CGROUP_DRM @@ -18,6 +19,11 @@ void drmcgrp_unchg_bo_alloc(struct drmcgrp *drmcgrp, struct drm_device *dev, size_t size); bool drmcgrp_bo_can_allocate(struct task_struct *task, struct drm_device *dev, size_t size); +void drmcgrp_chg_mem(struct ttm_buffer_object *tbo); +void drmcgrp_unchg_mem(struct ttm_buffer_object *tbo); +void drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, bool evict, + struct ttm_mem_reg *new_mem); + #else static inline int drmcgrp_register_device(struct drm_device *device) { @@ -50,5 +56,18 @@ static inline bool drmcgrp_bo_can_allocate(struct task_struct *task, { return true; } + +static inline void drmcgrp_chg_mem(struct ttm_buffer_object *tbo) +{ +} + +static inline void drmcgrp_unchg_mem(struct ttm_buffer_object *tbo) +{ +} + +static inline void drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, + bool evict, struct ttm_mem_reg *new_mem) +{ +} #endif /* CONFIG_CGROUP_DRM */ #endif /* __DRM_CGROUP_H__ */ diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 49d9cdfc58f2..ae1bb6daec81 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -128,6 +128,7 @@ struct ttm_tt; * struct ttm_buffer_object * * @bdev: Pointer to the buffer object device structure. + * @drmcgrp: DRM cgroup this object belongs to. * @type: The bo type. * @destroy: Destruction function. If NULL, kfree is used. * @num_pages: Actual number of pages. @@ -174,6 +175,7 @@ struct ttm_buffer_object { */ struct ttm_bo_device *bdev; + struct drmcgrp *drmcgrp; enum ttm_bo_type type; void (*destroy) (struct ttm_buffer_object *); unsigned long num_pages; diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index c008346c2401..4cbcb41e5aa9 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -30,6 +30,7 @@ #ifndef _TTM_BO_DRIVER_H_ #define _TTM_BO_DRIVER_H_ +#include <drm/drm_device.h> #include <drm/drm_mm.h> #include <drm/drm_vma_manager.h> #include <linux/workqueue.h> @@ -442,6 +443,7 @@ extern struct ttm_bo_global { * @driver: Pointer to a struct ttm_bo_driver struct setup by the driver. * @man: An array of mem_type_managers. * @vma_manager: Address space manager + * @ddev: Pointer to struct drm_device that this ttm_bo_device belongs to * lru_lock: Spinlock that protects the buffer+device lru lists and * ddestroy lists. * @dev_mapping: A pointer to the struct address_space representing the @@ -460,6 +462,7 @@ struct ttm_bo_device { struct ttm_bo_global *glob; struct ttm_bo_driver *driver; struct ttm_mem_type_manager man[TTM_NUM_MEM_TYPES]; + struct drm_device *ddev; /* * Protected by internal locks. @@ -598,6 +601,11 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, struct address_space *mapping, bool need_dma32); +int ttm_bo_device_init_tmp(struct ttm_bo_device *bdev, + struct ttm_bo_driver *driver, + struct drm_device *ddev, + struct address_space *mapping, + bool need_dma32); /** * ttm_bo_unmap_virtual * diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h index e4400b21ab8e..141bea06f74c 100644 --- a/include/linux/cgroup_drm.h +++ b/include/linux/cgroup_drm.h @@ -9,6 +9,7 @@ #include <linux/mutex.h> #include <linux/cgroup.h> #include <drm/drm_file.h> +#include <drm/ttm/ttm_placement.h> /* limit defined per the way drm_minor_alloc operates */ #define MAX_DRM_DEV (64 * DRM_MINOR_RENDER) @@ -22,6 +23,9 @@ struct drmcgrp_device_resource { size_t bo_limits_peak_allocated; s64 bo_stats_count_allocated; + + s64 mem_stats[TTM_PL_PRIV+1]; + s64 mem_stats_evict; }; struct drmcgrp { diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c index 9144f93b851f..5aee42a628c1 100644 --- a/kernel/cgroup/drm.c +++ b/kernel/cgroup/drm.c @@ -8,6 +8,8 @@ #include <linux/mutex.h> #include <linux/cgroup_drm.h> #include <linux/kernel.h> +#include <drm/ttm/ttm_bo_api.h> +#include <drm/ttm/ttm_bo_driver.h> #include <drm/drm_device.h> #include <drm/drm_ioctl.h> #include <drm/drm_cgroup.h> @@ -34,6 +36,8 @@ enum drmcgrp_res_type { DRMCGRP_TYPE_BO_TOTAL, DRMCGRP_TYPE_BO_PEAK, DRMCGRP_TYPE_BO_COUNT, + DRMCGRP_TYPE_MEM, + DRMCGRP_TYPE_MEM_EVICT, }; enum drmcgrp_file_type { @@ -42,6 +46,13 @@ enum drmcgrp_file_type { DRMCGRP_FTYPE_DEFAULT, }; +static char const *ttm_placement_names[] = { + [TTM_PL_SYSTEM] = "system", + [TTM_PL_TT] = "tt", + [TTM_PL_VRAM] = "vram", + [TTM_PL_PRIV] = "priv", +}; + /* indexed by drm_minor for access speed */ static struct drmcgrp_device *known_drmcgrp_devs[MAX_DRM_DEV]; @@ -134,6 +145,7 @@ drmcgrp_css_alloc(struct cgroup_subsys_state *parent_css) static inline void drmcgrp_print_stats(struct drmcgrp_device_resource *ddr, struct seq_file *sf, enum drmcgrp_res_type type) { + int i; if (ddr == NULL) { seq_puts(sf, "\n"); return; @@ -149,6 +161,16 @@ static inline void drmcgrp_print_stats(struct drmcgrp_device_resource *ddr, case DRMCGRP_TYPE_BO_COUNT: seq_printf(sf, "%lld\n", ddr->bo_stats_count_allocated); break; + case DRMCGRP_TYPE_MEM: + for (i = 0; i <= TTM_PL_PRIV; i++) { + seq_printf(sf, "%s=%lld ", ttm_placement_names[i], + ddr->mem_stats[i]); + } + seq_puts(sf, "\n"); + break; + case DRMCGRP_TYPE_MEM_EVICT: + seq_printf(sf, "%lld\n", ddr->mem_stats_evict); + break; default: seq_puts(sf, "\n"); break; @@ -406,6 +428,18 @@ struct cftype files[] = { .private = DRMCG_CTF_PRIV(DRMCGRP_TYPE_BO_COUNT, DRMCGRP_FTYPE_STATS), }, + { + .name = "memory.stats", + .seq_show = drmcgrp_bo_show, + .private = DRMCG_CTF_PRIV(DRMCGRP_TYPE_MEM, + DRMCGRP_FTYPE_STATS), + }, + { + .name = "memory.evict.stats", + .seq_show = drmcgrp_bo_show, + .private = DRMCG_CTF_PRIV(DRMCGRP_TYPE_MEM_EVICT, + DRMCGRP_FTYPE_STATS), + }, { } /* terminate */ }; @@ -555,3 +589,82 @@ void drmcgrp_unchg_bo_alloc(struct drmcgrp *drmcgrp, struct drm_device *dev, mutex_unlock(&known_drmcgrp_devs[devIdx]->mutex); } EXPORT_SYMBOL(drmcgrp_unchg_bo_alloc); + +void drmcgrp_chg_mem(struct ttm_buffer_object *tbo) +{ + struct drm_device *dev = tbo->bdev->ddev; + struct drmcgrp *drmcgrp = tbo->drmcgrp; + int devIdx = dev->primary->index; + s64 size = (s64)(tbo->mem.size); + int mem_type = tbo->mem.mem_type; + struct drmcgrp_device_resource *ddr; + + if (drmcgrp == NULL || known_drmcgrp_devs[devIdx] == NULL) + return; + + mem_type = mem_type > TTM_PL_PRIV ? TTM_PL_PRIV : mem_type; + + mutex_lock(&known_drmcgrp_devs[devIdx]->mutex); + for ( ; drmcgrp != NULL; drmcgrp = parent_drmcgrp(drmcgrp)) { + ddr = drmcgrp->dev_resources[devIdx]; + ddr->mem_stats[mem_type] += size; + } + mutex_unlock(&known_drmcgrp_devs[devIdx]->mutex); +} +EXPORT_SYMBOL(drmcgrp_chg_mem); + +void drmcgrp_unchg_mem(struct ttm_buffer_object *tbo) +{ + struct drm_device *dev = tbo->bdev->ddev; + struct drmcgrp *drmcgrp = tbo->drmcgrp; + int devIdx = dev->primary->index; + s64 size = (s64)(tbo->mem.size); + int mem_type = tbo->mem.mem_type; + struct drmcgrp_device_resource *ddr; + + if (drmcgrp == NULL || known_drmcgrp_devs[devIdx] == NULL) + return; + + mem_type = mem_type > TTM_PL_PRIV ? TTM_PL_PRIV : mem_type; + + mutex_lock(&known_drmcgrp_devs[devIdx]->mutex); + for ( ; drmcgrp != NULL; drmcgrp = parent_drmcgrp(drmcgrp)) { + ddr = drmcgrp->dev_resources[devIdx]; + ddr->mem_stats[mem_type] -= size; + } + mutex_unlock(&known_drmcgrp_devs[devIdx]->mutex); +} +EXPORT_SYMBOL(drmcgrp_unchg_mem); + +void drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, bool evict, + struct ttm_mem_reg *new_mem) +{ + struct drm_device *dev = old_bo->bdev->ddev; + struct drmcgrp *drmcgrp = old_bo->drmcgrp; + s64 move_in_bytes = (s64)(old_bo->mem.size); + int devIdx = dev->primary->index; + int old_mem_type = old_bo->mem.mem_type; + int new_mem_type = new_mem->mem_type; + struct drmcgrp_device_resource *ddr; + struct drmcgrp_device *known_dev; + + known_dev = known_drmcgrp_devs[devIdx]; + + if (drmcgrp == NULL || known_dev == NULL) + return; + + old_mem_type = old_mem_type > TTM_PL_PRIV ? TTM_PL_PRIV : old_mem_type; + new_mem_type = new_mem_type > TTM_PL_PRIV ? TTM_PL_PRIV : new_mem_type; + + mutex_lock(&known_dev->mutex); + for ( ; drmcgrp != NULL; drmcgrp = parent_drmcgrp(drmcgrp)) { + ddr = drmcgrp->dev_resources[devIdx]; + ddr->mem_stats[old_mem_type] -= move_in_bytes; + ddr->mem_stats[new_mem_type] += move_in_bytes; + + if (evict) + ddr->mem_stats_evict++; + } + mutex_unlock(&known_dev->mutex); +} +EXPORT_SYMBOL(drmcgrp_mem_track_move);
The drm resource being measured is the TTM (Translation Table Manager) buffers. TTM manages different types of memory that a GPU might access. These memory types include dedicated Video RAM (VRAM) and host/system memory accessible through IOMMU (GART/GTT). TTM is currently used by multiple drm drivers (amd, ast, bochs, cirrus, hisilicon, maga200, nouveau, qxl, virtio, vmwgfx.) drm.memory.stats A read-only nested-keyed file which exists on all cgroups. Each entry is keyed by the drm device's major:minor. The following nested keys are defined. ====== ============================================= system Host/system memory tt Host memory used by the drm device (GTT/GART) vram Video RAM used by the drm device priv Other drm device, vendor specific memory ====== ============================================= Reading returns the following:: 226:0 system=0 tt=0 vram=0 priv=0 226:1 system=0 tt=9035776 vram=17768448 priv=16809984 226:2 system=0 tt=9035776 vram=17768448 priv=16809984 drm.memory.evict.stats A read-only flat-keyed file which exists on all cgroups. Each entry is keyed by the drm device's major:minor. Total number of evictions. Change-Id: Ice2c4cc845051229549bebeb6aa2d7d6153bdf6a Signed-off-by: Kenny Ho <Kenny.Ho@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 +- drivers/gpu/drm/ttm/ttm_bo.c | 30 +++++++ drivers/gpu/drm/ttm/ttm_bo_util.c | 4 + include/drm/drm_cgroup.h | 19 ++++ include/drm/ttm/ttm_bo_api.h | 2 + include/drm/ttm/ttm_bo_driver.h | 8 ++ include/linux/cgroup_drm.h | 4 + kernel/cgroup/drm.c | 113 ++++++++++++++++++++++++ 8 files changed, 182 insertions(+), 1 deletion(-)