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

From: Daniel Vetter <daniel@ffwll.ch>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: daniel.vetter@ffwll.ch, emil.l.velikov@gmail.com,
	dri-devel@lists.freedesktop.org, john.chehab@intel.com,
	fei.jiang@intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [RFC PATCH v3 4/4] tests/drv_module_reload: add ipvr support
Date: Mon, 24 Nov 2014 14:14:48 +0100	[thread overview]
Message-ID: <20141124131448.GP25711@phenom.ffwll.local> (raw)
In-Reply-To: <20141124095544.GA25912@ulmo.nvidia.com>

On Mon, Nov 24, 2014 at 10:55:46AM +0100, Thierry Reding wrote:
> On Fri, Nov 21, 2014 at 09:36:33PM +0100, Daniel Vetter wrote:
> > On Fri, Nov 21, 2014 at 09:27:04PM +0100, Thierry Reding wrote:
> > > On Sat, Nov 22, 2014 at 03:10:01AM +0800, Yao Cheng wrote:
> > > > on vlv, if ipvr is installed, it need be manually unloaded before
> > > > i915, otherwise user might run into use-after-free issue.
> > > 
> > > Huh? That doesn't sound right. What exactly is it that's going wrong?
> > > You should never have to do this. If you do you're almost certainly
> > > doing something wrong in the kernel module.
> > 
> > It's the hilarity called platform devices. Removing them is somewhat racy,
> > so doing that upfront makes the entire thing a bit safer. The use after
> > free is on the text, since grabbing a module refcount for the platform
> > device doesn't work (it would pin the module forever).
> 
> I don't understand what the issue is here. I've used platform devices
> quite extensively on ARM and I've never encountered a situation where
> they were insufficient (or racy for that matter).
> 
> If I understand correctly what this commit tries to achieve, then it
> unloads one module before another module that it depends on so that the
> dependency can be removed subsequently without causing a crash. That
> sounds really brittle to me. How are you going to document this for
> users so that they don't accidentally go and unload the i915 module and
> crash their system?

Module unloading taints your kernel and isn't an end-user supported
feature. That simple ;-)

Also afaik the problem is that you actually can't unload i915 until you've
unloaded the subordinate driver, since i915 registering the platform
driver prevents unload. Or at least that was my understanding, I didn't
test this myself. I just asked whether the unload script still works and
apparently it breaks.

I guess what's different with ARM is that DT creates all the platform
devices, and not modules themselves?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx