Re: [PATCH 8/9] drm: kselftest for drm_mm and top-down alignment

On pe, 2016-12-09 at 13:08 +0000, Chris Wilson wrote:

Check that if we request top-down allocation with a particular alignment from drm_mm_insert_node() that the start of the node matches our request.

Signed-off-by: Chris Wilson chris@chris-wilson.co.uk

<SNIP>

@@ -1038,6 +1038,98 @@ static int igt_topdown(void *ignored)   return ret;  }   +static int igt_topdown_align(void *ignored) +{

• struct drm_mm mm;
• struct drm_mm_node tmp, resv;
• int ret = -EINVAL;
• int n, m, err;
• drm_mm_init(&mm, 0, ~0ull);

U64_MAX

• memset(&tmp, 0, sizeof(tmp));
• memset(&resv, 0, sizeof(resv));
• for (m = 0; m < 32; m++) {

• u64 end = ~0ull;
  

• if (m) {
  

• 	resv.size = BIT_ULL(m);
  

• 	end -= resv.size;
  

• 	resv.start = end;
  

• 	err = drm_mm_reserve_node(&mm, &resv);
  

• 	if (err) {
  

• 		pr_err("reservation of sentinel node failed\n");
  

• 		ret = err;
  

• 		goto out;
  

• 	}
  

• }
  

• for (n = 0; n < 63 - m; n++) {
  

• 	u64 align = BIT_ULL(n);
  

• 	err = drm_mm_insert_node_generic(&mm, &tmp, 1, align, 0,
  

• 					 DRM_MM_SEARCH_BELOW,
  

• 					 DRM_MM_CREATE_TOP);
  

• 	drm_mm_remove_node(&tmp);
  

• 	if (err) {
  

• 		pr_err("insert failed, ret=%d\n", err);
  

• 		ret = err;
  

• 		goto out;
  

• 	}
  

Just drm_mm_remove_node(&tmp) here to avoid an unnecessary extra splat. I think the tests should be valid code.

• 	if (tmp.start & (align - 1)) {
  

• 		pr_err("insert alignment failed, aligment=%llx, start=%llx\n",
  

• 		       align, tmp.start);
  

• 		goto out;
  

• 	}
  

• 	if (tmp.start < end - align) {
  

• 		pr_err("topdown insert failed, start=%llx, align=%llx, end=%llx\n",
  

• 		       tmp.start, align, end);
  

• 		goto out;
  

• 	}
  

• }
  

• for_each_prime(n, min(8192ull, end - 1)) {
  

• 	u64 rem;
  

• 	err = drm_mm_insert_node_generic(&mm, &tmp, n, 0, 0,
  

• 					 DRM_MM_SEARCH_BELOW,
  

• 					 DRM_MM_CREATE_TOP);
  

• 	drm_mm_remove_node(&tmp);
  

• 	if (err) {
  

• 		pr_err("insert failed, ret=%d\n", err);
  

• 		ret = err;
  

• 		goto out;
  

• 	}
  

Ditto.

In addition to those, I would use more meaningful variable names than m & n.

Regards, Joonas