linux-kernel - Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing

Open Source and information security mailing list archives

Message-ID: <CAObL_7F+xiQrsdJzaSR3nnWm-=ZiHZf8dE+Q3yv+TQC7PUYjEA@mail.gmail.com>
Date:	Mon, 25 Jul 2011 13:40:58 -0400
From:	Andrew Lutomirski <luto@....edu>
To:	Jesse Barnes <jbarnes@...tuousgeek.org>
Cc:	Keith Packard <keithp@...thp.com>,
	Dave Airlie <airlied@...hat.com>,
	intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	dri-devel@...ts.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing

Will test tonight.

It looks like there is a lot of hotplug activity when 'xset dpms force
off' gets run.  (That's not a typo.  I do mean "off," not "on."

See attached trace.  perf rocks, even over ssh :)

--Andy

On Mon, Jul 25, 2011 at 1:37 PM, Jesse Barnes <jbarnes@...tuousgeek.org> wrote:
> On Mon, 25 Jul 2011 10:10:29 -0700
> Keith Packard <keithp@...thp.com> wrote:
>
>> Hotplug detection is a mode setting operation and must hold the
>> struct_mutex or risk colliding with other mode setting operations.
>>
>> In particular, the display port hotplug function attempts to re-train
>> the link if the monitor is supposed to be running when plugged back
>> in. If that happens while mode setting is underway, the link will get
>> scrambled, leaving it in an inconsistent state.
>>
>> Signed-off-by: Keith Packard <keithp@...thp.com>
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 3b03f85..5fe8f28 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -306,12 +306,15 @@ static void i915_hotplug_work_func(struct work_struct *work)
>>       struct drm_mode_config *mode_config = &dev->mode_config;
>>       struct intel_encoder *encoder;
>>
>> +     mutex_lock(&dev_priv->dev->struct_mutex);
>>       DRM_DEBUG_KMS("running encoder hotplug functions\n");
>>
>>       list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
>>               if (encoder->hot_plug)
>>                       encoder->hot_plug(encoder);
>>
>> +     mutex_unlock(&dev_priv->dev->struct_mutex);
>> +
>>       /* Just fire off a uevent and let userspace tell us what to do */
>>       drm_helper_hpd_irq_event(dev);
>>  }
>
> yay, sounds like this will fix Andrew's problem and probably lots of
> other random DP related failures.
>
> Looks like the ->detect function is similarly protected at the call
> site (though one level up in ->fill_modes), so it should be safe.
> Looks like all the call sites in the link_status function are safe too.
>
> Reviewed-by: Jesse Barnes <jbarnes@...tuousgeek.org>
>
> Let's get this one upstream asap.  Should probably be cc'd to
> stable@...nel.org as well.
>
> --
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@...ts.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

View attachment "trace.txt" of type "text/plain" (15633 bytes)

View attachment "dmesg_2.txt" of type "text/plain" (6523 bytes)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives