On Tue, Jun 21, 2016 at 02:29:48PM +0200, Daniel Vetter wrote: > 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. > > v2: Actually try to not blow up, somehow I lost the hunk that checks > we don't copy too much. Noticed by Chris. > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> The defacto API is call once with count_connectors=0 then with again with the previous value of count_connectors. On the second pass, the user is expecting no more then count_connectors to be copied, and is not expecting to be told about new ones. The expected ABI looks unchanged. > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 28c109ff7330..59c5261a309c 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); Ok, since in the future num_connector is not guarded by mode_conf.mutex, moving the read underneath that lock doesn't obviate the need for READ_ONCE. Still it reduces the window and how far one has too look back if it were set just before the connector loop. Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
