[PATCH 2/3] drm: simplify authentication management

[Chris Wilson]

On Mon, May 04, 2015 at 04:05:13PM +0200, David Herrmann wrote:
> The magic auth tokens we have are a simple map from cyclic IDs to drm_file
> objects. Remove all the old bulk of code and replace it with a simple,
> direct IDR.
> 
> The previous behavior is kept. Especially calling authmagic multiple times
> on the same magic results in EINVAL except on the first call. The only
> difference in behavior is that we never allocate IDs multiple times, even
> if they were already authenticated. To trigger that, you need 2^31 open
> DRM file descriptions at the same time, though.
> 
> Diff-stat:
>    5 files changed, 45 insertions(+), 157 deletions(-)
> 
> Signed-off-by: David Herrmann <dh.herrmann at gmail.com>

Just one quibble,

> @@ -189,10 +80,17 @@ int drm_authmagic(struct drm_device *dev, void *data,
>  	struct drm_file *file;
>  
>  	DRM_DEBUG("%u\n", auth->magic);
> -	if ((file = drm_find_file(file_priv->master, auth->magic))) {
> +
> +	if (auth->magic >= INT_MAX)
> +		return -EINVAL;

The idr upper bound is INT_MAX [inclusive].

> +	mutex_lock(&dev->struct_mutex);
> +	file = idr_find(&file_priv->master->magic_map, auth->magic);

But given that it will return NULL anyway, do you really want to
short-circuit the erroneous lookup, and just leave it to the idr to
validate it itself?

So... How efficient is the cyclic idr for a long running system that has
accumulated several hundred thousand stale magics?

Maybe an igt to create a couple of billion shortlived clients (with
overlapping lifetimes) and check the behaviour?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre