On Wed, Nov 19, 2014 at 12:38 PM, Daniel Vetter daniel.vetter@ffwll.ch wrote:
Yet another fallout from not considering DP MST hotplug. With the previous patches we have stable indices, but it might still happen that a connector gets added between when we allocate the array and when we actually add a connector. Especially when we back off due to ww mutex contention or similar issues.
So store the sizes of the arrays in struct drm_atomic_state and double check them. We don't really care about races except that we want to use a consistent value, so ACCESS_ONCE is all we need. And if we indeed notice that we'd overrun the array then just give up and restart the entire ioctl.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Reviewed-by: Rob Clark robdclark@gmail.com
drivers/gpu/drm/drm_atomic.c | 26 +++++++++++++++++++++----- drivers/gpu/drm/drm_atomic_helper.c | 23 ++++++++--------------- include/drm/drm_crtc.h | 2 ++ 3 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 67c1dc894bd9..3624632084e2 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -56,6 +56,8 @@ drm_atomic_state_alloc(struct drm_device *dev) if (!state) return NULL;
state->num_connector = ACCESS_ONCE(dev->mode_config.num_connector);state->crtcs = kcalloc(dev->mode_config.num_crtc, sizeof(*state->crtcs), GFP_KERNEL); if (!state->crtcs)@@ -72,12 +74,12 @@ drm_atomic_state_alloc(struct drm_device *dev) sizeof(*state->plane_states), GFP_KERNEL); if (!state->plane_states) goto fail;
state->connectors = kcalloc(dev->mode_config.num_connector,
state->connectors = kcalloc(state->num_connector, sizeof(*state->connectors), GFP_KERNEL); if (!state->connectors) goto fail;
state->connector_states = kcalloc(dev->mode_config.num_connector,
state->connector_states = kcalloc(state->num_connector, sizeof(*state->connector_states), GFP_KERNEL); if (!state->connector_states)@@ -117,7 +119,7 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)
DRM_DEBUG_KMS("Clearing atomic state %p\n", state);
for (i = 0; i < config->num_connector; i++) {
for (i = 0; i < state->num_connector; i++) { struct drm_connector *connector = state->connectors[i]; if (!connector)@@ -304,6 +306,21 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
index = drm_connector_index(connector);
/** Construction of atomic state updates can race with a connector* hot-add which might overflow. In this case flip the table and just* restart the entire ioctl - no one is fast enough to livelock a cpu* with physical hotplug events anyway.** Note that we only grab the indexes once we have the right lock to* prevent hotplug/unplugging of connectors. So removal is no problem,* at most the array is a bit too large.*/if (index >= state->num_connector) {DRM_DEBUG_KMS("Hot-added connector would overflow state array, restarting\n");return -EAGAIN;}if (state->connector_states[index]) return state->connector_states[index];@@ -499,10 +516,9 @@ int drm_atomic_connectors_for_crtc(struct drm_atomic_state *state, struct drm_crtc *crtc) {
int nconnectors = state->dev->mode_config.num_connector; int i, num_connected_connectors = 0;for (i = 0; i < nconnectors; i++) {
for (i = 0; i < state->num_connector; i++) { struct drm_connector_state *conn_state; conn_state = state->connector_states[i];diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 0cd054615920..99095ef147ef 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -249,7 +249,6 @@ static int mode_fixup(struct drm_atomic_state *state) { int ncrtcs = state->dev->mode_config.num_crtc;
int nconnectors = state->dev->mode_config.num_connector; struct drm_crtc_state *crtc_state; struct drm_connector_state *conn_state; int i;@@ -264,7 +263,7 @@ mode_fixup(struct drm_atomic_state *state) drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode); }
for (i = 0; i < nconnectors; i++) {
for (i = 0; i < state->num_connector; i++) { struct drm_encoder_helper_funcs *funcs; struct drm_encoder *encoder;@@ -336,7 +335,6 @@ drm_atomic_helper_check_prepare(struct drm_device *dev, struct drm_atomic_state *state) { int ncrtcs = dev->mode_config.num_crtc;
int nconnectors = dev->mode_config.num_connector; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; int i, ret;@@ -361,7 +359,7 @@ drm_atomic_helper_check_prepare(struct drm_device *dev, } }
for (i = 0; i < nconnectors; i++) {
for (i = 0; i < state->num_connector; i++) { /* * This only sets crtc->mode_changed for routing changes, * drivers must set crtc->mode_changed themselves when connector@@ -485,10 +483,9 @@ static void disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) { int ncrtcs = old_state->dev->mode_config.num_crtc;
int nconnectors = old_state->dev->mode_config.num_connector; int i;for (i = 0; i < nconnectors; i++) {
for (i = 0; i < old_state->num_connector; i++) { struct drm_connector_state *old_conn_state; struct drm_connector *connector; struct drm_encoder_helper_funcs *funcs;@@ -553,12 +550,11 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) static void set_routing_links(struct drm_device *dev, struct drm_atomic_state *old_state) {
int nconnectors = dev->mode_config.num_connector; int ncrtcs = old_state->dev->mode_config.num_crtc; int i; /* clear out existing links */for (i = 0; i < nconnectors; i++) {
for (i = 0; i < old_state->num_connector; i++) { struct drm_connector *connector; connector = old_state->connectors[i];@@ -573,7 +569,7 @@ set_routing_links(struct drm_device *dev, struct drm_atomic_state *old_state) }
/* set new links */
for (i = 0; i < nconnectors; i++) {
for (i = 0; i < old_state->num_connector; i++) { struct drm_connector *connector; connector = old_state->connectors[i];@@ -608,7 +604,6 @@ static void crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) { int ncrtcs = old_state->dev->mode_config.num_crtc;
int nconnectors = old_state->dev->mode_config.num_connector; int i; for (i = 0; i < ncrtcs; i++) {@@ -626,7 +621,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) funcs->mode_set_nofb(crtc); }
for (i = 0; i < nconnectors; i++) {
for (i = 0; i < old_state->num_connector; i++) { struct drm_connector *connector; struct drm_crtc_state *new_crtc_state; struct drm_encoder_helper_funcs *funcs;@@ -687,7 +682,6 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev, struct drm_atomic_state *old_state) { int ncrtcs = old_state->dev->mode_config.num_crtc;
int nconnectors = old_state->dev->mode_config.num_connector; int i; for (i = 0; i < ncrtcs; i++) {@@ -706,7 +700,7 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev, funcs->commit(crtc); }
for (i = 0; i < nconnectors; i++) {
for (i = 0; i < old_state->num_connector; i++) { struct drm_connector *connector; struct drm_encoder_helper_funcs *funcs; struct drm_encoder *encoder;@@ -1304,7 +1298,6 @@ static int update_output_state(struct drm_atomic_state *state, { struct drm_device *dev = set->crtc->dev; struct drm_connector_state *conn_state;
int nconnectors = state->dev->mode_config.num_connector; int ncrtcs = state->dev->mode_config.num_crtc; int ret, i, j;@@ -1333,7 +1326,7 @@ static int update_output_state(struct drm_atomic_state *state, }
/* Then recompute connector->crtc links and crtc enabling state. */
for (i = 0; i < nconnectors; i++) {
for (i = 0; i < state->num_connector; i++) { struct drm_connector *connector; connector = state->connectors[i];diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 91c09520aad3..cdbae7bdac70 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -845,6 +845,7 @@ struct drm_bridge {
- @plane_states: pointer to array of plane states pointers
- @crtcs: pointer to array of CRTC pointers
- @crtc_states: pointer to array of CRTC states pointers
- @num_connector: size of the @connectors and @connector_states arrays
- @connectors: pointer to array of connector pointers
- @connector_states: pointer to array of connector states pointers
- @acquire_ctx: acquire context for this atomic modeset state update
@@ -856,6 +857,7 @@ struct drm_atomic_state { struct drm_plane_state **plane_states; struct drm_crtc **crtcs; struct drm_crtc_state **crtc_states;
int num_connector; struct drm_connector **connectors; struct drm_connector_state **connector_states;-- 2.1.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx