[Intel-gfx] [RFC PATCH v3 4/4] tests/drv_module_reload: add ipvr support

[Cheng, Yao]

> -----Original Message-----
> From: Thierry Reding [mailto:thierry.reding at gmail.com]
> Sent: Wednesday, December 17, 2014 16:13
> To: Cheng, Yao
> Cc: Daniel Vetter; intel-gfx at lists.freedesktop.org; dri-
> devel at lists.freedesktop.org; daniel.vetter at ffwll.ch; Kelley, Sean V; Chehab,
> John; emil.l.velikov at 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