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

Cheng, Yao
2014-12-18T05:44:37Z

> -----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.).

> > I don't understand why this happens but I believe what Daniel said:
> > "grabbing a module refcount for the platform device doesn't work (it
> > would pin the module forever)"
>
> What I'd expect to happen is this:
>
> # modprobe i915
> i915 registers a platform devices
> # modprobe ipvr
> driver core probes ipvr device
> # modprobe -r i915
> i915 removes the platform device (ipvr's ->remove() is called)
>
> I guess if you don't do anything else, then indeed the ipvr module will stay
> around, but the above should work idempotently, that is you should be able
> to repeat it an unlimited number of times and nothing should break.
>
> In fact you should be able to run the following in any permutation without
> causing a crash:
>
> # modprobe i915
> # modprobe ipvr
> # modprobe -r ipvr
> # modprobe -r i915
>
> If any permutation results in a crash you have a bug.

I assume all the permutations will work after fixing the deadlock.

>
> Thierry