| Message ID | 20190304092908.57382-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] drm/selftests/mm: Switch to bitmap_zalloc() | expand |
On Mon, Mar 04, 2019 at 09:41:34AM +0000, Chris Wilson wrote: > 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. Why? You use it as a bitmap anyway since you are telling below you are using bit ops like set/clear_bit. > > 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. bitops are _luckily_ part of bitmap iface. bitmap iface has been evolved specifically the way the existing ops will work on it w/o any change. > Simply because it kills BITS_TO_LONGS(), even though I do not see why > the bitmap_[z]alloc and bitmap_free are not inlines... Because of circular dependencies (hell) in the headers. > 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. This just a simple API change w/o functional changes. > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Thank you.
Quoting Chris Wilson (2019-03-04 09:43:43) > Quoting Andy Shevchenko (2019-03-04 09:29:07) > > Switch to bitmap_zalloc() to show clearly what we are allocating. > > Besides that it returns pointer of bitmap type instead of opaque void *. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Applied to drm-misc-next. -Chris
diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c index fbed2c90fd51..286a0eeefcb6 100644 --- a/drivers/gpu/drm/selftests/test-drm_mm.c +++ b/drivers/gpu/drm/selftests/test-drm_mm.c @@ -1615,7 +1615,7 @@ static int igt_topdown(void *ignored) DRM_RND_STATE(prng, random_seed); const unsigned int count = 8192; unsigned int size; - unsigned long *bitmap = NULL; + unsigned long *bitmap; struct drm_mm mm; struct drm_mm_node *nodes, *node, *next; unsigned int *order, n, m, o = 0; @@ -1631,8 +1631,7 @@ static int igt_topdown(void *ignored) if (!nodes) goto err; - bitmap = kcalloc(count / BITS_PER_LONG, sizeof(unsigned long), - GFP_KERNEL); + bitmap = bitmap_zalloc(count, GFP_KERNEL); if (!bitmap) goto err_nodes; @@ -1717,7 +1716,7 @@ static int igt_topdown(void *ignored) drm_mm_takedown(&mm); kfree(order); err_bitmap: - kfree(bitmap); + bitmap_free(bitmap); err_nodes: vfree(nodes); err: @@ -1745,8 +1744,7 @@ static int igt_bottomup(void *ignored) if (!nodes) goto err; - bitmap = kcalloc(count / BITS_PER_LONG, sizeof(unsigned long), - GFP_KERNEL); + bitmap = bitmap_zalloc(count, GFP_KERNEL); if (!bitmap) goto err_nodes; @@ -1818,7 +1816,7 @@ static int igt_bottomup(void *ignored) drm_mm_takedown(&mm); kfree(order); err_bitmap: - kfree(bitmap); + bitmap_free(bitmap); err_nodes: vfree(nodes); err:
Switch to bitmap_zalloc() to show clearly what we are allocating. Besides that it returns pointer of bitmap type instead of opaque void *. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpu/drm/selftests/test-drm_mm.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)