[Intel-gfx] [PATCH 2/5] drm/i915: destroy connector sysfs files earlier
Daniel Vetter
daniel at ffwll.ch
Mon Sep 30 23:10:44 CEST 2013
On Thu, Sep 26, 2013 at 08:05:59PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>
> For some reason, every single time I try to run module_reload
> something tries to read the connector sysfs files. This happens
> after we destroy the encoders and before we destroy the connectors, so
> when the sysfs read triggers the connector detect() function,
> intel_conector->encoder points to memory that was already freed.
>
> The bad backtrace is just:
> [<ffffffff8163ca9a>] dump_stack+0x54/0x74
> [<ffffffffa00c2c8e>] intel_dp_detect+0x1e/0x4b0 [i915]
> [<ffffffffa001913d>] status_show+0x3d/0x80 [drm]
> [<ffffffff813d5340>] dev_attr_show+0x20/0x60
> [<ffffffff81221f50>] ? sysfs_read_file+0x80/0x1b0
> [<ffffffff81221f79>] sysfs_read_file+0xa9/0x1b0
> [<ffffffff811aaf1e>] vfs_read+0x9e/0x170
> [<ffffffff811aba4c>] SyS_read+0x4c/0xa0
> [<ffffffff8164e392>] system_call_fastpath+0x16/0x1b
>
> But if you add tons of memory checking debug options to your Kernel
> you'll also see:
> - general protection fault: 0000
> - BUG kmalloc-4096 (Tainted: G D W ): Poison overwritten
> - INFO: Allocated in intel_ddi_init+0x65/0x270 [i915]
> - INFO: Freed in intel_dp_encoder_destroy+0x69/0xb0 [i915]
> Among a bunch of other error messages.
>
> So this commit just destroys the sysfs files before both the encoder
> and connectors are freed.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
Queued for -next, thanks for the patch. I'm unsure what to do with the
first patch, maybe Ville can take a look. For the next three patches to
wire up bind/unbind hooks I think the right approach would be to just
disallow the vgaconsole once drm/i915 is loaded:
- We don't want to even tighter integrate with the locking madness that is
fbcon.
- We want to kick out the vgacon anyway, even when e.g. fbcon isn't
loaded.
David Herrmann might have an idea how to solve this.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_crt.c | 1 -
> drivers/gpu/drm/i915/intel_display.c | 5 +++++
> drivers/gpu/drm/i915/intel_dp.c | 1 -
> drivers/gpu/drm/i915/intel_dsi.c | 1 -
> drivers/gpu/drm/i915/intel_dvo.c | 1 -
> drivers/gpu/drm/i915/intel_hdmi.c | 1 -
> drivers/gpu/drm/i915/intel_lvds.c | 1 -
> drivers/gpu/drm/i915/intel_sdvo.c | 7 +++++--
> drivers/gpu/drm/i915/intel_tv.c | 1 -
> 9 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 0263629..942b9ac 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -677,7 +677,6 @@ intel_crt_detect(struct drm_connector *connector, bool force)
>
> static void intel_crt_destroy(struct drm_connector *connector)
> {
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 065ffed..793061f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10726,6 +10726,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *crtc;
> + struct drm_connector *connector;
>
> /*
> * Interrupts and polling as the first thing to avoid creating havoc.
> @@ -10768,6 +10769,10 @@ void intel_modeset_cleanup(struct drm_device *dev)
> /* destroy backlight, if any, before the connectors */
> intel_panel_destroy_backlight(dev);
>
> + /* destroy the sysfs files before encoders/connectors */
> + list_for_each_entry(connector, &dev->mode_config.connector_list, head)
> + drm_sysfs_connector_remove(connector);
> +
> drm_mode_config_cleanup(dev);
>
> intel_cleanup_overlay(dev);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 95a3159..7bdbb36 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3139,7 +3139,6 @@ intel_dp_connector_destroy(struct drm_connector *connector)
> if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> intel_panel_fini(&intel_connector->panel);
>
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 674fd49..9a2fdd2 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -504,7 +504,6 @@ static void intel_dsi_destroy(struct drm_connector *connector)
>
> DRM_DEBUG_KMS("\n");
> intel_panel_fini(&intel_connector->panel);
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 91287d1..1b64145 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -367,7 +367,6 @@ static int intel_dvo_get_modes(struct drm_connector *connector)
>
> static void intel_dvo_destroy(struct drm_connector *connector)
> {
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 6004f9c..4f4d346 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1181,7 +1181,6 @@ static void intel_hdmi_post_disable(struct intel_encoder *encoder)
>
> static void intel_hdmi_destroy(struct drm_connector *connector)
> {
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index fb3f8ef..ae0c843 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -474,7 +474,6 @@ static void intel_lvds_destroy(struct drm_connector *connector)
>
> intel_panel_fini(&lvds_connector->base.panel);
>
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 5e59d64..a583e8f 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2009,7 +2009,6 @@ static void intel_sdvo_destroy(struct drm_connector *connector)
> intel_sdvo_connector->tv_format);
>
> intel_sdvo_destroy_enhance_property(connector);
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(intel_sdvo_connector);
> }
> @@ -2482,6 +2481,7 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
> return true;
>
> err:
> + drm_sysfs_connector_remove(connector);
> intel_sdvo_destroy(connector);
> return false;
> }
> @@ -2553,6 +2553,7 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
> return true;
>
> err:
> + drm_sysfs_connector_remove(connector);
> intel_sdvo_destroy(connector);
> return false;
> }
> @@ -2624,8 +2625,10 @@ static void intel_sdvo_output_cleanup(struct intel_sdvo *intel_sdvo)
>
> list_for_each_entry_safe(connector, tmp,
> &dev->mode_config.connector_list, head) {
> - if (intel_attached_encoder(connector) == &intel_sdvo->base)
> + if (intel_attached_encoder(connector) == &intel_sdvo->base) {
> + drm_sysfs_connector_remove(connector);
> intel_sdvo_destroy(connector);
> + }
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 75925a1..92895f9 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1433,7 +1433,6 @@ intel_tv_get_modes(struct drm_connector *connector)
> static void
> intel_tv_destroy(struct drm_connector *connector)
> {
> - drm_sysfs_connector_remove(connector);
> drm_connector_cleanup(connector);
> kfree(connector);
> }
> --
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list