* [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX
@ 2022-11-02 5:14 Niranjana Vishwanathapura
2022-11-02 6:21 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Niranjana Vishwanathapura @ 2022-11-02 5:14 UTC (permalink / raw)
To: intel-gfx; +Cc: matthew.auld
Currently on DG1, which do not have LLC, we hit the below
warning while rebinding an userptr invalidated object.
WARNING: CPU: 4 PID: 13008 at drivers/gpu/drm/i915/gem/i915_gem_pages.c:34 __i915_gem_object_set_pages+0x296/0x2d0 [i915]
...
RIP: 0010:__i915_gem_object_set_pages+0x296/0x2d0 [i915]
...
Call Trace:
<TASK>
i915_gem_userptr_get_pages+0x175/0x1a0 [i915]
____i915_gem_object_get_pages+0x32/0xb0 [i915]
i915_gem_object_userptr_submit_init+0x286/0x470 [i915]
eb_lookup_vmas+0x2ff/0xcf0 [i915]
? __intel_wakeref_get_first+0x55/0xb0 [i915]
i915_gem_do_execbuffer+0x785/0x21d0 [i915]
i915_gem_execbuffer2_ioctl+0xe7/0x3d0 [i915]
We shouldn't be setting the obj->cache_dirty for DGFX,
fix it.
Suggested-by: Matthew Auld <matthew.auld@intel.com>
Reported-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 11125c32dd35..2f7804492cd5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -369,14 +369,14 @@ __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj,
__start_cpu_write(obj);
/*
- * On non-LLC platforms, force the flush-on-acquire if this is ever
+ * On non-LLC igfx platforms, force the flush-on-acquire if this is ever
* swapped-in. Our async flush path is not trust worthy enough yet(and
* happens in the wrong order), and with some tricks it's conceivable
* for userspace to change the cache-level to I915_CACHE_NONE after the
* pages are swapped-in, and since execbuf binds the object before doing
* the async flush, we have a race window.
*/
- if (!HAS_LLC(i915))
+ if (!HAS_LLC(i915) && !IS_DGFX(i915))
obj->cache_dirty = true;
}
--
2.21.0.rc0.32.g243a4c7e27
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Do not set cache_dirty for DGFX
2022-11-02 5:14 [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX Niranjana Vishwanathapura
@ 2022-11-02 6:21 ` Patchwork
2022-11-02 6:45 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2022-11-02 6:21 UTC (permalink / raw)
To: Niranjana Vishwanathapura; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Do not set cache_dirty for DGFX
URL : https://patchwork.freedesktop.org/series/110399/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Do not set cache_dirty for DGFX
2022-11-02 5:14 [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX Niranjana Vishwanathapura
2022-11-02 6:21 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
@ 2022-11-02 6:45 ` Patchwork
2022-11-02 7:39 ` [Intel-gfx] [PATCH] " Das, Nirmoy
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2022-11-02 6:45 UTC (permalink / raw)
To: Niranjana Vishwanathapura; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 7690 bytes --]
== Series Details ==
Series: drm/i915: Do not set cache_dirty for DGFX
URL : https://patchwork.freedesktop.org/series/110399/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_12330 -> Patchwork_110399v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_110399v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_110399v1, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/index.html
Participating hosts (38 -> 29)
------------------------------
Additional (3): fi-icl-u2 fi-tgl-dsi fi-pnv-d510
Missing (12): fi-adl-ddr5 bat-dg2-8 bat-adlm-1 bat-dg2-9 bat-adlp-6 bat-adlp-4 bat-adln-1 bat-rplp-1 bat-rpls-1 bat-rpls-2 bat-dg2-11 bat-jsl-1
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_110399v1:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live@dmabuf:
- fi-bsw-kefka: [PASS][1] -> [DMESG-FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12330/fi-bsw-kefka/igt@i915_selftest@live@dmabuf.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-bsw-kefka/igt@i915_selftest@live@dmabuf.html
Known issues
------------
Here are the changes found in Patchwork_110399v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_huc_copy@huc-copy:
- fi-icl-u2: NOTRUN -> [SKIP][3] ([i915#2190])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-icl-u2/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@random-engines:
- fi-icl-u2: NOTRUN -> [SKIP][4] ([i915#4613]) +3 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-icl-u2/igt@gem_lmem_swapping@random-engines.html
* igt@i915_pm_rpm@module-reload:
- fi-icl-u2: NOTRUN -> [INCOMPLETE][5] ([i915#4890])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-icl-u2/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-cfl-guc: [PASS][6] -> [DMESG-FAIL][7] ([i915#5334])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12330/fi-cfl-guc/igt@i915_selftest@live@gt_heartbeat.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-cfl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_suspend@basic-s3-without-i915:
- fi-bdw-5557u: [PASS][8] -> [INCOMPLETE][9] ([i915#146])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12330/fi-bdw-5557u/igt@i915_suspend@basic-s3-without-i915.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-bdw-5557u/igt@i915_suspend@basic-s3-without-i915.html
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-hsw-4770: NOTRUN -> [SKIP][10] ([fdo#109271] / [fdo#111827])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-icl-u2: NOTRUN -> [SKIP][11] ([fdo#111827]) +7 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
- fi-icl-u2: NOTRUN -> [SKIP][12] ([i915#4103])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html
* igt@kms_force_connector_basic@force-load-detect:
- fi-icl-u2: NOTRUN -> [SKIP][13] ([fdo#109285])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-icl-u2/igt@kms_force_connector_basic@force-load-detect.html
* igt@kms_psr@primary_page_flip:
- fi-pnv-d510: NOTRUN -> [SKIP][14] ([fdo#109271]) +43 similar issues
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-pnv-d510/igt@kms_psr@primary_page_flip.html
* igt@kms_setmode@basic-clone-single-crtc:
- fi-icl-u2: NOTRUN -> [SKIP][15] ([i915#3555])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-icl-u2/igt@kms_setmode@basic-clone-single-crtc.html
* igt@prime_vgem@basic-userptr:
- fi-icl-u2: NOTRUN -> [SKIP][16] ([fdo#109295] / [i915#3301])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-icl-u2/igt@prime_vgem@basic-userptr.html
* igt@runner@aborted:
- fi-icl-u2: NOTRUN -> [FAIL][17] ([i915#4312])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-icl-u2/igt@runner@aborted.html
#### Possible fixes ####
* igt@gem_ctx_create@basic-files:
- {fi-tgl-mst}: [DMESG-WARN][18] ([i915#6434]) -> [PASS][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12330/fi-tgl-mst/igt@gem_ctx_create@basic-files.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-tgl-mst/igt@gem_ctx_create@basic-files.html
* igt@i915_selftest@live@hangcheck:
- fi-hsw-4770: [INCOMPLETE][20] ([i915#4785]) -> [PASS][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12330/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
[i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
[i915#4890]: https://gitlab.freedesktop.org/drm/intel/issues/4890
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#6434]: https://gitlab.freedesktop.org/drm/intel/issues/6434
[i915#6856]: https://gitlab.freedesktop.org/drm/intel/issues/6856
[i915#7125]: https://gitlab.freedesktop.org/drm/intel/issues/7125
Build changes
-------------
* Linux: CI_DRM_12330 -> Patchwork_110399v1
CI-20190529: 20190529
CI_DRM_12330: b4169c84bf30172f83c6d365a927dcea0cd7a2ab @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7037: 6a25c53624502fc85cec3cf0a0bf244a2346e30f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_110399v1: b4169c84bf30172f83c6d365a927dcea0cd7a2ab @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
6be90ff6eb2b drm/i915: Do not set cache_dirty for DGFX
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110399v1/index.html
[-- Attachment #2: Type: text/html, Size: 8513 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX
2022-11-02 5:14 [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX Niranjana Vishwanathapura
2022-11-02 6:21 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
2022-11-02 6:45 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2022-11-02 7:39 ` Das, Nirmoy
2022-11-02 10:36 ` Matthew Auld
2022-11-02 21:59 ` Andi Shyti
2022-11-02 22:31 ` Andi Shyti
4 siblings, 1 reply; 9+ messages in thread
From: Das, Nirmoy @ 2022-11-02 7:39 UTC (permalink / raw)
To: Niranjana Vishwanathapura, intel-gfx; +Cc: matthew.auld
[-- Attachment #1: Type: text/plain, Size: 2258 bytes --]
On 11/2/2022 6:14 AM, Niranjana Vishwanathapura wrote:
> Currently on DG1, which do not have LLC, we hit the below
> warning while rebinding an userptr invalidated object.
>
> WARNING: CPU: 4 PID: 13008 at drivers/gpu/drm/i915/gem/i915_gem_pages.c:34 __i915_gem_object_set_pages+0x296/0x2d0 [i915]
> ...
> RIP: 0010:__i915_gem_object_set_pages+0x296/0x2d0 [i915]
> ...
> Call Trace:
> <TASK>
> i915_gem_userptr_get_pages+0x175/0x1a0 [i915]
> ____i915_gem_object_get_pages+0x32/0xb0 [i915]
> i915_gem_object_userptr_submit_init+0x286/0x470 [i915]
> eb_lookup_vmas+0x2ff/0xcf0 [i915]
> ? __intel_wakeref_get_first+0x55/0xb0 [i915]
> i915_gem_do_execbuffer+0x785/0x21d0 [i915]
> i915_gem_execbuffer2_ioctl+0xe7/0x3d0 [i915]
>
> We shouldn't be setting the obj->cache_dirty for DGFX,
> fix it.
With Fixes: |d70af57944 |("drm/i915/shmem: ensure flush during swap-in
on non-LLC")
Acked-by: Nirmoy Das <nirmoy.das@intel.com>
> Suggested-by: Matthew Auld<matthew.auld@intel.com>
> Reported-by: Niranjana Vishwanathapura<niranjana.vishwanathapura@intel.com>
> Signed-off-by: Niranjana Vishwanathapura<niranjana.vishwanathapura@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 11125c32dd35..2f7804492cd5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -369,14 +369,14 @@ __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj,
>
> __start_cpu_write(obj);
> /*
> - * On non-LLC platforms, force the flush-on-acquire if this is ever
> + * On non-LLC igfx platforms, force the flush-on-acquire if this is ever
> * swapped-in. Our async flush path is not trust worthy enough yet(and
> * happens in the wrong order), and with some tricks it's conceivable
> * for userspace to change the cache-level to I915_CACHE_NONE after the
> * pages are swapped-in, and since execbuf binds the object before doing
> * the async flush, we have a race window.
> */
> - if (!HAS_LLC(i915))
> + if (!HAS_LLC(i915) && !IS_DGFX(i915))
> obj->cache_dirty = true;
> }
>
[-- Attachment #2: Type: text/html, Size: 3108 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX
2022-11-02 7:39 ` [Intel-gfx] [PATCH] " Das, Nirmoy
@ 2022-11-02 10:36 ` Matthew Auld
2022-11-02 16:09 ` Das, Nirmoy
0 siblings, 1 reply; 9+ messages in thread
From: Matthew Auld @ 2022-11-02 10:36 UTC (permalink / raw)
To: Das, Nirmoy, Niranjana Vishwanathapura, intel-gfx
On 02/11/2022 07:39, Das, Nirmoy wrote:
>
> On 11/2/2022 6:14 AM, Niranjana Vishwanathapura wrote:
>> Currently on DG1, which do not have LLC, we hit the below
>> warning while rebinding an userptr invalidated object.
>>
>> WARNING: CPU: 4 PID: 13008 at drivers/gpu/drm/i915/gem/i915_gem_pages.c:34 __i915_gem_object_set_pages+0x296/0x2d0 [i915]
>> ...
>> RIP: 0010:__i915_gem_object_set_pages+0x296/0x2d0 [i915]
>> ...
>> Call Trace:
>> <TASK>
>> i915_gem_userptr_get_pages+0x175/0x1a0 [i915]
>> ____i915_gem_object_get_pages+0x32/0xb0 [i915]
>> i915_gem_object_userptr_submit_init+0x286/0x470 [i915]
>> eb_lookup_vmas+0x2ff/0xcf0 [i915]
>> ? __intel_wakeref_get_first+0x55/0xb0 [i915]
>> i915_gem_do_execbuffer+0x785/0x21d0 [i915]
>> i915_gem_execbuffer2_ioctl+0xe7/0x3d0 [i915]
>>
>> We shouldn't be setting the obj->cache_dirty for DGFX,
>> fix it.
>
> With Fixes: |d70af57944 |("drm/i915/shmem: ensure flush during swap-in
> on non-LLC")
>
> Acked-by: Nirmoy Das <nirmoy.das@intel.com>
Any idea why this escaped our testing in CI? Perhaps something to improve.
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>
>> Suggested-by: Matthew Auld<matthew.auld@intel.com>
>> Reported-by: Niranjana Vishwanathapura<niranjana.vishwanathapura@intel.com>
>> Signed-off-by: Niranjana Vishwanathapura<niranjana.vishwanathapura@intel.com>
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>> index 11125c32dd35..2f7804492cd5 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>> @@ -369,14 +369,14 @@ __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj,
>>
>> __start_cpu_write(obj);
>> /*
>> - * On non-LLC platforms, force the flush-on-acquire if this is ever
>> + * On non-LLC igfx platforms, force the flush-on-acquire if this is ever
>> * swapped-in. Our async flush path is not trust worthy enough yet(and
>> * happens in the wrong order), and with some tricks it's conceivable
>> * for userspace to change the cache-level to I915_CACHE_NONE after the
>> * pages are swapped-in, and since execbuf binds the object before doing
>> * the async flush, we have a race window.
>> */
>> - if (!HAS_LLC(i915))
>> + if (!HAS_LLC(i915) && !IS_DGFX(i915))
>> obj->cache_dirty = true;
>> }
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX
2022-11-02 10:36 ` Matthew Auld
@ 2022-11-02 16:09 ` Das, Nirmoy
2022-11-02 16:35 ` Niranjana Vishwanathapura
0 siblings, 1 reply; 9+ messages in thread
From: Das, Nirmoy @ 2022-11-02 16:09 UTC (permalink / raw)
To: Matthew Auld, Niranjana Vishwanathapura, intel-gfx
On 11/2/2022 11:36 AM, Matthew Auld wrote:
> On 02/11/2022 07:39, Das, Nirmoy wrote:
>>
>> On 11/2/2022 6:14 AM, Niranjana Vishwanathapura wrote:
>>> Currently on DG1, which do not have LLC, we hit the below
>>> warning while rebinding an userptr invalidated object.
>>>
>>> WARNING: CPU: 4 PID: 13008 at
>>> drivers/gpu/drm/i915/gem/i915_gem_pages.c:34
>>> __i915_gem_object_set_pages+0x296/0x2d0 [i915]
>>> ...
>>> RIP: 0010:__i915_gem_object_set_pages+0x296/0x2d0 [i915]
>>> ...
>>> Call Trace:
>>> <TASK>
>>> i915_gem_userptr_get_pages+0x175/0x1a0 [i915]
>>> ____i915_gem_object_get_pages+0x32/0xb0 [i915]
>>> i915_gem_object_userptr_submit_init+0x286/0x470 [i915]
>>> eb_lookup_vmas+0x2ff/0xcf0 [i915]
>>> ? __intel_wakeref_get_first+0x55/0xb0 [i915]
>>> i915_gem_do_execbuffer+0x785/0x21d0 [i915]
>>> i915_gem_execbuffer2_ioctl+0xe7/0x3d0 [i915]
>>>
>>> We shouldn't be setting the obj->cache_dirty for DGFX,
>>> fix it.
>>
>> With Fixes: |d70af57944 |("drm/i915/shmem: ensure flush during
>> swap-in on non-LLC")
>>
>> Acked-by: Nirmoy Das <nirmoy.das@intel.com>
>
> Any idea why this escaped our testing in CI? Perhaps something to
> improve.
I ran some userptr related igt tests none hit
__i915_gem_object_release_shmem . So I think we are missing
coverage here or I/CI isn't running such test.
Niranjana, what test did you ran to hit this case WARN ?
Regards,
Nirmoy
>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>
>>
>>> Suggested-by: Matthew Auld<matthew.auld@intel.com>
>>> Reported-by: Niranjana
>>> Vishwanathapura<niranjana.vishwanathapura@intel.com>
>>> Signed-off-by: Niranjana
>>> Vishwanathapura<niranjana.vishwanathapura@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>>> b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>>> index 11125c32dd35..2f7804492cd5 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>>> @@ -369,14 +369,14 @@ __i915_gem_object_release_shmem(struct
>>> drm_i915_gem_object *obj,
>>> __start_cpu_write(obj);
>>> /*
>>> - * On non-LLC platforms, force the flush-on-acquire if this is
>>> ever
>>> + * On non-LLC igfx platforms, force the flush-on-acquire if
>>> this is ever
>>> * swapped-in. Our async flush path is not trust worthy enough
>>> yet(and
>>> * happens in the wrong order), and with some tricks it's
>>> conceivable
>>> * for userspace to change the cache-level to I915_CACHE_NONE
>>> after the
>>> * pages are swapped-in, and since execbuf binds the object
>>> before doing
>>> * the async flush, we have a race window.
>>> */
>>> - if (!HAS_LLC(i915))
>>> + if (!HAS_LLC(i915) && !IS_DGFX(i915))
>>> obj->cache_dirty = true;
>>> }
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX
2022-11-02 16:09 ` Das, Nirmoy
@ 2022-11-02 16:35 ` Niranjana Vishwanathapura
0 siblings, 0 replies; 9+ messages in thread
From: Niranjana Vishwanathapura @ 2022-11-02 16:35 UTC (permalink / raw)
To: Das, Nirmoy; +Cc: intel-gfx, Matthew Auld
On Wed, Nov 02, 2022 at 05:09:27PM +0100, Das, Nirmoy wrote:
>
>On 11/2/2022 11:36 AM, Matthew Auld wrote:
>>On 02/11/2022 07:39, Das, Nirmoy wrote:
>>>
>>>On 11/2/2022 6:14 AM, Niranjana Vishwanathapura wrote:
>>>>Currently on DG1, which do not have LLC, we hit the below
>>>>warning while rebinding an userptr invalidated object.
>>>>
>>>>WARNING: CPU: 4 PID: 13008 at
>>>>drivers/gpu/drm/i915/gem/i915_gem_pages.c:34
>>>>__i915_gem_object_set_pages+0x296/0x2d0 [i915]
>>>>...
>>>>RIP: 0010:__i915_gem_object_set_pages+0x296/0x2d0 [i915]
>>>>...
>>>>Call Trace:
>>>> <TASK>
>>>> i915_gem_userptr_get_pages+0x175/0x1a0 [i915]
>>>> ____i915_gem_object_get_pages+0x32/0xb0 [i915]
>>>> i915_gem_object_userptr_submit_init+0x286/0x470 [i915]
>>>> eb_lookup_vmas+0x2ff/0xcf0 [i915]
>>>> ? __intel_wakeref_get_first+0x55/0xb0 [i915]
>>>> i915_gem_do_execbuffer+0x785/0x21d0 [i915]
>>>> i915_gem_execbuffer2_ioctl+0xe7/0x3d0 [i915]
>>>>
>>>>We shouldn't be setting the obj->cache_dirty for DGFX,
>>>>fix it.
>>>
>>>With Fixes: |d70af57944 |("drm/i915/shmem: ensure flush during
>>>swap-in on non-LLC")
>>>
Ok, will add.
>>>Acked-by: Nirmoy Das <nirmoy.das@intel.com>
>>
>>Any idea why this escaped our testing in CI? Perhaps something to
>>improve.
>
>
>I ran some userptr related igt tests none hit
>__i915_gem_object_release_shmem . So I think we are missing
>
>coverage here or I/CI isn't running such test.
>
>Niranjana, what test did you ran to hit this case WARN ?
>
I hit this issue with modified gem_userptr_blits@vma-merge where
I added additional execbuf call after userptr invalidation as below
to test rebind happens properly after an userptr invalidation.
igt_spin_end(spin);
+ igt_spin_reset(spin);
+
+ gem_execbuf_wr(i915, &spin->execbuf);
+ igt_spin_end(spin);
+
gem_close(i915, handle);
munmap(addr, sz);
Note that vma-merge subtest fails due to some other issue, but still
is good enough to reproduce this issue and test the fix.
Niranjana
>
>Regards,
>
>Nirmoy
>
>
>>
>>Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>>
>>>
>>>>Suggested-by: Matthew Auld<matthew.auld@intel.com>
>>>>Reported-by: Niranjana
>>>>Vishwanathapura<niranjana.vishwanathapura@intel.com>
>>>>Signed-off-by: Niranjana
>>>>Vishwanathapura<niranjana.vishwanathapura@intel.com>
>>>>---
>>>> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>>>>b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>>>>index 11125c32dd35..2f7804492cd5 100644
>>>>--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>>>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>>>>@@ -369,14 +369,14 @@ __i915_gem_object_release_shmem(struct
>>>>drm_i915_gem_object *obj,
>>>> __start_cpu_write(obj);
>>>> /*
>>>>- * On non-LLC platforms, force the flush-on-acquire if this
>>>>is ever
>>>>+ * On non-LLC igfx platforms, force the flush-on-acquire if
>>>>this is ever
>>>> * swapped-in. Our async flush path is not trust worthy
>>>>enough yet(and
>>>> * happens in the wrong order), and with some tricks it's
>>>>conceivable
>>>> * for userspace to change the cache-level to
>>>>I915_CACHE_NONE after the
>>>> * pages are swapped-in, and since execbuf binds the
>>>>object before doing
>>>> * the async flush, we have a race window.
>>>> */
>>>>- if (!HAS_LLC(i915))
>>>>+ if (!HAS_LLC(i915) && !IS_DGFX(i915))
>>>> obj->cache_dirty = true;
>>>> }
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX
2022-11-02 5:14 [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX Niranjana Vishwanathapura
` (2 preceding siblings ...)
2022-11-02 7:39 ` [Intel-gfx] [PATCH] " Das, Nirmoy
@ 2022-11-02 21:59 ` Andi Shyti
2022-11-02 22:31 ` Andi Shyti
4 siblings, 0 replies; 9+ messages in thread
From: Andi Shyti @ 2022-11-02 21:59 UTC (permalink / raw)
To: Niranjana Vishwanathapura; +Cc: intel-gfx, matthew.auld
Hi Niranjana,
On Tue, Nov 01, 2022 at 10:14:16PM -0700, Niranjana Vishwanathapura wrote:
> Currently on DG1, which do not have LLC, we hit the below
> warning while rebinding an userptr invalidated object.
>
> WARNING: CPU: 4 PID: 13008 at drivers/gpu/drm/i915/gem/i915_gem_pages.c:34 __i915_gem_object_set_pages+0x296/0x2d0 [i915]
> ...
> RIP: 0010:__i915_gem_object_set_pages+0x296/0x2d0 [i915]
> ...
> Call Trace:
> <TASK>
> i915_gem_userptr_get_pages+0x175/0x1a0 [i915]
> ____i915_gem_object_get_pages+0x32/0xb0 [i915]
> i915_gem_object_userptr_submit_init+0x286/0x470 [i915]
> eb_lookup_vmas+0x2ff/0xcf0 [i915]
> ? __intel_wakeref_get_first+0x55/0xb0 [i915]
> i915_gem_do_execbuffer+0x785/0x21d0 [i915]
> i915_gem_execbuffer2_ioctl+0xe7/0x3d0 [i915]
>
> We shouldn't be setting the obj->cache_dirty for DGFX,
> fix it.
>
> Suggested-by: Matthew Auld <matthew.auld@intel.com>
> Reported-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
The BAT failure doesn't look related to this patch, to me.
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Thanks,
Andi
> ---
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 11125c32dd35..2f7804492cd5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -369,14 +369,14 @@ __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj,
>
> __start_cpu_write(obj);
> /*
> - * On non-LLC platforms, force the flush-on-acquire if this is ever
> + * On non-LLC igfx platforms, force the flush-on-acquire if this is ever
> * swapped-in. Our async flush path is not trust worthy enough yet(and
> * happens in the wrong order), and with some tricks it's conceivable
> * for userspace to change the cache-level to I915_CACHE_NONE after the
> * pages are swapped-in, and since execbuf binds the object before doing
> * the async flush, we have a race window.
> */
> - if (!HAS_LLC(i915))
> + if (!HAS_LLC(i915) && !IS_DGFX(i915))
> obj->cache_dirty = true;
> }
>
> --
> 2.21.0.rc0.32.g243a4c7e27
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX
2022-11-02 5:14 [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX Niranjana Vishwanathapura
` (3 preceding siblings ...)
2022-11-02 21:59 ` Andi Shyti
@ 2022-11-02 22:31 ` Andi Shyti
4 siblings, 0 replies; 9+ messages in thread
From: Andi Shyti @ 2022-11-02 22:31 UTC (permalink / raw)
To: Niranjana Vishwanathapura; +Cc: intel-gfx, matthew.auld
Hi Niranjana,
On Tue, Nov 01, 2022 at 10:14:16PM -0700, Niranjana Vishwanathapura wrote:
> Currently on DG1, which do not have LLC, we hit the below
> warning while rebinding an userptr invalidated object.
>
> WARNING: CPU: 4 PID: 13008 at drivers/gpu/drm/i915/gem/i915_gem_pages.c:34 __i915_gem_object_set_pages+0x296/0x2d0 [i915]
> ...
> RIP: 0010:__i915_gem_object_set_pages+0x296/0x2d0 [i915]
> ...
> Call Trace:
> <TASK>
> i915_gem_userptr_get_pages+0x175/0x1a0 [i915]
> ____i915_gem_object_get_pages+0x32/0xb0 [i915]
> i915_gem_object_userptr_submit_init+0x286/0x470 [i915]
> eb_lookup_vmas+0x2ff/0xcf0 [i915]
> ? __intel_wakeref_get_first+0x55/0xb0 [i915]
> i915_gem_do_execbuffer+0x785/0x21d0 [i915]
> i915_gem_execbuffer2_ioctl+0xe7/0x3d0 [i915]
>
> We shouldn't be setting the obj->cache_dirty for DGFX,
> fix it.
>
> Suggested-by: Matthew Auld <matthew.auld@intel.com>
> Reported-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Pushed in drm-intel-gt-next.
Thanks,
Andi
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-11-02 22:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-02 5:14 [Intel-gfx] [PATCH] drm/i915: Do not set cache_dirty for DGFX Niranjana Vishwanathapura
2022-11-02 6:21 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
2022-11-02 6:45 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2022-11-02 7:39 ` [Intel-gfx] [PATCH] " Das, Nirmoy
2022-11-02 10:36 ` Matthew Auld
2022-11-02 16:09 ` Das, Nirmoy
2022-11-02 16:35 ` Niranjana Vishwanathapura
2022-11-02 21:59 ` Andi Shyti
2022-11-02 22:31 ` Andi Shyti
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.