Re: [PATCH 2/3] drm/i915: Fix fbdev unload sequence

From: Chris Wilson <chris@chris-wilson.co.uk>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	martin.peres@free.fr, Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH 2/3] drm/i915: Fix fbdev unload sequence
Date: Fri, 14 Jul 2017 20:30:34 +0100	[thread overview]
Message-ID: <150006063435.13756.15570851134809686071@mail.alporthouse.com> (raw)
In-Reply-To: <20170714191439.31169-2-daniel.vetter@ffwll.ch>

Quoting Daniel Vetter (2017-07-14 20:14:38)
> First thing we need to do is unregister the fbdev instance, but we
> can't just go ahead and kfree it. That must wait until the hotplug and
> polling work are stopped, since they can race with the with the
> teardown. That means we need to split up the fbdev teardown into the
> unregister part and the cleanup part.
> 
> I originally suspected that this was broken in one of the unload
> shuffles, but on closer inspection the oldest sequence I've dug out
> also gets this wrong. Just not quite so badly.
> 
> I've run drv_module_reload a few hundred times and it's rock solid
> compared to insta-death beforehand. This bug seems to have been
> uncovered by
> 
> commit 88be58be886f1215cc73dc8c273c985eecd7385c
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Thu Jul 6 15:00:19 2017 +0200
> 
>     drm/i915/fbdev: Always forward hotplug events
> 
> But the effect of that seems to only be to increase the race window
> enough to make it blow up easier. I'm not exactly clear on what's
> going on there ...
> 
> Testcase: igt/drv_module_reload
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101791
> Cc: martin.peres@free.fr
> Cc: chris@chris-wilson.co.uk
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Splitting out the unregister and running it early makes sense and is how
we expect to unload to proceed.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Nit:

> @@ -1383,7 +1383,6 @@ void i915_driver_unload(struct drm_device *dev)
>         intel_gvt_cleanup(dev_priv);
>  
>         i915_driver_unregister(dev_priv);
> -
>         intel_modeset_cleanup(dev);

I think it will useful to maintain the visual break here. (I know next
patch moves it again...)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx