All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915/region: fix max size calculation
@ 2020-10-21  9:49 Matthew Auld
  2020-10-21  9:55 ` Chris Wilson
  2020-10-21 10:54 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
  0 siblings, 2 replies; 3+ messages in thread
From: Matthew Auld @ 2020-10-21  9:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson

We are incorrectly limiting the max allocation size as per the mm
max_order, which is effectively the largest power-of-two that we can fit
in the region size. However, it's normal to setup the region or
allocator with a non-power-of-two size(for example 3G), which we should
already handle correctly, except it seems for the early too-big-check.

Fixes: b908be543e44 ("drm/i915: support creating LMEM objects")
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: CQ Tang <cq.tang@intel.com>
---
 drivers/gpu/drm/i915/intel_memory_region.c    |  2 +-
 .../drm/i915/selftests/intel_memory_region.c  | 50 +++++++++++++++++++
 drivers/gpu/drm/i915/selftests/mock_region.c  |  2 +-
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
index 6b5e9d88646d..180e1078ef7c 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -87,7 +87,7 @@ __intel_memory_region_get_pages_buddy(struct intel_memory_region *mem,
 		min_order = ilog2(size) - ilog2(mem->mm.chunk_size);
 	}
 
-	if (size > BIT(mem->mm.max_order) * mem->mm.chunk_size)
+	if (size > mem->mm.size)
 		return -E2BIG;
 
 	n_pages = size >> ilog2(mem->mm.chunk_size);
diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
index 334b0648e253..34d6de916b3e 100644
--- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
@@ -261,6 +261,55 @@ static int igt_mock_contiguous(void *arg)
 	return err;
 }
 
+static int igt_mock_splintered_region(void *arg)
+{
+	struct intel_memory_region *mem = arg;
+	struct drm_i915_private *i915 = mem->i915;
+	struct drm_i915_gem_object *obj;
+	unsigned int expected_order;
+	LIST_HEAD(objects);
+	u64 size;
+	int err = 0;
+
+	/*
+	 * Sanity check we can still allocate everything even if the
+	 * mm.max_order != mm.size. i.e our starting address space size is not a
+	 * power-of-two.
+	 */
+
+	size = (SZ_4G - 1) & PAGE_MASK;
+	mem = mock_region_create(i915, 0, size, PAGE_SIZE, 0);
+	if (IS_ERR(mem))
+		return PTR_ERR(mem);
+
+	if (mem->mm.size != size) {
+		pr_err("%s size mismatch(%llu != %llu)\n",
+		       __func__, mem->mm.size, size);
+		err = -EINVAL;
+		goto out_put;
+	}
+
+	expected_order = get_order(rounddown_pow_of_two(size));
+	if (mem->mm.max_order != expected_order) {
+		pr_err("%s order mismatch(%u != %u)\n",
+		       __func__, mem->mm.max_order, expected_order);
+		err = -EINVAL;
+		goto out_put;
+	}
+
+	obj = igt_object_create(mem, &objects, size, 0);
+	if (IS_ERR(obj)) {
+		err = PTR_ERR(obj);
+		goto out_close;
+	}
+
+out_close:
+	close_objects(mem, &objects);
+out_put:
+	intel_memory_region_put(mem);
+	return err;
+}
+
 static int igt_gpu_write_dw(struct intel_context *ce,
 			    struct i915_vma *vma,
 			    u32 dword,
@@ -771,6 +820,7 @@ int intel_memory_region_mock_selftests(void)
 	static const struct i915_subtest tests[] = {
 		SUBTEST(igt_mock_fill),
 		SUBTEST(igt_mock_contiguous),
+		SUBTEST(igt_mock_splintered_region),
 	};
 	struct intel_memory_region *mem;
 	struct drm_i915_private *i915;
diff --git a/drivers/gpu/drm/i915/selftests/mock_region.c b/drivers/gpu/drm/i915/selftests/mock_region.c
index 09660f5a0a4c..979d96f27c43 100644
--- a/drivers/gpu/drm/i915/selftests/mock_region.c
+++ b/drivers/gpu/drm/i915/selftests/mock_region.c
@@ -24,7 +24,7 @@ mock_object_create(struct intel_memory_region *mem,
 	struct drm_i915_private *i915 = mem->i915;
 	struct drm_i915_gem_object *obj;
 
-	if (size > BIT(mem->mm.max_order) * mem->mm.chunk_size)
+	if (size > mem->mm.size)
 		return ERR_PTR(-E2BIG);
 
 	obj = i915_gem_object_alloc();
-- 
2.26.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/region: fix max size calculation
  2020-10-21  9:49 [Intel-gfx] [PATCH] drm/i915/region: fix max size calculation Matthew Auld
@ 2020-10-21  9:55 ` Chris Wilson
  2020-10-21 10:54 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
  1 sibling, 0 replies; 3+ messages in thread
From: Chris Wilson @ 2020-10-21  9:55 UTC (permalink / raw)
  To: Matthew Auld, intel-gfx

