[PATCH] drm: Don't compute obj counts expensively in get_resources
Chris Wilson
chris at chris-wilson.co.uk
Tue Jun 21 12:40:12 UTC 2016
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
More information about the dri-devel
mailing list