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;elsereturn 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