Quoting Matthew Auld (2020-10-21 10:49:57)
> We are incorrectly limiting the max allocation size as per the mm
> max_order, which is effectively the largest power-of-two that we can fit
> in the region size. However, it's normal to setup the region or
> allocator with a non-power-of-two size(for example 3G), which we should
> already handle correctly, except it seems for the early too-big-check.
> 
> Fixes: b908be543e44 ("drm/i915: support creating LMEM objects")
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: CQ Tang <cq.tang@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_memory_region.c    |  2 +-
>  .../drm/i915/selftests/intel_memory_region.c  | 50 +++++++++++++++++++
>  drivers/gpu/drm/i915/selftests/mock_region.c  |  2 +-
>  3 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c
> index 6b5e9d88646d..180e1078ef7c 100644
> --- a/drivers/gpu/drm/i915/intel_memory_region.c
> +++ b/drivers/gpu/drm/i915/intel_memory_region.c
> @@ -87,7 +87,7 @@ __intel_memory_region_get_pages_buddy(struct intel_memory_region *mem,
>                 min_order = ilog2(size) - ilog2(mem->mm.chunk_size);
>         }
>  
> -       if (size > BIT(mem->mm.max_order) * mem->mm.chunk_size)
> +       if (size > mem->mm.size)
>                 return -E2BIG;
>  
>         n_pages = size >> ilog2(mem->mm.chunk_size);
> diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> index 334b0648e253..34d6de916b3e 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> @@ -261,6 +261,55 @@ static int igt_mock_contiguous(void *arg)
>         return err;
>  }
>  
> +static int igt_mock_splintered_region(void *arg)
> +{
> +       struct intel_memory_region *mem = arg;
> +       struct drm_i915_private *i915 = mem->i915;
> +       struct drm_i915_gem_object *obj;
> +       unsigned int expected_order;
> +       LIST_HEAD(objects);
> +       u64 size;
> +       int err = 0;
> +
> +       /*
> +        * Sanity check we can still allocate everything even if the
> +        * mm.max_order != mm.size. i.e our starting address space size is not a
> +        * power-of-two.
> +        */
> +
> +       size = (SZ_4G - 1) & PAGE_MASK;
> +       mem = mock_region_create(i915, 0, size, PAGE_SIZE, 0);
> +       if (IS_ERR(mem))
> +               return PTR_ERR(mem);
> +
> +       if (mem->mm.size != size) {
> +               pr_err("%s size mismatch(%llu != %llu)\n",
> +                      __func__, mem->mm.size, size);
> +               err = -EINVAL;
> +               goto out_put;
> +       }
> +
> +       expected_order = get_order(rounddown_pow_of_two(size));
> +       if (mem->mm.max_order != expected_order) {
> +               pr_err("%s order mismatch(%u != %u)\n",
> +                      __func__, mem->mm.max_order, expected_order);
> +               err = -EINVAL;
> +               goto out_put;
> +       }
> +
> +       obj = igt_object_create(mem, &objects, size, 0);
> +       if (IS_ERR(obj)) {
> +               err = PTR_ERR(obj);
> +               goto out_close;
> +       }

A second pass to see if we correctly reject an attempt to allocate a
contiguous object larger than max chunk size (but still within
mem_size)?

+1 for splintered!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/region: fix max size calculation
  2020-10-21  9:49 [Intel-gfx] [PATCH] drm/i915/region: fix max size calculation Matthew Auld
  2020-10-21  9:55 ` Chris Wilson
@ 2020-10-21 10:54 ` Patchwork
  1 sibling, 0 replies; 3+ messages in thread
From: Patchwork @ 2020-10-21 10:54 UTC (permalink / raw)
  To: Matthew Auld; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 4200 bytes --]

== Series Details ==

Series: drm/i915/region: fix max size calculation
URL   : https://patchwork.freedesktop.org/series/82909/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9175 -> Patchwork_18749
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18749/index.html

Known issues
------------

  Here are the changes found in Patchwork_18749 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-blb-e6850:       [PASS][1] -> [INCOMPLETE][2] ([i915#2540])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9175/fi-blb-e6850/igt@core_hotunplug@unbind-rebind.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18749/fi-blb-e6850/igt@core_hotunplug@unbind-rebind.html
    - fi-icl-y:           [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9175/fi-icl-y/igt@core_hotunplug@unbind-rebind.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18749/fi-icl-y/igt@core_hotunplug@unbind-rebind.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-icl-u2:          [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9175/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18749/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@vgem_basic@unload:
    - fi-skl-guc:         [PASS][7] -> [DMESG-WARN][8] ([i915#2203])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9175/fi-skl-guc/igt@vgem_basic@unload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18749/fi-skl-guc/igt@vgem_basic@unload.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-tgl-u2:          [INCOMPLETE][9] ([i915#2557]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9175/fi-tgl-u2/igt@i915_selftest@live@gt_heartbeat.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18749/fi-tgl-u2/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-bsw-kefka:       [DMESG-WARN][11] ([i915#1982]) -> [PASS][12] +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9175/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18749/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  
#### Warnings ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-icl-u2:          [DMESG-WARN][13] ([i915#1982] / [i915#289]) -> [DMESG-WARN][14] ([i915#289])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9175/fi-icl-u2/igt@core_hotunplug@unbind-rebind.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18749/fi-icl-u2/igt@core_hotunplug@unbind-rebind.html

  
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#2540]: https://gitlab.freedesktop.org/drm/intel/issues/2540
  [i915#2557]: https://gitlab.freedesktop.org/drm/intel/issues/2557
  [i915#289]: https://gitlab.freedesktop.org/drm/intel/issues/289


Participating hosts (46 -> 39)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_9175 -> Patchwork_18749

  CI-20190529: 20190529
  CI_DRM_9175: 39db87924cbebbf2be2ef849d5be1c761d7865a2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5822: b4bcf05cb9839037128905deda7146434155cc41 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18749: 9787b185affa7ef9ad06885f25df647f560626dc @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9787b185affa drm/i915/region: fix max size calculation

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18749/index.html

[-- Attachment #1.2: Type: text/html, Size: 5228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-10-21 10:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21  9:49 [Intel-gfx] [PATCH] drm/i915/region: fix max size calculation Matthew Auld
2020-10-21  9:55 ` Chris Wilson
2020-10-21 10:54 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.