On ke, 2016-04-20 at 12:04 +0100, Chris Wilson wrote:
> Inside the shrinker we call can_release_pages() to indicate whether or
> not we can make forward progress in freeing up memory by unbinding that
> object. When adding our report to oom, we should be using the same
> logic.
>
> Whilst here, change the reporting from bytes to pages so that it looks
> smaller to the user!, is consistent with the neighbouring oom report
> itself which displays counts in pages, and makes the unsigned long
> overflow less likely.
Makes sense as that is the smallest pinning unit, too.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem_shrinker.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index 4e4fcfa76b4c..cb225e039d48 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -336,7 +336,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
> container_of(nb, struct drm_i915_private, mm.oom_notifier);
> struct shrinker_lock_uninterruptible slu;
> struct drm_i915_gem_object *obj;
> - unsigned long pinned, bound, unbound, freed_pages;
> + unsigned long unevictable, bound, unbound, freed_pages;
>
> if (!i915_gem_shrinker_lock_uninterruptible(dev_priv, &slu, 5000))
> return NOTIFY_DONE;
> @@ -347,33 +347,33 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
> * assert that there are no objects with pinned pages that are not
> * being pointed to by hardware.
> */
> - unbound = bound = pinned = 0;
> + unbound = bound = unevictable = 0;
> list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list) {
> if (!obj->base.filp) /* not backed by a freeable object */
> continue;
>
> - if (obj->pages_pin_count)
> - pinned += obj->base.size;
> + if (!can_release_pages(obj))
> + unevictable += obj->base.size >> PAGE_SHIFT;
> else
> - unbound += obj->base.size;
> + unbound += obj->base.size >> PAGE_SHIFT;
> }
> list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> if (!obj->base.filp)
> continue;
>
> - if (obj->pages_pin_count > num_vma_bound(obj))
> - pinned += obj->base.size;
> + if (!can_release_pages(obj))
> + unevictable += obj->base.size >> PAGE_SHIFT;
> else
> - bound += obj->base.size;
> + bound += obj->base.size >> PAGE_SHIFT;
> }
>
> i915_gem_shrinker_unlock_uninterruptible(dev_priv, &slu);
>
> if (freed_pages || unbound || bound)
> - pr_info("Purging GPU memory, %lu bytes freed, %lu bytes still pinned.\n",
> - freed_pages << PAGE_SHIFT, pinned);
> + pr_info("Purging GPU memory, %lu pages freed, %lu pages still pinned.\n",
> + freed_pages, unevictable);
My code checker senses are tingling, this looks very much like
oversized line, mind cutting it down?
With that addressed,
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> if (unbound || bound)
> - pr_err("%lu and %lu bytes still available in the "
> + pr_err("%lu and %lu pages still available in the "
> "bound and unbound GPU page lists.\n",
> bound, unbound);
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
