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

Thierry Reding
2014-12-18T10:04:27Z

[-- Attachment #1.1: Type: text/plain, Size: 1882 bytes --]

On Thu, Dec 18, 2014 at 05:44:37AM +0000, Cheng, Yao wrote:
> > -----Original Message-----
> > From: Thierry Reding [mailto:thierry.reding@gmail.com]
> > Sent: Wednesday, December 17, 2014 16:13
> > To: Cheng, Yao
> > Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; dri-
> > devel@lists.freedesktop.org; daniel.vetter@ffwll.ch; 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
>
> Thanks Thiery for the suggestion, pls see my inline comments
>
> >
> > > Thierry/Daniel, the actual symptom is, after "rmmod i915", though
> > > drm_drv_release() is also called on the child device "ipvr", I still
> > > see the module exist in the system (check it by "lsmod").
> >
> > Which module? ipvr or i915?
>
> The ipvr module still exist by checking "lsmod" after rmmod i915
>
> >
> > > This causes issue when I modprobe i915 and ipvr again later.
> >
> > What issue are you seeing? If your driver can't deal with a situation where it's
> > probed again after being removed then you have a bug.
> >
>
> 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.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]