| Message ID | 20220621104434.190962-4-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | small BAR uapi bits | expand |
Hi Matthew,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on linus/master v5.19-rc3 next-20220621]
[cannot apply to drm-intel/for-linux-next drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Matthew-Auld/small-BAR-uapi-bits/20220621-184858
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: i386-randconfig-a004 (https://download.01.org/0day-ci/archive/20220622/202206220325.JAXbxJ16-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project af6d2a0b6825e71965f3e2701a63c239fa0ad70f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c421b7689a4552aa8e3acc2fe558d097c464870e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Matthew-Auld/small-BAR-uapi-bits/20220621-184858
git checkout c421b7689a4552aa8e3acc2fe558d097c464870e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/i915_ttm_buddy_manager.c:374: warning: expecting prototype for i915_ttm_buddy_man_visible_size(). Prototype was for i915_ttm_buddy_man_avail() instead
vim +374 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
362
363 /**
364 * i915_ttm_buddy_man_visible_size - Query the avail tracking for the manager.
365 *
366 * @man: The buddy allocator ttm manager
367 * @avail: The total available memory in pages for the entire manager.
368 * @visible_avail: The total available memory in pages for the CPU visible
369 * portion. Note that this will always give the same value as @avail on
370 * configurations that don't have a small BAR.
371 */
372 void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man,
373 u64 *avail, u64 *visible_avail)
> 374 {
375 struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
376
377 mutex_lock(&bman->lock);
378 *avail = bman->mm.avail >> PAGE_SHIFT;
379 *visible_avail = bman->visible_avail;
380 mutex_unlock(&bman->lock);
381 }
382
On Tue, 2022-06-21 at 11:44 +0100, Matthew Auld wrote: > Vulkan would like to have a rough measure of how much device memory > can > in theory be allocated. Also add unallocated_cpu_visible_size to > track > the visible portion, in case the device is using small BAR. Also > tweak > the locking so we nice consistent values for both the mm->avail and > the > visible tracking. > > v2: tweak the locking slightly so we update the mm->avail and visible > tracking as one atomic operation, such that userspace doesn't get > strange values when sampling the values. > > Testcase: igt@i915_query@query-regions-unallocated > Testcase: igt@i915_query@query-regions-sanity-check > Signed-off-by: Matthew Auld <matthew.auld@intel.com> Note the kernel test robot warning for inconsistent kerneldoc and function name. With that fixed, Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Cc: Jon Bloomfield <jon.bloomfield@intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Kenneth Graunke <kenneth@whitecape.org> > Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> > --- > drivers/gpu/drm/i915/i915_query.c | 10 +++++- > drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 31 ++++++++++++++--- > -- > drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 3 ++ > drivers/gpu/drm/i915/intel_memory_region.c | 14 +++++++++ > drivers/gpu/drm/i915/intel_memory_region.h | 3 ++ > include/uapi/drm/i915_drm.h | 31 > ++++++++++++++++++- > 6 files changed, 82 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_query.c > b/drivers/gpu/drm/i915/i915_query.c > index 9894add651dd..6ec9c9fb7b0d 100644 > --- a/drivers/gpu/drm/i915/i915_query.c > +++ b/drivers/gpu/drm/i915/i915_query.c > @@ -504,7 +504,15 @@ static int query_memregion_info(struct > drm_i915_private *i915, > else > info.probed_cpu_visible_size = mr->total; > > - info.unallocated_size = mr->avail; > + if (perfmon_capable()) { > + intel_memory_region_avail(mr, > + > &info.unallocated_size, > + > &info.unallocated_cpu_visible_size); > + } else { > + info.unallocated_size = info.probed_size; > + info.unallocated_cpu_visible_size = > + info.probed_cpu_visible_size; > + } > > if (__copy_to_user(info_ptr, &info, sizeof(info))) > return -EFAULT; > diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > index a5109548abc0..864c8f55cacb 100644 > --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > @@ -104,18 +104,15 @@ static int i915_ttm_buddy_man_alloc(struct > ttm_resource_manager *man, > min_page_size, > &bman_res->blocks, > bman_res->flags); > - mutex_unlock(&bman->lock); > if (unlikely(err)) > goto err_free_blocks; > > if (place->flags & TTM_PL_FLAG_CONTIGUOUS) { > u64 original_size = (u64)bman_res->base.num_pages << > PAGE_SHIFT; > > - mutex_lock(&bman->lock); > drm_buddy_block_trim(mm, > original_size, > &bman_res->blocks); > - mutex_unlock(&bman->lock); > } > > if (lpfn <= bman->visible_size) { > @@ -137,11 +134,10 @@ static int i915_ttm_buddy_man_alloc(struct > ttm_resource_manager *man, > } > } > > - if (bman_res->used_visible_size) { > - mutex_lock(&bman->lock); > + if (bman_res->used_visible_size) > bman->visible_avail -= bman_res->used_visible_size; > - mutex_unlock(&bman->lock); > - } > + > + mutex_unlock(&bman->lock); > > if (place->lpfn - place->fpfn == n_pages) > bman_res->base.start = place->fpfn; > @@ -154,7 +150,6 @@ static int i915_ttm_buddy_man_alloc(struct > ttm_resource_manager *man, > return 0; > > err_free_blocks: > - mutex_lock(&bman->lock); > drm_buddy_free_list(mm, &bman_res->blocks); > mutex_unlock(&bman->lock); > err_free_res: > @@ -365,6 +360,26 @@ u64 i915_ttm_buddy_man_visible_size(struct > ttm_resource_manager *man) > return bman->visible_size; > } > > +/** > + * i915_ttm_buddy_man_visible_size - Query the avail tracking for > the manager. > + * > + * @man: The buddy allocator ttm manager > + * @avail: The total available memory in pages for the entire > manager. > + * @visible_avail: The total available memory in pages for the CPU > visible > + * portion. Note that this will always give the same value as @avail > on > + * configurations that don't have a small BAR. > + */ > +void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man, > + u64 *avail, u64 *visible_avail) > +{ > + struct i915_ttm_buddy_manager *bman = to_buddy_manager(man); > + > + mutex_lock(&bman->lock); > + *avail = bman->mm.avail >> PAGE_SHIFT; > + *visible_avail = bman->visible_avail; > + mutex_unlock(&bman->lock); > +} > + > #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) > void i915_ttm_buddy_man_force_visible_size(struct > ttm_resource_manager *man, > u64 size) > diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h > b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h > index 52d9586d242c..d64620712830 100644 > --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h > +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h > @@ -61,6 +61,9 @@ int i915_ttm_buddy_man_reserve(struct > ttm_resource_manager *man, > > u64 i915_ttm_buddy_man_visible_size(struct ttm_resource_manager > *man); > > +void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man, > + u64 *avail, u64 *avail_visible); > + > #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) > void i915_ttm_buddy_man_force_visible_size(struct > ttm_resource_manager *man, > u64 size); > diff --git a/drivers/gpu/drm/i915/intel_memory_region.c > b/drivers/gpu/drm/i915/intel_memory_region.c > index e38d2db1c3e3..94ee26e99549 100644 > --- a/drivers/gpu/drm/i915/intel_memory_region.c > +++ b/drivers/gpu/drm/i915/intel_memory_region.c > @@ -279,6 +279,20 @@ void intel_memory_region_set_name(struct > intel_memory_region *mem, > va_end(ap); > } > > +void intel_memory_region_avail(struct intel_memory_region *mr, > + u64 *avail, u64 *visible_avail) > +{ > + if (mr->type == INTEL_MEMORY_LOCAL) { > + i915_ttm_buddy_man_avail(mr->region_private, > + avail, visible_avail); > + *avail <<= PAGE_SHIFT; > + *visible_avail <<= PAGE_SHIFT; > + } else { > + *avail = mr->total; > + *visible_avail = mr->total; > + } > +} > + > void intel_memory_region_destroy(struct intel_memory_region *mem) > { > int ret = 0; > diff --git a/drivers/gpu/drm/i915/intel_memory_region.h > b/drivers/gpu/drm/i915/intel_memory_region.h > index 3d8378c1b447..2214f251bec3 100644 > --- a/drivers/gpu/drm/i915/intel_memory_region.h > +++ b/drivers/gpu/drm/i915/intel_memory_region.h > @@ -127,6 +127,9 @@ int intel_memory_region_reserve(struct > intel_memory_region *mem, > void intel_memory_region_debug(struct intel_memory_region *mr, > struct drm_printer *printer); > > +void intel_memory_region_avail(struct intel_memory_region *mr, > + u64 *avail, u64 *visible_avail); > + > struct intel_memory_region * > i915_gem_ttm_system_setup(struct drm_i915_private *i915, > u16 type, u16 instance); > diff --git a/include/uapi/drm/i915_drm.h > b/include/uapi/drm/i915_drm.h > index 7eacacb00373..e4847436bab8 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -3228,7 +3228,15 @@ struct drm_i915_memory_region_info { > */ > __u64 probed_size; > > - /** @unallocated_size: Estimate of memory remaining */ > + /** > + * @unallocated_size: Estimate of memory remaining > + * > + * Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable > accounting. > + * Without this (or if this is an older kernel) the value > here will > + * always equal the @probed_size. Note this is only currently > tracked > + * for I915_MEMORY_CLASS_DEVICE regions (for other types the > value here > + * will always equal the @probed_size). > + */ > __u64 unallocated_size; > > union { > @@ -3262,6 +3270,27 @@ struct drm_i915_memory_region_info { > * @probed_size. > */ > __u64 probed_cpu_visible_size; > + > + /** > + * @unallocated_cpu_visible_size: Estimate of > CPU > + * visible memory remaining. > + * > + * Note this is only tracked for > + * I915_MEMORY_CLASS_DEVICE regions (for > other types the > + * value here will always equal the > + * @probed_cpu_visible_size). > + * > + * Requires CAP_PERFMON or CAP_SYS_ADMIN to > get reliable > + * accounting. Without this the value here > will always > + * equal the @probed_cpu_visible_size. Note > this is only > + * currently tracked for > I915_MEMORY_CLASS_DEVICE > + * regions (for other types the value here > will also > + * always equal the > @probed_cpu_visible_size). > + * > + * If this is an older kernel the value here > will be > + * zero, see also @probed_cpu_visible_size. > + */ > + __u64 unallocated_cpu_visible_size; > }; > }; > };
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c index 9894add651dd..6ec9c9fb7b0d 100644 --- a/drivers/gpu/drm/i915/i915_query.c +++ b/drivers/gpu/drm/i915/i915_query.c @@ -504,7 +504,15 @@ static int query_memregion_info(struct drm_i915_private *i915, else info.probed_cpu_visible_size = mr->total; - info.unallocated_size = mr->avail; + if (perfmon_capable()) { + intel_memory_region_avail(mr, + &info.unallocated_size, + &info.unallocated_cpu_visible_size); + } else { + info.unallocated_size = info.probed_size; + info.unallocated_cpu_visible_size = + info.probed_cpu_visible_size; + } if (__copy_to_user(info_ptr, &info, sizeof(info))) return -EFAULT; diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index a5109548abc0..864c8f55cacb 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -104,18 +104,15 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, min_page_size, &bman_res->blocks, bman_res->flags); - mutex_unlock(&bman->lock); if (unlikely(err)) goto err_free_blocks; if (place->flags & TTM_PL_FLAG_CONTIGUOUS) { u64 original_size = (u64)bman_res->base.num_pages << PAGE_SHIFT; - mutex_lock(&bman->lock); drm_buddy_block_trim(mm, original_size, &bman_res->blocks); - mutex_unlock(&bman->lock); } if (lpfn <= bman->visible_size) { @@ -137,11 +134,10 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, } } - if (bman_res->used_visible_size) { - mutex_lock(&bman->lock); + if (bman_res->used_visible_size) bman->visible_avail -= bman_res->used_visible_size; - mutex_unlock(&bman->lock); - } + + mutex_unlock(&bman->lock); if (place->lpfn - place->fpfn == n_pages) bman_res->base.start = place->fpfn; @@ -154,7 +150,6 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man, return 0; err_free_blocks: - mutex_lock(&bman->lock); drm_buddy_free_list(mm, &bman_res->blocks); mutex_unlock(&bman->lock); err_free_res: @@ -365,6 +360,26 @@ u64 i915_ttm_buddy_man_visible_size(struct ttm_resource_manager *man) return bman->visible_size; } +/** + * i915_ttm_buddy_man_visible_size - Query the avail tracking for the manager. + * + * @man: The buddy allocator ttm manager + * @avail: The total available memory in pages for the entire manager. + * @visible_avail: The total available memory in pages for the CPU visible + * portion. Note that this will always give the same value as @avail on + * configurations that don't have a small BAR. + */ +void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man, + u64 *avail, u64 *visible_avail) +{ + struct i915_ttm_buddy_manager *bman = to_buddy_manager(man); + + mutex_lock(&bman->lock); + *avail = bman->mm.avail >> PAGE_SHIFT; + *visible_avail = bman->visible_avail; + mutex_unlock(&bman->lock); +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) void i915_ttm_buddy_man_force_visible_size(struct ttm_resource_manager *man, u64 size) diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h index 52d9586d242c..d64620712830 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h @@ -61,6 +61,9 @@ int i915_ttm_buddy_man_reserve(struct ttm_resource_manager *man, u64 i915_ttm_buddy_man_visible_size(struct ttm_resource_manager *man); +void i915_ttm_buddy_man_avail(struct ttm_resource_manager *man, + u64 *avail, u64 *avail_visible); + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) void i915_ttm_buddy_man_force_visible_size(struct ttm_resource_manager *man, u64 size); diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c index e38d2db1c3e3..94ee26e99549 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.c +++ b/drivers/gpu/drm/i915/intel_memory_region.c @@ -279,6 +279,20 @@ void intel_memory_region_set_name(struct intel_memory_region *mem, va_end(ap); } +void intel_memory_region_avail(struct intel_memory_region *mr, + u64 *avail, u64 *visible_avail) +{ + if (mr->type == INTEL_MEMORY_LOCAL) { + i915_ttm_buddy_man_avail(mr->region_private, + avail, visible_avail); + *avail <<= PAGE_SHIFT; + *visible_avail <<= PAGE_SHIFT; + } else { + *avail = mr->total; + *visible_avail = mr->total; + } +} + void intel_memory_region_destroy(struct intel_memory_region *mem) { int ret = 0; diff --git a/drivers/gpu/drm/i915/intel_memory_region.h b/drivers/gpu/drm/i915/intel_memory_region.h index 3d8378c1b447..2214f251bec3 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.h +++ b/drivers/gpu/drm/i915/intel_memory_region.h @@ -127,6 +127,9 @@ int intel_memory_region_reserve(struct intel_memory_region *mem, void intel_memory_region_debug(struct intel_memory_region *mr, struct drm_printer *printer); +void intel_memory_region_avail(struct intel_memory_region *mr, + u64 *avail, u64 *visible_avail); + struct intel_memory_region * i915_gem_ttm_system_setup(struct drm_i915_private *i915, u16 type, u16 instance); diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 7eacacb00373..e4847436bab8 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -3228,7 +3228,15 @@ struct drm_i915_memory_region_info { */ __u64 probed_size; - /** @unallocated_size: Estimate of memory remaining */ + /** + * @unallocated_size: Estimate of memory remaining + * + * Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable accounting. + * Without this (or if this is an older kernel) the value here will + * always equal the @probed_size. Note this is only currently tracked + * for I915_MEMORY_CLASS_DEVICE regions (for other types the value here + * will always equal the @probed_size). + */ __u64 unallocated_size; union { @@ -3262,6 +3270,27 @@ struct drm_i915_memory_region_info { * @probed_size. */ __u64 probed_cpu_visible_size; + + /** + * @unallocated_cpu_visible_size: Estimate of CPU + * visible memory remaining. + * + * Note this is only tracked for + * I915_MEMORY_CLASS_DEVICE regions (for other types the + * value here will always equal the + * @probed_cpu_visible_size). + * + * Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable + * accounting. Without this the value here will always + * equal the @probed_cpu_visible_size. Note this is only + * currently tracked for I915_MEMORY_CLASS_DEVICE + * regions (for other types the value here will also + * always equal the @probed_cpu_visible_size). + * + * If this is an older kernel the value here will be + * zero, see also @probed_cpu_visible_size. + */ + __u64 unallocated_cpu_visible_size; }; }; };
Vulkan would like to have a rough measure of how much device memory can in theory be allocated. Also add unallocated_cpu_visible_size to track the visible portion, in case the device is using small BAR. Also tweak the locking so we nice consistent values for both the mm->avail and the visible tracking. v2: tweak the locking slightly so we update the mm->avail and visible tracking as one atomic operation, such that userspace doesn't get strange values when sampling the values. Testcase: igt@i915_query@query-regions-unallocated Testcase: igt@i915_query@query-regions-sanity-check Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Jon Bloomfield <jon.bloomfield@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Kenneth Graunke <kenneth@whitecape.org> Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> --- drivers/gpu/drm/i915/i915_query.c | 10 +++++- drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 31 ++++++++++++++----- drivers/gpu/drm/i915/i915_ttm_buddy_manager.h | 3 ++ drivers/gpu/drm/i915/intel_memory_region.c | 14 +++++++++ drivers/gpu/drm/i915/intel_memory_region.h | 3 ++ include/uapi/drm/i915_drm.h | 31 ++++++++++++++++++- 6 files changed, 82 insertions(+), 10 deletions(-)