[Intel-gfx] [PATCH] drm: Don't compute obj counts expensively in get_resources

[Chris Wilson]

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 at chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>

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 at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre