| Message ID | wv4aw6syqjox52lpgkddxykr5namvan4eb7b4obj3rligwyp7m@33c3ko2mj7sp (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/selftest: allow larger memory allocation | expand |
Hi Mikolaj, On 2025-03-14 at 15:42:23 GMT, Mikolaj Wasiak wrote: > Due to changes in allocator, the size of the allocation for > contiguous region is not rounded up to a power-of-two and > instead allocated as is. Thus, change the part of test that > expected the allocation to fail. > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9311 > > Signed-off-by: Mikolaj Wasiak <mikolaj.wasiak@intel.com> > --- > .../gpu/drm/i915/selftests/intel_memory_region.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c > index f08f6674911e..ce848ae878c8 100644 > --- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c > +++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c > @@ -413,22 +413,17 @@ static int igt_mock_splintered_region(void *arg) > > close_objects(mem, &objects); > > - /* > - * While we should be able allocate everything without any flag > - * restrictions, if we consider I915_BO_ALLOC_CONTIGUOUS then we are > - * actually limited to the largest power-of-two for the region size i.e > - * max_order, due to the inner workings of the buddy allocator. So make > - * sure that does indeed hold true. > - */ > - > obj = igt_object_create(mem, &objects, size, I915_BO_ALLOC_CONTIGUOUS); > - if (!IS_ERR(obj)) { > - pr_err("%s too large contiguous allocation was not rejected\n", > + if (!IS_ERR(obj) && !is_contiguous(obj)) { > + pr_err("%s large allocation was not contiguous\n", > __func__); > err = -EINVAL; > goto out_close; > } > If I understand the test correctly, the goal of the part that you're changing is to see if an attempt at allocating more memory than max_order is properly rejected. Since the allocations are more granular now (not only limited to powers of two), and size doesn't get rounded up to the higher power of two, we should be able to allocate 'size' exactly. Meaning we lose the intended functionality of the test (check if we can't allocate too big of an object), because we're not allocating too big of an object anymore. I guess a check for contiguousness does not hurt, but the test behavior is fundamentally different here. Maybe manually rounding size up to the nearest larger power of two would be a better idea here? > + if (!IS_ERR(obj)) > + close_objects(mem, &objects); > + > obj = igt_object_create(mem, &objects, rounddown_pow_of_two(size), > I915_BO_ALLOC_CONTIGUOUS); > if (IS_ERR(obj)) { I'll paste some more lines from that test here: obj = igt_object_create(mem, &objects, rounddown_pow_of_two(size), I915_BO_ALLOC_CONTIGUOUS); if (IS_ERR(obj)) { pr_err("%s largest possible contiguous allocation failed\n", __func__); err = PTR_ERR(obj); goto out_close; } This is the next check - we see if the largest possible allocation (according to the old logic, it would be 'size' rounded down to the largest smaller or equal power of two) _does_ go through. The success of this particular check isn't affected by the allocator changes, but since rounddown_pow_of_two(size) is not the largest possible allocation anymore, maybe it's better to change this too (e.g. drop the rounddown function). This way we keep the intended test behavior here as well. I suppose this is still in scope of the patch. Thanks Krzysztof > -- > 2.43.0 >
diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c index f08f6674911e..ce848ae878c8 100644 --- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c +++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c @@ -413,22 +413,17 @@ static int igt_mock_splintered_region(void *arg) close_objects(mem, &objects); - /* - * While we should be able allocate everything without any flag - * restrictions, if we consider I915_BO_ALLOC_CONTIGUOUS then we are - * actually limited to the largest power-of-two for the region size i.e - * max_order, due to the inner workings of the buddy allocator. So make - * sure that does indeed hold true. - */ - obj = igt_object_create(mem, &objects, size, I915_BO_ALLOC_CONTIGUOUS); - if (!IS_ERR(obj)) { - pr_err("%s too large contiguous allocation was not rejected\n", + if (!IS_ERR(obj) && !is_contiguous(obj)) { + pr_err("%s large allocation was not contiguous\n", __func__); err = -EINVAL; goto out_close; } + if (!IS_ERR(obj)) + close_objects(mem, &objects); + obj = igt_object_create(mem, &objects, rounddown_pow_of_two(size), I915_BO_ALLOC_CONTIGUOUS); if (IS_ERR(obj)) {
Due to changes in allocator, the size of the allocation for contiguous region is not rounded up to a power-of-two and instead allocated as is. Thus, change the part of test that expected the allocation to fail. Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9311 Signed-off-by: Mikolaj Wasiak <mikolaj.wasiak@intel.com> --- .../gpu/drm/i915/selftests/intel_memory_region.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)