[Intel-gfx] [PATCH] drm/mst: use kref_get_unless_zero for looking up mst branch device

[Daniel Vetter]

On Mon, Nov 10, 2014 at 7:41 AM, Dave Airlie <airlied at 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