[Intel-gfx] [PATCH v2 2/2] drm: i915: Switch to bitmap_zalloc()

[Chris Wilson]

Quoting Andy Shevchenko (2019-03-04 09:29:08)
> Switch to bitmap_zalloc() to show clearly what we are allocating.
> Besides that it returns pointer of bitmap type instead of opaque void *.

Which is confusing; since we explicitly want unsigned longs, not some
amorphous bitmap type.

> Signed-off-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c               | 2 +-
>  drivers/gpu/drm/i915/i915_gem_fence_reg.c     | 3 +--
>  drivers/gpu/drm/i915/i915_gem_tiling.c        | 6 +++---
>  drivers/gpu/drm/i915/selftests/intel_uncore.c | 5 ++---
>  4 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6728ea5c71d4..0d96520cfdb3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4410,7 +4410,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
>                 drm_gem_object_release(&obj->base);
>                 i915_gem_info_remove_obj(i915, obj->base.size);
>  
> -               kfree(obj->bit_17);
> +               bitmap_free(obj->bit_17);
>                 i915_gem_object_free(obj);
>  
>                 GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
> diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.c b/drivers/gpu/drm/i915/i915_gem_fence_reg.c
> index e037e94792f3..1f880e5b79b0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_fence_reg.c
> +++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.c
> @@ -790,8 +790,7 @@ i915_gem_object_save_bit_17_swizzle(struct drm_i915_gem_object *obj,
>         int i;
>  
>         if (obj->bit_17 == NULL) {
> -               obj->bit_17 = kcalloc(BITS_TO_LONGS(page_count),
> -                                     sizeof(long), GFP_KERNEL);
> +               obj->bit_17 = bitmap_zalloc(page_count, GFP_KERNEL);

That feels a bit more of an overreach, as we just use bitops and never
actually use the bitmap iface.

Simply because it kills BITS_TO_LONGS(), even though I do not see why
the bitmap_[z]alloc and bitmap_free are not inlines...

And for this is not the overflow protection of kcalloc silly? We start
with a large value, factorise it, then check that the two factors do not
overflow? If it were to overflow, it would overflow in the
BITS_TO_LONGS() itself.

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris