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@xxxxxxxxx>
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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
