* ✓ CI.Patch_applied: success for drm/xe: Check empty pinned BO list with lock held.
2024-05-28 11:54 [PATCH] drm/xe: Check empty pinned BO list with lock held Nirmoy Das
@ 2024-05-28 12:14 ` Patchwork
2024-05-28 12:14 ` ✓ CI.checkpatch: " Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2024-05-28 12:14 UTC (permalink / raw)
To: Nirmoy Das; +Cc: intel-xe
== Series Details ==
Series: drm/xe: Check empty pinned BO list with lock held.
URL : https://patchwork.freedesktop.org/series/134135/
State : success
== Summary ==
=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: 5ba74aaea55b drm-tip: 2024y-05m-28d-09h-56m-00s UTC integration manifest
=== git am output follows ===
Applying: drm/xe: Check empty pinned BO list with lock held.
^ permalink raw reply [flat|nested] 7+ messages in thread* ✓ CI.checkpatch: success for drm/xe: Check empty pinned BO list with lock held.
2024-05-28 11:54 [PATCH] drm/xe: Check empty pinned BO list with lock held Nirmoy Das
2024-05-28 12:14 ` ✓ CI.Patch_applied: success for " Patchwork
@ 2024-05-28 12:14 ` Patchwork
2024-05-28 12:15 ` ✗ CI.KUnit: failure " Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2024-05-28 12:14 UTC (permalink / raw)
To: Nirmoy Das; +Cc: intel-xe
== Series Details ==
Series: drm/xe: Check empty pinned BO list with lock held.
URL : https://patchwork.freedesktop.org/series/134135/
State : success
== Summary ==
+ KERNEL=/kernel
+ git clone https://gitlab.freedesktop.org/drm/maintainer-tools mt
Cloning into 'mt'...
warning: redirecting to https://gitlab.freedesktop.org/drm/maintainer-tools.git/
+ git -C mt rev-list -n1 origin/master
51ce9f6cd981d42d7467409d7dbc559a450abc1e
+ cd /kernel
+ git config --global --add safe.directory /kernel
+ git log -n1
commit 9fd2f70abab4b10902109adc80104c112d6b9894
Author: Nirmoy Das <nirmoy.das@intel.com>
Date: Tue May 28 13:54:08 2024 +0200
drm/xe: Check empty pinned BO list with lock held.
Use lock that is meant to use for accessing the BO pin list.
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
+ /mt/dim checkpatch 5ba74aaea55bf295898f2a4f69c59fe12e84e842 drm-intel
9fd2f70abab4 drm/xe: Check empty pinned BO list with lock held.
^ permalink raw reply [flat|nested] 7+ messages in thread* ✗ CI.KUnit: failure for drm/xe: Check empty pinned BO list with lock held.
2024-05-28 11:54 [PATCH] drm/xe: Check empty pinned BO list with lock held Nirmoy Das
2024-05-28 12:14 ` ✓ CI.Patch_applied: success for " Patchwork
2024-05-28 12:14 ` ✓ CI.checkpatch: " Patchwork
@ 2024-05-28 12:15 ` Patchwork
2024-05-28 20:56 ` [PATCH] " Matthew Brost
2024-05-29 9:12 ` ✗ CI.Patch_applied: failure for drm/xe: Check empty pinned BO list with lock held. (rev2) Patchwork
4 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2024-05-28 12:15 UTC (permalink / raw)
To: Nirmoy Das; +Cc: intel-xe
== Series Details ==
Series: drm/xe: Check empty pinned BO list with lock held.
URL : https://patchwork.freedesktop.org/series/134135/
State : failure
== Summary ==
+ trap cleanup EXIT
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
[12:14:31] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[12:14:35] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make ARCH=um O=.kunit --jobs=48
../lib/iomap.c:156:5: warning: no previous prototype for ‘ioread64_lo_hi’ [-Wmissing-prototypes]
156 | u64 ioread64_lo_hi(const void __iomem *addr)
| ^~~~~~~~~~~~~~
../lib/iomap.c:163:5: warning: no previous prototype for ‘ioread64_hi_lo’ [-Wmissing-prototypes]
163 | u64 ioread64_hi_lo(const void __iomem *addr)
| ^~~~~~~~~~~~~~
../lib/iomap.c:170:5: warning: no previous prototype for ‘ioread64be_lo_hi’ [-Wmissing-prototypes]
170 | u64 ioread64be_lo_hi(const void __iomem *addr)
| ^~~~~~~~~~~~~~~~
../lib/iomap.c:178:5: warning: no previous prototype for ‘ioread64be_hi_lo’ [-Wmissing-prototypes]
178 | u64 ioread64be_hi_lo(const void __iomem *addr)
| ^~~~~~~~~~~~~~~~
../lib/iomap.c:264:6: warning: no previous prototype for ‘iowrite64_lo_hi’ [-Wmissing-prototypes]
264 | void iowrite64_lo_hi(u64 val, void __iomem *addr)
| ^~~~~~~~~~~~~~~
../lib/iomap.c:272:6: warning: no previous prototype for ‘iowrite64_hi_lo’ [-Wmissing-prototypes]
272 | void iowrite64_hi_lo(u64 val, void __iomem *addr)
| ^~~~~~~~~~~~~~~
../lib/iomap.c:280:6: warning: no previous prototype for ‘iowrite64be_lo_hi’ [-Wmissing-prototypes]
280 | void iowrite64be_lo_hi(u64 val, void __iomem *addr)
| ^~~~~~~~~~~~~~~~~
../lib/iomap.c:288:6: warning: no previous prototype for ‘iowrite64be_hi_lo’ [-Wmissing-prototypes]
288 | void iowrite64be_hi_lo(u64 val, void __iomem *addr)
| ^~~~~~~~~~~~~~~~~
[12:15:02] Starting KUnit Kernel (1/1)...
[12:15:02] ============================================================
Running tests with:
$ .kunit/linux kunit.enable=1 mem=1G console=tty kunit_shutdown=halt
[12:15:02] =================== guc_dbm (7 subtests) ===================
[12:15:02] [PASSED] test_empty
[12:15:02] [PASSED] test_default
[12:15:02] ======================== test_size ========================
[12:15:02] [PASSED] 4
[12:15:02] [PASSED] 8
[12:15:02] [PASSED] 32
[12:15:02] [PASSED] 256
[12:15:02] ==================== [PASSED] test_size ====================
[12:15:02] ======================= test_reuse ========================
[12:15:02] [PASSED] 4
[12:15:02] [PASSED] 8
[12:15:02] [PASSED] 32
[12:15:02] [PASSED] 256
[12:15:02] =================== [PASSED] test_reuse ====================
[12:15:02] =================== test_range_overlap ====================
[12:15:02] [PASSED] 4
[12:15:02] [PASSED] 8
[12:15:02] [PASSED] 32
[12:15:02] [PASSED] 256
[12:15:02] =============== [PASSED] test_range_overlap ================
[12:15:02] =================== test_range_compact ====================
[12:15:02] [PASSED] 4
[12:15:02] [PASSED] 8
[12:15:02] [PASSED] 32
[12:15:02] [PASSED] 256
[12:15:02] =============== [PASSED] test_range_compact ================
[12:15:02] ==================== test_range_spare =====================
[12:15:02] [PASSED] 4
[12:15:02] [PASSED] 8
[12:15:02] [PASSED] 32
[12:15:02] [PASSED] 256
[12:15:02] ================ [PASSED] test_range_spare =================
[12:15:02] ===================== [PASSED] guc_dbm =====================
[12:15:02] =================== guc_idm (6 subtests) ===================
[12:15:02] [PASSED] bad_init
[12:15:02] [PASSED] no_init
[12:15:02] [PASSED] init_fini
[12:15:02] [PASSED] check_used
[12:15:02] [PASSED] check_quota
[12:15:02] [PASSED] check_all
[12:15:02] ===================== [PASSED] guc_idm =====================
[12:15:02] ================== no_relay (3 subtests) ===================
[12:15:02] [PASSED] xe_drops_guc2pf_if_not_ready
[12:15:02] [PASSED] xe_drops_guc2vf_if_not_ready
[12:15:02] [PASSED] xe_rejects_send_if_not_ready
[12:15:02] ==================== [PASSED] no_relay =====================
[12:15:02] ================== pf_relay (14 subtests) ==================
[12:15:02] [PASSED] pf_rejects_guc2pf_too_short
[12:15:02] [PASSED] pf_rejects_guc2pf_too_long
[12:15:02] [PASSED] pf_rejects_guc2pf_no_payload
[12:15:02] [PASSED] pf_fails_no_payload
[12:15:02] [PASSED] pf_fails_bad_origin
[12:15:02] [PASSED] pf_fails_bad_type
[12:15:02] [PASSED] pf_txn_reports_error
[12:15:02] [PASSED] pf_txn_sends_pf2guc
[12:15:02] [PASSED] pf_sends_pf2guc
[12:15:02] [SKIPPED] pf_loopback_nop
[12:15:02] [SKIPPED] pf_loopback_echo
[12:15:02] [SKIPPED] pf_loopback_fail
[12:15:02] [SKIPPED] pf_loopback_busy
[12:15:02] [SKIPPED] pf_loopback_retry
[12:15:02] ==================== [PASSED] pf_relay =====================
[12:15:02] ================== vf_relay (3 subtests) ===================
[12:15:02] [PASSED] vf_rejects_guc2vf_too_short
[12:15:02] [PASSED] vf_rejects_guc2vf_too_long
[12:15:02] [PASSED] vf_rejects_guc2vf_no_payload
[12:15:02] ==================== [PASSED] vf_relay =====================
[12:15:02] ================= pf_service (11 subtests) =================
[12:15:02] [PASSED] pf_negotiate_any
[12:15:02] [PASSED] pf_negotiate_base_match
[12:15:02] [PASSED] pf_negotiate_base_newer
[12:15:02] [PASSED] pf_negotiate_base_next
[12:15:02] [SKIPPED] pf_negotiate_base_older
[12:15:02] [PASSED] pf_negotiate_base_prev
[12:15:02] [PASSED] pf_negotiate_latest_match
[12:15:02] [PASSED] pf_negotiate_latest_newer
[12:15:02] [PASSED] pf_negotiate_latest_next
[12:15:02] [SKIPPED] pf_negotiate_latest_older
[12:15:02] [SKIPPED] pf_negotiate_latest_prev
[12:15:02] =================== [PASSED] pf_service ====================
[12:15:02] ===================== lmtt (1 subtest) =====================
[12:15:02] ======================== test_ops =========================
[12:15:02] [PASSED] 2-level
[12:15:02] [PASSED] multi-level
[12:15:02] ==================== [PASSED] test_ops =====================
[12:15:02] ====================== [PASSED] lmtt =======================
[12:15:02] ==================== xe_bo (2 subtests) ====================
[12:15:02] [SKIPPED] xe_ccs_migrate_kunit
[12:15:02] [SKIPPED] xe_bo_evict_kunit
[12:15:02] ===================== [SKIPPED] xe_bo ======================
[12:15:02] ================== xe_dma_buf (1 subtest) ==================
[12:15:02] [SKIPPED] xe_dma_buf_kunit
[12:15:02] =================== [SKIPPED] xe_dma_buf ===================
[12:15:02] ================== xe_migrate (1 subtest) ==================
[12:15:02] [SKIPPED] xe_migrate_sanity_kunit
[12:15:02] =================== [SKIPPED] xe_migrate ===================
[12:15:02] =================== xe_mocs (2 subtests) ===================
[12:15:02] [SKIPPED] xe_live_mocs_kernel_kunit
[12:15:02] [SKIPPED] xe_live_mocs_reset_kunit
[12:15:02] ==================== [SKIPPED] xe_mocs =====================
[12:15:02] ==================== args (11 subtests) ====================
[12:15:02] [PASSED] count_args_test
[12:15:02] [PASSED] call_args_example
[12:15:02] [PASSED] call_args_test
[12:15:02] [PASSED] drop_first_arg_example
[12:15:02] [PASSED] drop_first_arg_test
[12:15:02] [PASSED] first_arg_example
[12:15:02] [PASSED] first_arg_test
[12:15:02] [PASSED] last_arg_example
[12:15:02] [PASSED] last_arg_test
[12:15:02] [PASSED] pick_arg_example
[12:15:02] [PASSED] sep_comma_example
[12:15:02] ====================== [PASSED] args =======================
[12:15:02] =================== xe_pci (2 subtests) ====================
[12:15:02] [PASSED] xe_gmdid_graphics_ip
[12:15:02] [PASSED] xe_gmdid_media_ip
[12:15:02] ===================== [PASSED] xe_pci ======================
[12:15:02] ==================== xe_rtp (1 subtest) ====================
[12:15:02] ================== xe_rtp_process_tests ===================
[12:15:02] [PASSED] coalesce-same-reg
[12:15:02] [PASSED] no-match-no-add
[12:15:02] [PASSED] no-match-no-add-multiple-rules
[12:15:02] [PASSED] two-regs-two-entries
[12:15:02] [PASSED] clr-one-set-other
[12:15:02] [PASSED] set-field
[12:15:02] [PASSED] conflict-duplicate
[12:15:02] [PASSED] conflict-not-disjoint
[12:15:02] [PASSED] conflict-reg-type
[12:15:02] ============== [PASSED] xe_rtp_process_tests ===============
stty: 'standard input': Inappropriate ioctl for device
[12:15:02] ===================== [PASSED] xe_rtp ======================
[12:15:02] ==================== xe_wa (1 subtest) =====================
[12:15:02] ======================== xe_wa_gt =========================
[12:15:02] [PASSED] TIGERLAKE (B0)
[12:15:02] [PASSED] DG1 (A0)
[12:15:02] [PASSED] DG1 (B0)
[12:15:02] [PASSED] ALDERLAKE_S (A0)
[12:15:02] [PASSED] ALDERLAKE_S (B0)
[12:15:02] [PASSED] ALDERLAKE_S (C0)
[12:15:02] [PASSED] ALDERLAKE_S (D0)
[12:15:02] [PASSED] ALDERLAKE_P (A0)
[12:15:02] [PASSED] ALDERLAKE_P (B0)
[12:15:02] [PASSED] ALDERLAKE_P (C0)
[12:15:02] [PASSED] ALDERLAKE_S_RPLS (D0)
[12:15:02] [PASSED] ALDERLAKE_P_RPLU (E0)
[12:15:02] [PASSED] DG2_G10 (C0)
[12:15:02] [PASSED] DG2_G11 (B1)
[12:15:02] [PASSED] DG2_G12 (A1)
[12:15:02] [PASSED] METEORLAKE (g:A0, m:A0)
[12:15:02] [PASSED] METEORLAKE (g:A0, m:A0)
[12:15:02] [PASSED] METEORLAKE (g:A0, m:A0)
[12:15:02] [PASSED] LUNARLAKE (g:A0, m:A0)
[12:15:02] [PASSED] LUNARLAKE (g:B0, m:A0)
[12:15:02] ==================== [PASSED] xe_wa_gt =====================
[12:15:02] ====================== [PASSED] xe_wa ======================
[12:15:02] ============================================================
[12:15:02] Testing complete. Ran 109 tests: passed: 95, skipped: 14
[12:15:02] Elapsed time: 31.095s total, 4.265s configuring, 26.559s building, 0.224s running
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/tests/.kunitconfig
ERROR:root:
WARNING: unmet direct dependencies detected for DRM_DISPLAY_HDMI_STATE_HELPER
Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && DRM_DISPLAY_HELPER [=y] && DRM_DISPLAY_HDMI_HELPER [=n]
Selected by [y]:
- DRM_KUNIT_TEST [=y] && HAS_IOMEM [=y] && DRM [=y] && KUNIT [=y] && MMU [=y]
WARNING: unmet direct dependencies detected for DRM_DISPLAY_HDMI_STATE_HELPER
Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && DRM_DISPLAY_HELPER [=y] && DRM_DISPLAY_HDMI_HELPER [=n]
Selected by [y]:
- DRM_KUNIT_TEST [=y] && HAS_IOMEM [=y] && DRM [=y] && KUNIT [=y] && MMU [=y]
WARNING: unmet direct dependencies detected for DRM_DISPLAY_HDMI_STATE_HELPER
Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && DRM_DISPLAY_HELPER [=y] && DRM_DISPLAY_HDMI_HELPER [=n]
Selected by [y]:
- DRM_KUNIT_TEST [=y] && HAS_IOMEM [=y] && DRM [=y] && KUNIT [=y] && MMU [=y]
../lib/iomap.c:156:5: warning: no previous prototype for ‘ioread64_lo_hi’ [-Wmissing-prototypes]
156 | u64 ioread64_lo_hi(const void __iomem *addr)
| ^~~~~~~~~~~~~~
../lib/iomap.c:163:5: warning: no previous prototype for ‘ioread64_hi_lo’ [-Wmissing-prototypes]
163 | u64 ioread64_hi_lo(const void __iomem *addr)
| ^~~~~~~~~~~~~~
../lib/iomap.c:170:5: warning: no previous prototype for ‘ioread64be_lo_hi’ [-Wmissing-prototypes]
170 | u64 ioread64be_lo_hi(const void __iomem *addr)
| ^~~~~~~~~~~~~~~~
../lib/iomap.c:178:5: warning: no previous prototype for ‘ioread64be_hi_lo’ [-Wmissing-prototypes]
178 | u64 ioread64be_hi_lo(const void __iomem *addr)
| ^~~~~~~~~~~~~~~~
../lib/iomap.c:264:6: warning: no previous prototype for ‘iowrite64_lo_hi’ [-Wmissing-prototypes]
264 | void iowrite64_lo_hi(u64 val, void __iomem *addr)
| ^~~~~~~~~~~~~~~
../lib/iomap.c:272:6: warning: no previous prototype for ‘iowrite64_hi_lo’ [-Wmissing-prototypes]
272 | void iowrite64_hi_lo(u64 val, void __iomem *addr)
| ^~~~~~~~~~~~~~~
../lib/iomap.c:280:6: warning: no previous prototype for ‘iowrite64be_lo_hi’ [-Wmissing-prototypes]
280 | void iowrite64be_lo_hi(u64 val, void __iomem *addr)
| ^~~~~~~~~~~~~~~~~
../lib/iomap.c:288:6: warning: no previous prototype for ‘iowrite64be_hi_lo’ [-Wmissing-prototypes]
288 | void iowrite64be_hi_lo(u64 val, void __iomem *addr)
| ^~~~~~~~~~~~~~~~~
/usr/bin/ld: drivers/gpu/drm/tests/drm_connector_test.o: in function `drm_test_connector_hdmi_compute_mode_clock_yuv422_12_bpc':
drm_connector_test.c:(.text+0x570): undefined reference to `drm_hdmi_compute_mode_clock'
/usr/bin/ld: drivers/gpu/drm/tests/drm_connector_test.o: in function `drm_test_connector_hdmi_compute_mode_clock_yuv422_10_bpc':
drm_connector_test.c:(.text+0x760): undefined reference to `drm_hdmi_compute_mode_clock'
/usr/bin/ld: drivers/gpu/drm/tests/drm_connector_test.o: in function `drm_test_connector_hdmi_compute_mode_clock_yuv422_8_bpc':
drm_connector_test.c:(.text+0x950): undefined reference to `drm_hdmi_compute_mode_clock'
/usr/bin/ld: drivers/gpu/drm/tests/drm_connector_test.o: in function `drm_test_connector_hdmi_compute_mode_clock_yuv420_12_bpc':
drm_connector_test.c:(.text+0xb40): undefined reference to `drm_hdmi_compute_mode_clock'
/usr/bin/ld: drivers/gpu/drm/tests/drm_connector_test.o: in function `drm_test_connector_hdmi_compute_mode_clock_yuv420_10_bpc':
drm_connector_test.c:(.text+0xd30): undefined reference to `drm_hdmi_compute_mode_clock'
/usr/bin/ld: drivers/gpu/drm/tests/drm_connector_test.o:drm_connector_test.c:(.text+0xf26): more undefined references to `drm_hdmi_compute_mode_clock' follow
/usr/bin/ld: drivers/gpu/drm/display/drm_hdmi_state_helper.o: in function `drm_atomic_helper_connector_hdmi_check':
drm_hdmi_state_helper.c:(.text+0x9d8): undefined reference to `drm_hdmi_avi_infoframe_colorimetry'
/usr/bin/ld: drm_hdmi_state_helper.c:(.text+0x9ea): undefined reference to `drm_hdmi_avi_infoframe_bars'
/usr/bin/ld: drm_hdmi_state_helper.c:(.text+0xa54): undefined reference to `drm_hdmi_infoframe_set_hdr_metadata'
collect2: error: ld returned 1 exit status
make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1
make[2]: *** [/kernel/Makefile:1171: vmlinux] Error 2
make[1]: *** [/kernel/Makefile:240: __sub-make] Error 2
make: *** [Makefile:240: __sub-make] Error 2
[12:15:02] Configuring KUnit Kernel ...
Regenerating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[12:15:04] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make ARCH=um O=.kunit --jobs=48
+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/xe: Check empty pinned BO list with lock held.
2024-05-28 11:54 [PATCH] drm/xe: Check empty pinned BO list with lock held Nirmoy Das
` (2 preceding siblings ...)
2024-05-28 12:15 ` ✗ CI.KUnit: failure " Patchwork
@ 2024-05-28 20:56 ` Matthew Brost
2024-05-29 8:35 ` Nirmoy Das
2024-05-29 9:12 ` ✗ CI.Patch_applied: failure for drm/xe: Check empty pinned BO list with lock held. (rev2) Patchwork
4 siblings, 1 reply; 7+ messages in thread
From: Matthew Brost @ 2024-05-28 20:56 UTC (permalink / raw)
To: Nirmoy Das; +Cc: intel-xe
On Tue, May 28, 2024 at 01:54:08PM +0200, Nirmoy Das wrote:
> Use lock that is meant to use for accessing the BO pin list.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
Agree with this patch removing removing micro optimization of bypassing
&xe->pinned.lock, we should avoid thing like this.
Curious if this was an actual problem (i.e. was their a bug report which
lead to this change)?
Anyways the patch LGTM. With that:
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/xe/xe_bo.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 03f7fe7acf8c..2bae01ce4e5b 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1758,11 +1758,10 @@ void xe_bo_unpin_external(struct xe_bo *bo)
> xe_assert(xe, xe_bo_is_pinned(bo));
> xe_assert(xe, xe_bo_is_user(bo));
>
> - if (bo->ttm.pin_count == 1 && !list_empty(&bo->pinned_link)) {
> - spin_lock(&xe->pinned.lock);
> + spin_lock(&xe->pinned.lock);
> + if (bo->ttm.pin_count == 1 && !list_empty(&bo->pinned_link))
> list_del_init(&bo->pinned_link);
> - spin_unlock(&xe->pinned.lock);
> - }
> + spin_unlock(&xe->pinned.lock);
>
> ttm_bo_unpin(&bo->ttm);
>
> @@ -1785,9 +1784,8 @@ void xe_bo_unpin(struct xe_bo *bo)
> struct ttm_place *place = &(bo->placements[0]);
>
> if (mem_type_is_vram(place->mem_type)) {
> - xe_assert(xe, !list_empty(&bo->pinned_link));
> -
> spin_lock(&xe->pinned.lock);
> + xe_assert(xe, !list_empty(&bo->pinned_link));
> list_del_init(&bo->pinned_link);
> spin_unlock(&xe->pinned.lock);
> }
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drm/xe: Check empty pinned BO list with lock held.
2024-05-28 20:56 ` [PATCH] " Matthew Brost
@ 2024-05-29 8:35 ` Nirmoy Das
0 siblings, 0 replies; 7+ messages in thread
From: Nirmoy Das @ 2024-05-29 8:35 UTC (permalink / raw)
To: Matthew Brost, Nirmoy Das; +Cc: intel-xe
Hi Matt,
On 5/28/2024 10:56 PM, Matthew Brost wrote:
> On Tue, May 28, 2024 at 01:54:08PM +0200, Nirmoy Das wrote:
>> Use lock that is meant to use for accessing the BO pin list.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Agree with this patch removing removing micro optimization of bypassing
> &xe->pinned.lock, we should avoid thing like this.
>
> Curious if this was an actual problem (i.e. was their a bug report which
> lead to this change)?
There was a warning from static analyzer tool so no real bug report yet.
>
> Anyways the patch LGTM. With that:
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Thanks,
Nirmoy
>
>> ---
>> drivers/gpu/drm/xe/xe_bo.c | 10 ++++------
>> 1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
>> index 03f7fe7acf8c..2bae01ce4e5b 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.c
>> +++ b/drivers/gpu/drm/xe/xe_bo.c
>> @@ -1758,11 +1758,10 @@ void xe_bo_unpin_external(struct xe_bo *bo)
>> xe_assert(xe, xe_bo_is_pinned(bo));
>> xe_assert(xe, xe_bo_is_user(bo));
>>
>> - if (bo->ttm.pin_count == 1 && !list_empty(&bo->pinned_link)) {
>> - spin_lock(&xe->pinned.lock);
>> + spin_lock(&xe->pinned.lock);
>> + if (bo->ttm.pin_count == 1 && !list_empty(&bo->pinned_link))
>> list_del_init(&bo->pinned_link);
>> - spin_unlock(&xe->pinned.lock);
>> - }
>> + spin_unlock(&xe->pinned.lock);
>>
>> ttm_bo_unpin(&bo->ttm);
>>
>> @@ -1785,9 +1784,8 @@ void xe_bo_unpin(struct xe_bo *bo)
>> struct ttm_place *place = &(bo->placements[0]);
>>
>> if (mem_type_is_vram(place->mem_type)) {
>> - xe_assert(xe, !list_empty(&bo->pinned_link));
>> -
>> spin_lock(&xe->pinned.lock);
>> + xe_assert(xe, !list_empty(&bo->pinned_link));
>> list_del_init(&bo->pinned_link);
>> spin_unlock(&xe->pinned.lock);
>> }
>> --
>> 2.42.0
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✗ CI.Patch_applied: failure for drm/xe: Check empty pinned BO list with lock held. (rev2)
2024-05-28 11:54 [PATCH] drm/xe: Check empty pinned BO list with lock held Nirmoy Das
` (3 preceding siblings ...)
2024-05-28 20:56 ` [PATCH] " Matthew Brost
@ 2024-05-29 9:12 ` Patchwork
4 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2024-05-29 9:12 UTC (permalink / raw)
To: Nirmoy Das; +Cc: intel-xe
== Series Details ==
Series: drm/xe: Check empty pinned BO list with lock held. (rev2)
URL : https://patchwork.freedesktop.org/series/134135/
State : failure
== Summary ==
=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: b84122b07cfd drm-tip: 2024y-05m-29d-09h-06m-15s UTC integration manifest
=== git am output follows ===
error: patch failed: drivers/gpu/drm/xe/xe_bo.c:1758
error: drivers/gpu/drm/xe/xe_bo.c: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: drm/xe: Check empty pinned BO list with lock held.
Patch failed at 0001 drm/xe: Check empty pinned BO list with lock held.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
^ permalink raw reply [flat|nested] 7+ messages in thread