| Message ID | 1466500235-21282-3-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 21, 2016 at 11:10:27AM +0200, Daniel Vetter wrote: > And to avoid surprising userspace, make sure we don't copy more > connectors than planned, and report the actual number of connectors > copied. That way any racing hot-add/remove will be handled. > @@ -1938,6 +1929,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data, > } > copied++; > } > + connector_count = copied; You forgot to actually make sure we don't copy more conectors than planned. -Chris
On Tue, Jun 21, 2016 at 10:48:08AM +0100, Chris Wilson wrote: > On Tue, Jun 21, 2016 at 11:10:27AM +0200, Daniel Vetter wrote: > > And to avoid surprising userspace, make sure we don't copy more > > connectors than planned, and report the actual number of connectors > > copied. That way any racing hot-add/remove will be handled. > > > @@ -1938,6 +1929,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data, > > } > > copied++; > > } > > + connector_count = copied; > > You forgot to actually make sure we don't copy more conectors than > planned. Indeed, must have lost that hunk somewhere. At least I'm sure I've typed it ;-) Will resend. -Daniel
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 28c109ff7330..4f18299dc630 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1842,10 +1842,10 @@ int drm_mode_getresources(struct drm_device *dev, void *data, struct drm_crtc *crtc; struct drm_encoder *encoder; int ret = 0; - int connector_count = 0; - int crtc_count = 0; + int connector_count = READ_ONCE(dev->mode_config.num_connector); + int crtc_count = dev->mode_config.num_crtc; int fb_count = 0; - int encoder_count = 0; + int encoder_count = dev->mode_config.num_encoder; int copied = 0; uint32_t __user *fb_id; uint32_t __user *crtc_id; @@ -1883,15 +1883,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, /* mode_config.mutex protects the connector list against e.g. DP MST * connector hot-adding. CRTC/Plane lists are invariant. */ mutex_lock(&dev->mode_config.mutex); - drm_for_each_crtc(crtc, dev) - crtc_count++; - - drm_for_each_connector(connector, dev) - connector_count++; - - drm_for_each_encoder(encoder, dev) - encoder_count++; - card_res->max_height = dev->mode_config.max_height; card_res->min_height = dev->mode_config.min_height; card_res->max_width = dev->mode_config.max_width; @@ -1938,6 +1929,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data, } copied++; } + connector_count = copied; } card_res->count_connectors = connector_count;
Looping when we keep track of this is silly. Only thing we have to be careful is with sampling the connector count. To avoid inconsisten results due to gcc re-computing this, use READ_ONCE. And to avoid surprising userspace, make sure we don't copy more connectors than planned, and report the actual number of connectors copied. That way any racing hot-add/remove will be handled. Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_crtc.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)