Message ID | 20220222133209.587978-1-thomas.hellstrom@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] drm/i915: Clarify vma lifetime | expand |
On Tue, 2022-02-22 at 17:14 +0000, Patchwork wrote: > Patch Details > Series:drm/i915: Clarify vma lifetime > (rev2)URL:https://patchwork.freedesktop.org/series/99948/State:failur > e > Details:https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22350/index.html > CI Bug Log - changes from CI_DRM_11265_full -> Patchwork_22350_fullSummaryFAILURE > Serious unknown changes coming with Patchwork_22350_full absolutely > need to be > verified manually. > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_22350_full, please notify your bug team to > allow them > to document this new failure mode, which will reduce false positives > in CI. > Participating hosts (13 -> 13)No changes in participating hosts > Possible new issuesHere are the unknown changes that may have been introduced in > Patchwork_22350_full: > IGT changesPossible regressions * igt@i915_pm_dc@dc5-dpms:shard-tglb: PASS -> FAIL > * igt@kms_rotation_crc@primary-rotation-180: > - shard-tglb: PASS -> INCOMPLETE Errors are unrelated. /Thomas > Known issuesHere are the changes found in Patchwork_22350_full that come from > known issues: > IGT changesIssues hit * igt@gem_ctx_isolation@preservation-s3@rcs0:shard-skl: PASS -> > INCOMPLETE ([i915#4793]) > * igt@gem_exec_capture@pi@rcs0:shard-iclb: PASS -> INCOMPLETE > ([i915#3371]) > * igt@gem_exec_capture@pi@vecs0:shard-skl: NOTRUN -> INCOMPLETE > ([i915#4547]) > * igt@gem_exec_fair@basic-pace-share@rcs0:shard-iclb: PASS -> FAIL > ([i915#2842]) > * igt@gem_exec_fair@basic-pace-solo@rcs0:shard-kbl: PASS -> FAIL > ([i915#2842])shard-tglb: PASS -> FAIL ([i915#2851]) > * igt@gem_exec_fair@basic-throttle@rcs0:shard-iclb: PASS -> FAIL > ([i915#2849]) > * igt@gem_huc_copy@huc-copy:shard-tglb: PASS -> SKIP ([i915#2190]) > * igt@gem_lmem_swapping@basic:shard-kbl: NOTRUN -> SKIP ([fdo#109271] > / [i915#4613]) > * igt@gem_lmem_swapping@random:shard-iclb: NOTRUN -> SKIP > ([i915#4613]) > * igt@gem_pxp@reject-modify-context-protection-off-1:shard-iclb: > NOTRUN -> SKIP ([i915#4270]) +1 similar issue > * igt@gem_userptr_blits@coherency-unsync:shard-iclb: NOTRUN -> SKIP > ([i915#3297]) > * igt@gem_userptr_blits@dmabuf-sync:shard-skl: NOTRUN -> SKIP > ([fdo#109271] / [i915#3323]) > * igt@gen7_exec_parse@chained-batch:shard-iclb: NOTRUN -> SKIP > ([fdo#109289]) +1 similar issue > * igt@gen9_exec_parse@allowed-all:shard-glk: PASS -> DMESG-WARN > ([i915#1436] / [i915#716]) > * igt@i915_pm_dc@dc6-psr:shard-iclb: NOTRUN -> FAIL ([i915#454]) > * igt@i915_selftest@mock@hugepages:shard-skl: NOTRUN -> INCOMPLETE > ([i915#5123])shard-kbl: NOTRUN -> INCOMPLETE ([i915#5123]) > * igt@i915_suspend@debugfs-reader:shard-apl: PASS -> DMESG-WARN > ([i915#180]) +1 similar issue > * igt@kms_atomic_transition@plane-all-modeset-transition:shard-iclb: > NOTRUN -> SKIP ([i915#1769]) > * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async- > flip:shard-skl: NOTRUN -> SKIP ([fdo#109271] / [i915#3777]) > * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-180-async- > flip:shard-iclb: NOTRUN -> SKIP ([fdo#110723]) > * igt@kms_ccs@pipe-b-ccs-on-another-bo-y_tiled_gen12_mc_ccs:shard-kbl: > NOTRUN -> SKIP ([fdo#109271] / [i915#3886]) +2 similar issues > * igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc:shard- > iclb: NOTRUN -> SKIP ([fdo#109278] / [i915#3886]) +2 similar issues > * igt@kms_ccs@pipe-c-crc-primary-rotation-180- > y_tiled_gen12_rc_ccs_cc:shard-skl: NOTRUN -> SKIP ([fdo#109271] / > [i915#3886]) +3 similar issues > * igt@kms_chamelium@hdmi-hpd-storm-disable:shard-skl: NOTRUN -> SKIP > ([fdo#109271] / [fdo#111827]) +7 similar issues > * igt@kms_chamelium@hdmi-hpd-with-enabled-mode:shard-kbl: NOTRUN -> > SKIP ([fdo#109271] / [fdo#111827]) +4 similar issues > * igt@kms_chamelium@vga-hpd-enable-disable-mode:shard-tglb: NOTRUN -> > SKIP ([fdo#109284] / [fdo#111827]) > * igt@kms_color@pipe-d-ctm-max:shard-iclb: NOTRUN -> SKIP > ([fdo#109278] / [i915#1149]) > * igt@kms_color_chamelium@pipe-a-ctm-negative:shard-snb: NOTRUN -> > SKIP ([fdo#109271] / [fdo#111827]) +4 similar issues > * igt@kms_color_chamelium@pipe-b-gamma:shard-iclb: NOTRUN -> SKIP > ([fdo#109284] / [fdo#111827]) +3 similar issues > * igt@kms_content_protection@dp-mst-type-0:shard-iclb: NOTRUN -> SKIP > ([i915#3116]) > * igt@kms_content_protection@type1:shard-tglb: NOTRUN -> SKIP > ([i915#1063]) > * igt@kms_cursor_crc@pipe-b-cursor-32x32-offscreen:shard-tglb: NOTRUN > -> SKIP ([i915#3319]) > * igt@kms_cursor_legacy@cursorb-vs-flipa-varying-size:shard-iclb: > NOTRUN -> SKIP ([fdo#109274] / [fdo#109278]) > * igt@kms_cursor_legacy@pipe-d-torture-move:shard-skl: NOTRUN -> SKIP > ([fdo#109271]) +94 similar issues > * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-cpu-untiled:shard-skl: > PASS -> DMESG-WARN ([i915#1982]) +1 similar issue > * igt@kms_fbcon_fbt@fbc-suspend:shard-kbl: PASS -> INCOMPLETE > ([i915#180] / [i915#636]) > * igt@kms_flip@2x-flip-vs-panning:shard-iclb: NOTRUN -> SKIP > ([fdo#109274]) +1 similar issue > * igt@kms_flip@flip-vs-expired-vblank@a-edp1:shard-skl: PASS -> FAIL > ([i915#79]) > * igt@kms_flip@plain-flip-ts-check@c-edp1:shard-skl: PASS -> FAIL > ([i915#2122]) +1 similar issue > * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs- > downscaling:shard-iclb: NOTRUN -> SKIP ([i915#2587]) > * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw- > blt:shard-kbl: NOTRUN -> SKIP ([fdo#109271]) +55 similar issues > * igt@kms_frontbuffer_tracking@fbc-suspend:shard-kbl: PASS -> DMESG- > WARN ([i915#180]) +4 similar issues > * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-indfb-plflip- > blt:shard-tglb: NOTRUN -> SKIP ([fdo#109280] / [fdo#111825]) +2 > similar issues > * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-blt:shard-snb: > NOTRUN -> SKIP ([fdo#109271]) +89 similar issues > * igt@kms_frontbuffer_tracking@psr-2p-primscrn-shrfb-plflip-blt:shard- > iclb: NOTRUN -> SKIP ([fdo#109280]) +14 similar issues > * igt@kms_frontbuffer_tracking@psr-suspend:shard-iclb: PASS -> FAIL > ([i915#1888] / [i915#2546]) > * igt@kms_hdr@bpc-switch:shard-skl: PASS -> FAIL ([i915#1188]) > * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence:shard- > kbl: NOTRUN -> SKIP ([fdo#109271] / [i915#533]) > * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d:shard-skl: NOTRUN -> > SKIP ([fdo#109271] / [i915#533]) > * igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb:shard-skl: > NOTRUN -> FAIL ([i915#265]) > * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:shard-skl: PASS -> > FAIL ([fdo#108145] / [i915#265]) > * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:shard-skl: NOTRUN - > > FAIL ([fdo#108145] / [i915#265]) +1 similar issue > * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:shard-kbl: NOTRUN -> > FAIL ([fdo#108145] / [i915#265]) > * igt@kms_psr2_su@frontbuffer-xrgb8888:shard-iclb: NOTRUN -> SKIP > ([fdo#109642] / [fdo#111068] / [i915#658]) > * igt@kms_psr2_su@page_flip-xrgb8888:shard-kbl: NOTRUN -> SKIP > ([fdo#109271] / [i915#658]) > * igt@kms_psr@psr2_cursor_mmap_gtt:shard-iclb: NOTRUN -> SKIP > ([fdo#109441]) > * igt@kms_psr@psr2_sprite_plane_move:shard-iclb: PASS -> SKIP > ([fdo#109441]) +1 similar issue > * igt@kms_vblank@pipe-a-ts-continuation-suspend:shard-kbl: PASS -> > DMESG-WARN ([i915#180] / [i915#295]) > * igt@kms_vblank@pipe-d-ts-continuation-idle-hang:shard-iclb: NOTRUN - > > SKIP ([fdo#109278]) +20 similar issues > * igt@nouveau_crc@pipe-c-ctx-flip-skip-current-frame:shard-iclb: > NOTRUN -> SKIP ([i915#2530]) > * igt@nouveau_crc@pipe-d-ctx-flip-detection:shard-iclb: NOTRUN -> SKIP > ([fdo#109278] / [i915#2530]) > * igt@nouveau_crc@pipe-d-source-outp-inactive:shard-tglb: NOTRUN -> > SKIP ([i915#2530]) > * igt@perf@short-reads:shard-skl: PASS -> FAIL ([i915#51]) > * igt@prime_nv_pcopy@test1_macro:shard-tglb: NOTRUN -> SKIP > ([fdo#109291]) > * igt@prime_nv_pcopy@test3_5:shard-iclb: NOTRUN -> SKIP ([fdo#109291]) > +1 similar issue > * igt@syncobj_timeline@invalid-transfer-non-existent-point:shard-kbl: > NOTRUN -> DMESG-WARN ([i915#5098]) > * igt@sysfs_clients@fair-7:shard-iclb: NOTRUN -> SKIP ([i915#2994]) > Possible fixes * igt@fbdev@unaligned-write:{shard-rkl}: SKIP ([i915#2582]) -> PASS > * igt@gem_ctx_isolation@preservation-s3@vcs0:shard-kbl: DMESG-WARN > ([i915#180]) -> PASS +6 similar issues > * igt@gem_eio@suspend:{shard-rkl}: (PASS, FAIL) ([i915#5115]) -> PASS > * igt@gem_exec_big@single:shard-tglb: TIMEOUT ([i915#2647]) -> PASS > * igt@gem_exec_capture@pi@bcs0:shard-skl: INCOMPLETE ([i915#4547]) -> > PASS > * igt@gem_exec_fair@basic-flow@rcs0:shard-tglb: FAIL ([i915#2842]) -> > PASS > * igt@gem_exec_fair@basic-none@vecs0:shard-apl: FAIL ([i915#2842]) -> > PASS > * igt@gem_exec_fair@basic-pace@rcs0:shard-iclb: FAIL ([i915#2842]) -> > PASS > * igt@gem_exec_whisper@basic-forked:shard-glk: DMESG-WARN ([i915#118]) > -> PASS > * igt@gem_mmap_wc@set-cache-level:{shard-rkl}: SKIP ([i915#1850]) -> > PASS > * igt@i915_pm_dc@dc9-dpms:shard-iclb: SKIP ([i915#4281]) -> PASS > * igt@i915_pm_rpm@modeset-lpsp:{shard-rkl}: (PASS, SKIP) ([i915#1397]) > -> PASS > * igt@i915_pm_rpm@modeset-lpsp-stress:{shard-rkl}: SKIP ([i915#1397]) > -> PASS > * igt@i915_selftest@live@hangcheck:shard-snb: INCOMPLETE ([i915#3921]) > -> PASS > * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0:{shard-rkl}: > (SKIP, SKIP) ([i915#1845]) -> PASS +3 similar issues > * igt@kms_ccs@pipe-a-bad-rotation-90-y_tiled_gen12_rc_ccs_cc:{shard- > rkl}: (SKIP, SKIP) ([i915#1845] / [i915#4098]) -> PASS > * igt@kms_ccs@pipe-b-crc-primary-rotation-180- > y_tiled_gen12_rc_ccs_cc:{shard-rkl}: SKIP ([i915#1845] / > [i915#4098]) -> PASS +2 similar issues > * igt@kms_color@pipe-b-ctm-0-25:{shard-rkl}: (SKIP, PASS) ([i915#1149] > / [i915#4098]) -> PASS > * igt@kms_concurrent@pipe-b:{shard-rkl}: SKIP ([i915#1845] / > [i915#4070]) -> PASS > * igt@kms_cursor_crc@pipe-b-cursor-256x85-random:{shard-rkl}: SKIP > ([fdo#112022] / [i915#4070]) -> PASS +6 similar issues > * igt@kms_cursor_crc@pipe-b-cursor-alpha-transparent:{shard-rkl}: > (SKIP, SKIP) ([fdo#112022] / [i915#4070]) -> PASS +2 similar issues > * igt@kms_cursor_edge_walk@pipe-a-128x128-left-edge:{shard-rkl}: SKIP > ([i915#1849] / [i915#4070]) -> PASS +3 similar issues > * igt@kms_cursor_edge_walk@pipe-b-256x256-right-edge:{shard-rkl}: > (SKIP, [SKIP][145]) ([i915#1849] / [i915#4070] / [i915#4098]) -> > [PASS][146]
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index e03e362d320b..78c4cbe82031 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -267,12 +267,6 @@ void __i915_gem_object_pages_fini(struct drm_i915_gem_object *obj) if (!list_empty(&obj->vma.list)) { struct i915_vma *vma; - /* - * Note that the vma keeps an object reference while - * it is active, so it *should* not sleep while we - * destroy it. Our debug code errs insits it *might*. - * For the moment, play along. - */ spin_lock(&obj->vma.lock); while ((vma = list_first_entry_or_null(&obj->vma.list, struct i915_vma, @@ -280,13 +274,7 @@ void __i915_gem_object_pages_fini(struct drm_i915_gem_object *obj) GEM_BUG_ON(vma->obj != obj); spin_unlock(&obj->vma.lock); - /* Verify that the vma is unbound under the vm mutex. */ - mutex_lock(&vma->vm->mutex); - atomic_and(~I915_VMA_PIN_MASK, &vma->flags); - __i915_vma_unbind(vma); - mutex_unlock(&vma->vm->mutex); - - __i915_vma_put(vma); + i915_vma_destroy(vma); spin_lock(&obj->vma.lock); } diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c index ba29767348be..af36bffd064b 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c @@ -167,7 +167,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj, out: i915_gem_object_lock(obj, NULL); - __i915_vma_put(vma); + i915_vma_destroy(vma); i915_gem_object_unlock(obj); return err; } @@ -264,7 +264,7 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj, return err; i915_gem_object_lock(obj, NULL); - __i915_vma_put(vma); + i915_vma_destroy(vma); i915_gem_object_unlock(obj); if (igt_timeout(end_time, diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index df23ebdfc994..4363848f7411 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -105,14 +105,19 @@ void __i915_vm_close(struct i915_address_space *vm) list_for_each_entry_safe(vma, vn, &vm->bound_list, vm_link) { struct drm_i915_gem_object *obj = vma->obj; - /* Keep the obj (and hence the vma) alive as _we_ destroy it */ - if (!kref_get_unless_zero(&obj->base.refcount)) + if (!kref_get_unless_zero(&obj->base.refcount)) { + /* + * Unbind the dying vma to ensure the bound_list + * is completely drained. We leave the destruction to + * the object destructor. + */ + atomic_and(~I915_VMA_PIN_MASK, &vma->flags); + WARN_ON(__i915_vma_unbind(vma)); continue; + } - atomic_and(~I915_VMA_PIN_MASK, &vma->flags); - WARN_ON(__i915_vma_unbind(vma)); - __i915_vma_put(vma); - + /* Keep the obj (and hence the vma) alive as _we_ destroy it */ + i915_vma_destroy_locked(vma); i915_gem_object_put(obj); } GEM_BUG_ON(!list_empty(&vm->bound_list)); diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 52f43a465440..9c1582a473c6 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -1562,15 +1562,27 @@ void i915_vma_reopen(struct i915_vma *vma) void i915_vma_release(struct kref *ref) { struct i915_vma *vma = container_of(ref, typeof(*vma), ref); + + i915_vm_put(vma->vm); + i915_active_fini(&vma->active); + GEM_WARN_ON(vma->resource); + i915_vma_free(vma); +} + +static void force_unbind(struct i915_vma *vma) +{ + if (!drm_mm_node_allocated(&vma->node)) + return; + + atomic_and(~I915_VMA_PIN_MASK, &vma->flags); + WARN_ON(__i915_vma_unbind(vma)); + GEM_BUG_ON(drm_mm_node_allocated(&vma->node)); +} + +static void release_references(struct i915_vma *vma) +{ struct drm_i915_gem_object *obj = vma->obj; - if (drm_mm_node_allocated(&vma->node)) { - mutex_lock(&vma->vm->mutex); - atomic_and(~I915_VMA_PIN_MASK, &vma->flags); - WARN_ON(__i915_vma_unbind(vma)); - mutex_unlock(&vma->vm->mutex); - GEM_BUG_ON(drm_mm_node_allocated(&vma->node)); - } GEM_BUG_ON(i915_vma_is_active(vma)); spin_lock(&obj->vma.lock); @@ -1580,11 +1592,49 @@ void i915_vma_release(struct kref *ref) spin_unlock(&obj->vma.lock); __i915_vma_remove_closed(vma); - i915_vm_put(vma->vm); - i915_active_fini(&vma->active); - GEM_WARN_ON(vma->resource); - i915_vma_free(vma); + __i915_vma_put(vma); +} + +/** + * i915_vma_destroy_locked - Remove all weak reference to the vma and put + * the initial reference. + * + * This function should be called when it's decided the vma isn't needed + * anymore. The caller must assure that it doesn't race with another lookup + * plus destroy, typically by taking an appropriate reference. + * + * Current callsites are + * - __i915_gem_object_pages_fini() + * - __i915_vm_close() - Blocks the above function by taking a reference on + * the object. + * - __i915_vma_parked() - Blocks the above functions by taking an open-count on + * the vm and a reference on the object. + * + * Because of locks taken during destruction, a vma is also guaranteed to + * stay alive while the following locks are held if it was looked up while + * holding one of the locks: + * - vm->mutex + * - obj->vma.lock + * - gt->closed_lock + * + * A vma user can also temporarily keep the vma alive while holding a vma + * reference. + */ +void i915_vma_destroy_locked(struct i915_vma *vma) +{ + lockdep_assert_held(&vma->vm->mutex); + + force_unbind(vma); + release_references(vma); +} + +void i915_vma_destroy(struct i915_vma *vma) +{ + mutex_lock(&vma->vm->mutex); + force_unbind(vma); + mutex_unlock(&vma->vm->mutex); + release_references(vma); } void i915_vma_parked(struct intel_gt *gt) @@ -1618,7 +1668,7 @@ void i915_vma_parked(struct intel_gt *gt) if (i915_gem_object_trylock(obj, NULL)) { INIT_LIST_HEAD(&vma->closed_link); - __i915_vma_put(vma); + i915_vma_destroy(vma); i915_gem_object_unlock(obj); } else { /* back you go.. */ diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h index 011af044ad4f..67ae7341c7e0 100644 --- a/drivers/gpu/drm/i915/i915_vma.h +++ b/drivers/gpu/drm/i915/i915_vma.h @@ -236,6 +236,9 @@ static inline void __i915_vma_put(struct i915_vma *vma) kref_put(&vma->ref, i915_vma_release); } +void i915_vma_destroy_locked(struct i915_vma *vma); +void i915_vma_destroy(struct i915_vma *vma); + #define assert_vma_held(vma) dma_resv_assert_held((vma)->obj->base.resv) static inline void i915_vma_lock(struct i915_vma *vma)