| Message ID | 20170513105201.17658-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, May 13, 2017 at 11:52:00AM +0100, Chris Wilson wrote: > Both as an exercise to document that we are reading the state outside of > the appropriate mutex and to ensure that we only read the value once > before the multiple comparisons, use READ_ONCE. I think gcc could also opt to re-evalute and re-load it, resulting to diverging control flow and hilarity. At least I don't spot any protection against that in the wait_for macros. Applied to drm-misc, thanks. -Daniel > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index d1cbb9c8f806..3bdd314f02b1 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -737,16 +737,16 @@ static void drm_dp_mst_put_payload_id(struct drm_dp_mst_topology_mgr *mgr, > static bool check_txmsg_state(struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_sideband_msg_tx *txmsg) > { > - bool ret; > + unsigned int state; > > /* > * All updates to txmsg->state are protected by mgr->qlock, and the two > * cases we check here are terminal states. For those the barriers > * provided by the wake_up/wait_event pair are enough. > */ > - ret = (txmsg->state == DRM_DP_SIDEBAND_TX_RX || > - txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT); > - return ret; > + state = READ_ONCE(txmsg->state); > + return (state == DRM_DP_SIDEBAND_TX_RX || > + state == DRM_DP_SIDEBAND_TX_TIMEOUT); > } > > static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb, > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index d1cbb9c8f806..3bdd314f02b1 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -737,16 +737,16 @@ static void drm_dp_mst_put_payload_id(struct drm_dp_mst_topology_mgr *mgr, static bool check_txmsg_state(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_sideband_msg_tx *txmsg) { - bool ret; + unsigned int state; /* * All updates to txmsg->state are protected by mgr->qlock, and the two * cases we check here are terminal states. For those the barriers * provided by the wake_up/wait_event pair are enough. */ - ret = (txmsg->state == DRM_DP_SIDEBAND_TX_RX || - txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT); - return ret; + state = READ_ONCE(txmsg->state); + return (state == DRM_DP_SIDEBAND_TX_RX || + state == DRM_DP_SIDEBAND_TX_TIMEOUT); } static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
Both as an exercise to document that we are reading the state outside of the appropriate mutex and to ensure that we only read the value once before the multiple comparisons, use READ_ONCE. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)