* [PATCH] drm/mst: use kref_get_unless_zero for looking up mst branch device @ 2014-11-10 6:41 Dave Airlie 2014-11-10 8:34 ` [Intel-gfx] " Daniel Vetter 2014-11-10 14:40 ` [PATCH] drm/mst: use kref_get_unless_zero for looking shuang.he 0 siblings, 2 replies; 3+ messages in thread From: Dave Airlie @ 2014-11-10 6:41 UTC (permalink / raw) To: dri-devel, intel-gfx From: Dave Airlie <airlied@redhat.com> The backtrace below shows that while we tear down a branch device, when we remove the connector, we can attempt to disable MST on the port we are tearing down, since the port is going away, there it has a refcount of 0 already, so the validated ref lookup is trying to kref_get again and failing. Use kref_get_unless_zero and just don't return anything if we are looking up the object we are currently also cleaning. [<ffffffffa00cc262>] drm_dp_mst_get_validated_mstb_ref_locked+0x92/0xa0 [drm_kms_helper] [<ffffffffa00cc211>] drm_dp_mst_get_validated_mstb_ref_locked+0x41/0xa0 [drm_kms_helper] [<ffffffffa00cc2aa>] drm_dp_get_validated_mstb_ref+0x3a/0x60 [drm_kms_helper] [<ffffffffa00cc2fb>] drm_dp_payload_send_msg.isra.14+0x2b/0x100 [drm_kms_helper] [<ffffffffa00cc547>] drm_dp_update_payload_part1+0x177/0x360 [drm_kms_helper] [<ffffffffa015c52e>] intel_mst_disable_dp+0x3e/0x80 [i915] [<ffffffffa013d60b>] haswell_crtc_disable+0x1cb/0x340 [i915] [<ffffffffa0136739>] intel_crtc_control+0x49/0x100 [i915] [<ffffffffa0136857>] intel_crtc_update_dpms+0x67/0x80 [i915] [<ffffffffa013fa59>] intel_connector_dpms+0x59/0x70 [i915] [<ffffffffa015c752>] intel_dp_destroy_mst_connector+0x32/0xc0 [i915] [<ffffffffa00cb44b>] drm_dp_destroy_port+0x6b/0xa0 [drm_kms_helper] [<ffffffffa00cb588>] drm_dp_destroy_mst_branch_device+0x108/0x130 [drm_kms_helper] [<ffffffffa00cb3cd>] drm_dp_port_teardown_pdt+0x3d/0x50 [drm_kms_helper] [<ffffffffa00cdb79>] drm_dp_mst_handle_up_req+0x499/0x540 [drm_kms_helper] [<ffffffff810d9ead>] ? trace_hardirqs_on_caller+0x15d/0x200 [<ffffffffa00cdc73>] drm_dp_mst_hpd_irq+0x53/0xa00 [drm_kms_helper] [<ffffffffa00c7dfb>] ? drm_dp_dpcd_read+0x1b/0x20 [drm_kms_helper] [<ffffffffa0153ed8>] ? intel_dp_dpcd_read_wake+0x38/0x70 [i915] [<ffffffffa015a225>] intel_dp_check_mst_status+0xb5/0x250 [i915] [<ffffffffa015ac71>] intel_dp_hpd_pulse+0x181/0x210 [i915] [<ffffffffa01104f6>] i915_digport_work_func+0x96/0x120 [i915] Signed-off-by: Dave Airlie <airlied@redhat.com> --- drivers/gpu/drm/drm_dp_mst_topology.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index ce1113c..f703a5b 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -882,8 +882,10 @@ static struct drm_dp_mst_branch *drm_dp_mst_get_validated_mstb_ref_locked(struct struct drm_dp_mst_port *port; struct drm_dp_mst_branch *rmstb; if (to_find == mstb) { - kref_get(&mstb->kref); - return mstb; + if (!kref_get_unless_zero(&mstb->kref)) + return NULL; + else + return mstb; } list_for_each_entry(port, &mstb->ports, next) { if (port->mstb) { -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/mst: use kref_get_unless_zero for looking up mst branch device 2014-11-10 6:41 [PATCH] drm/mst: use kref_get_unless_zero for looking up mst branch device Dave Airlie @ 2014-11-10 8:34 ` Daniel Vetter 2014-11-10 14:40 ` [PATCH] drm/mst: use kref_get_unless_zero for looking shuang.he 1 sibling, 0 replies; 3+ messages in thread From: Daniel Vetter @ 2014-11-10 8:34 UTC (permalink / raw) To: Dave Airlie; +Cc: intel-gfx, dri-devel On Mon, Nov 10, 2014 at 7:41 AM, Dave Airlie <airlied@gmail.com> wrote: > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index ce1113c..f703a5b 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -882,8 +882,10 @@ static struct drm_dp_mst_branch *drm_dp_mst_get_validated_mstb_ref_locked(struct > struct drm_dp_mst_port *port; > struct drm_dp_mst_branch *rmstb; > if (to_find == mstb) { > - kref_get(&mstb->kref); > - return mstb; > + if (!kref_get_unless_zero(&mstb->kref)) > + return NULL; > + else > + return mstb; > } > list_for_each_entry(port, &mstb->ports, next) { > if (port->mstb) { kref_get_unless_zero only works with the lookup structure is protected through some locking. But the list removal in drm_dp_destroy_mst_branch_device is outside the mgr->glock, so that needs to be moved I think. That's not directly required by kref_get_unless_zero, just another effect of the ports list not holding a full reference. Aside for paranoia I'd put a mutex check next to the kref_get_unles_zero, just to document which lock protects this weak reference. And even more tangential: We should probably flatten the recursion, or add a comment stating that the dp spec limits recursion sufficiently. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/mst: use kref_get_unless_zero for looking 2014-11-10 6:41 [PATCH] drm/mst: use kref_get_unless_zero for looking up mst branch device Dave Airlie 2014-11-10 8:34 ` [Intel-gfx] " Daniel Vetter @ 2014-11-10 14:40 ` shuang.he 1 sibling, 0 replies; 3+ messages in thread From: shuang.he @ 2014-11-10 14:40 UTC (permalink / raw) To: shuang.he, intel-gfx, airlied Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate BYT: pass/total=249/348->248/348 PNV: pass/total=323/328->325/328 ILK: pass/total=328/330->330/330 IVB: pass/total=545/546->545/546 SNB: pass/total=551/556->550/556 HSW: pass/total=577/581->570/581 BDW: pass/total=435/435->434/435 -------------------------------------Detailed------------------------------------- test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)... BYT: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc-lut, PASS(4, M36) -> NSPT(1, M36)PASS(3, M36) PNV: Intel_gpu_tools, igt_gem_concurrent_blit_gpu-bcs-early-read, PASS(6, M23M24) -> DMESG_WARN(1, M24)PASS(3, M24) PNV: Intel_gpu_tools, igt_gem_linear_blits_normal, NSPT(1, M23)PASS(3, M24) -> PASS(4, M24) PNV: Intel_gpu_tools, igt_gem_mmap_offset_exhaustion, DMESG_WARN(2, M23M24)PASS(2, M24) -> PASS(4, M24) PNV: Intel_gpu_tools, igt_gem_unref_active_buffers, DMESG_WARN(1, M23)PASS(3, M24) -> PASS(4, M24) ILK: Intel_gpu_tools, igt_kms_render_gpu-blit, DMESG_WARN(1, M26)PASS(3, M6) -> PASS(4, M6) ILK: Intel_gpu_tools, igt_kms_flip_wf_vblank-vs-dpms-interruptible, DMESG_WARN(1, M26)PASS(3, M6) -> PASS(4, M6) IVB: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(1, M4)PASS(3, M34M4) -> NSPT(2, M4)PASS(2, M4) IVB: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-A-plane-1, TIMEOUT(1, M34)PASS(3, M4) -> PASS(4, M4) SNB: Intel_gpu_tools, igt_gem_concurrent_blit_gpuX-bcs-overwrite-source-forked, PASS(4, M35M22) -> FAIL(1, M22)PASS(3, M22) HSW: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(1, M39)PASS(3, M40M39) -> NSPT(2, M39)PASS(2, M39) HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-128x128-onscreen, PASS(4, M40M39) -> DMESG_WARN(2, M39)PASS(2, M39) HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-offscreen, PASS(4, M40M39) -> DMESG_WARN(2, M39)PASS(2, M39) HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-offscreen, DMESG_WARN(1, M39)PASS(3, M40M39) -> DMESG_WARN(2, M39)PASS(2, M39) HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-onscreen, DMESG_WARN(1, M39)PASS(3, M40M39) -> DMESG_WARN(1, M39)PASS(3, M39) HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-sliding, DMESG_WARN(1, M39)PASS(3, M40M39) -> DMESG_WARN(1, M39)PASS(3, M39) HSW: Intel_gpu_tools, igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip, PASS(4, M40M39) -> DMESG_WARN(1, M39)PASS(3, M39) BDW: Intel_gpu_tools, igt_gem_reset_stats_ban-bsd, PASS(4, M30) -> DMESG_WARN(1, M30)PASS(3, M30) _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-10 14:40 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-10 6:41 [PATCH] drm/mst: use kref_get_unless_zero for looking up mst branch device Dave Airlie 2014-11-10 8:34 ` [Intel-gfx] " Daniel Vetter 2014-11-10 14:40 ` [PATCH] drm/mst: use kref_get_unless_zero for looking shuang.he
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.