All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: gregkh@linuxfoundation.org
Cc: lyude@redhat.com, Wayne.Lin@amd.com, sean@poorly.run,
	ville.syrjala@linux.intel.com, stable@vger.kernel.org
Subject: Re: FAILED: patch "[PATCH] drm/dp_mst: Fix clearing payload state on topology disable" failed to apply to 5.6-stable tree
Date: Wed, 15 Apr 2020 22:21:17 -0400	[thread overview]
Message-ID: <20200416022117.GX1068@sasha-vm> (raw)
In-Reply-To: <1586950297139145@kroah.com>

On Wed, Apr 15, 2020 at 01:31:37PM +0200, gregkh@linuxfoundation.org wrote:
>
>The patch below does not apply to the 5.6-stable tree.
>If someone wants it applied there, or to any other stable or longterm
>tree, then please email the backport, including the original git commit
>id to <stable@vger.kernel.org>.
>
>thanks,
>
>greg k-h
>
>------------------ original commit in Linus's tree ------------------
>
>From 8732fe46b20c951493bfc4dba0ad08efdf41de81 Mon Sep 17 00:00:00 2001
>From: Lyude Paul <lyude@redhat.com>
>Date: Wed, 22 Jan 2020 14:43:20 -0500
>Subject: [PATCH] drm/dp_mst: Fix clearing payload state on topology disable
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>The issues caused by:
>
>commit 64e62bdf04ab ("drm/dp_mst: Remove VCPI while disabling topology
>mgr")
>
>Prompted me to take a closer look at how we clear the payload state in
>general when disabling the topology, and it turns out there's actually
>two subtle issues here.
>
>The first is that we're not grabbing &mgr.payload_lock when clearing the
>payloads in drm_dp_mst_topology_mgr_set_mst(). Seeing as the canonical
>lock order is &mgr.payload_lock -> &mgr.lock (because we always want
>&mgr.lock to be the inner-most lock so topology validation always
>works), this makes perfect sense. It also means that -technically- there
>could be racing between someone calling
>drm_dp_mst_topology_mgr_set_mst() to disable the topology, along with a
>modeset occurring that's modifying the payload state at the same time.
>
>The second is the more obvious issue that Wayne Lin discovered, that
>we're not clearing proposed_payloads when disabling the topology.
>
>I actually can't see any obvious places where the racing caused by the
>first issue would break something, and it could be that some of our
>higher-level locks already prevent this by happenstance, but better safe
>then sorry. So, let's make it so that drm_dp_mst_topology_mgr_set_mst()
>first grabs &mgr.payload_lock followed by &mgr.lock so that we never
>race when modifying the payload state. Then, we also clear
>proposed_payloads to fix the original issue of enabling a new topology
>with a dirty payload state. This doesn't clear any of the drm_dp_vcpi
>structures, but those are getting destroyed along with the ports anyway.
>
>Changes since v1:
>* Use sizeof(mgr->payloads[0])/sizeof(mgr->proposed_vcpis[0]) instead -
>  vsyrjala
>
>Cc: Sean Paul <sean@poorly.run>
>Cc: Wayne Lin <Wayne.Lin@amd.com>
>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Cc: stable@vger.kernel.org # v4.4+
>Signed-off-by: Lyude Paul <lyude@redhat.com>
>Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Link: https://patchwork.freedesktop.org/patch/msgid/20200122194321.14953-1-lyude@redhat.com

We needed a86675968e23 ("Revert "drm/dp_mst: Remove VCPI while disabling
topology mgr"") too (this is a revert of a stable tagged commit).

-- 
Thanks,
Sasha

      parent reply	other threads:[~2020-04-16  2:21 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-15 11:31 FAILED: patch "[PATCH] drm/dp_mst: Fix clearing payload state on topology disable" failed to apply to 5.6-stable tree gregkh
2020-04-15 20:32 ` [PATCH] drm/dp_mst: Fix clearing payload state on topology disable Lyude Paul
2020-04-15 20:32   ` Lyude Paul
2020-04-16  2:21 ` Sasha Levin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200416022117.GX1068@sasha-vm \
    --to=sashal@kernel.org \
    --cc=Wayne.Lin@amd.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=lyude@redhat.com \
    --cc=sean@poorly.run \
    --cc=stable@vger.kernel.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.