Re: [Intel-gfx] [PATCH 2/9] drm: kselftest for drm_mm_reserve_node()

[Chris Wilson]

On Fri, Dec 09, 2016 at 04:31:50PM +0200, Joonas Lahtinen wrote:
> On pe, 2016-12-09 at 13:08 +, Chris Wilson wrote:
> > +static int igt_reserve(void *ignored)
> > +{
> > +   int n, ret;
> > +
> > +   for (n = 1; n < 50; n++) {
> 
> The right amount of loops might be something to discuss.

The looping here is for all power-of-two (+-1 to catch the usual
interesting boundary conditions) upto total size == 1<<63 (a little room
to spare). So these I regard as exercising the likely errors, with the
looping inside being subject to discussion. Certainly also probably nice
to run through with primes as well (though we have a few mersenne primes
in this set, so that's probably overkill).

On top of these we also want to think how much error handling we want
inside drm_mm_node_reserve() and add the broken usage.

> > +   ret = __igt_reserve(8192, (1ull << n) - 1);
> 
> BIT_ULL(n) - 1?

Considered it, but felt calling it BIT was misleading / distraction.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/9] drm: kselftest for drm_mm_reserve_node()

[Joonas Lahtinen]

On pe, 2016-12-09 at 13:08 +, Chris Wilson wrote:
> Exercise drm_mm_reserve_node(), check that we can't reserve an already
> occupied range and that the lists are correct after reserving/removing.
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/drm_mm_selftests.h |   1 +
>  drivers/gpu/drm/test-drm_mm.c  | 143 
> +

Once there is more thorough coverage, I think we should have
drm/selftest/drm_mm.{h,c} 

> +static int __igt_reserve(int count, u64 size)
> +{
> + struct drm_mm mm;
> + struct drm_mm_node *node, *next;
> + int *order, n;
> + int ret;
> +
> + /* Fill a range with lots of nodes, check it doesn't fail too early */
> +
> + ret = -ENOMEM;
> + order = random_order(count);
> + if (!order)
> + goto err;
> +
> + ret = -EINVAL;
> + drm_mm_init(, 0, count * size);
> + if (!drm_mm_clean()) {
> + pr_err("mm not empty on creation\n");
> + goto out;
> + }

This code gets covered already done in more basic test, I think we
should avoid duplication from the start as we now have trouble with
runtime of tests.

> +static int igt_reserve(void *ignored)
> +{
> + int n, ret;
> +
> + for (n = 1; n < 50; n++) {

The right amount of loops might be something to discuss.

> + ret = __igt_reserve(8192, (1ull << n) - 1);

BIT_ULL(n) - 1?

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx