Re: [RFC PATCH v3 4/4] tests/drv_module_reload: add ipvr support

Cheng, Yao
2014-12-21T14:40:24Z

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch]
> Sent: Thursday, December 18, 2014 19:21
> To: Thierry Reding
> Cc: Cheng, Yao; intel-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org; Kelley, Sean V; Chehab, John;
> emil.l.velikov@gmail.com; Jiang, Fei
> Subject: Re: [RFC PATCH v3 4/4] tests/drv_module_reload: add ipvr support
>
> On Thu, Dec 18, 2014 at 11:04 AM, Thierry Reding <thierry.reding@gmail.com>
> wrote:
> >> I double checked the symptom and found it was a deadlock on
> drm_global_mutex.
> >> When i915_driver_load() registers the platform device while ipvr module
> is in the system, ipvr's probe() function tries to lock drm_global_mutex which
> was already held by i915.
> >> I think either of the following 2 actions need to be moved to a bottom half
> e.g. a work queue:
> >> platform_device_add () call in i915_ved.c (called during
> i915_driver_load())
> >> drm_dev_register() call during ipvr's probe() Which one makes
> >> more sense? pls kindly advise (I personally prefer the former one.).
> >
> > Yes, that's somewhat ugly, but I don't see a way around that. I'd also
> > think that moving platform_device_add() to a workqueue would be the
> > best option here.
>
> Or we simply kill drm_global_mutex for platform drivers that don't use the -
> >probe hook. It should work when they have a correct order betwen
> drm_dev_alloc and _register and all the code in between. So just ditch the -
> >load callback in teh ipvr driver and rework the load sequence as suggested
> somewhere else and this is fixed already. No need for bottom halfs I think.

Daniel, sorry I didn't quite understand "platform drivers that don't use the probe hook". For initialization, the ipvr platform driver's probe() is called in following 2 possible paths:
1. ipvr installed before i915. In this case, ipvr's probe() is called inside i915_driver_load() and falls into the drm_global_mutex dead lock.
2. i915 installed before ipvr. In this case, ipvr's probe() is called without drm_global_mutex held by i915 and no dead lock issue.
If we kill drm_global_mutex, will path 2 run into issue? And in your suggestion, how to rework the load sequence? Do you mean calling ipvr's load() callback directly during platform driver probe()?

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch