Hi
On Tue, Jul 16, 2013 at 3:14 PM, Daniel Vetter <daniel.vetter at ffwll.ch>
wrote:
> We not only have debugfs files to do pretty much the equivalent of
> lsof, we also have an ioctl. Not that compared to lsof this dumps a
> wee bit more information, but we can still get at that from debugfs
> easily.
>
> I've dug around in mesa, libdrm and ddx histories and the only users
> seem to be drm/tests/dristat.c and drm/tests/getclients.c. The later
> is a testcase for the ioctl itself since up to
>
> commit b018fcdaa5e8b4eabb8cffda687d00004a3c4785
> Author: Eric Anholt <eric at anholt.net>
> Date: Thu Nov 22 18:46:54 2007 +1000
>
> drm: Make DRM_IOCTL_GET_CLIENT return EINVAL when it can't find client
> #idx
>
> there was actually no way at all for userspace to enumerate all
> clients since the kernel just wouldn't tell it when to stop. Which
> completely broke it's only user, dristat -c.
>
> So obviously that ioctl wasn't much use for debugging. Hence I don't
> see any point in keeping support for a tool which was pretty obviously
> never really used, and while we have good replacements in the form of
> equivalent debugfs files.
>
> Still, to keep dristat -c from looping forever again stop it early by
> returning an unconditional -EINVAL. Also add a comment in the code
> about why.
>
> v2: Slightly less hollowed-out implementation. libva uses GET_CLIENTS
> to figure out whether the fd it has is already authenticated or not.
> So we need to keep that part of things working. Simplest way is to
> just return one entry to keep va_drm_is_authenticated in
> libva/va/drm/va_drm_auth.c working.
>
> Cc: Gwenole Beauchesne <gwenole.beauchesne at intel.com>
> Cc: David Herrmann <dh.herrmann at gmail.com>
> Cc: Eric Anholt <eric at anholt.net>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/drm_ioctl.c | 43 ++++++++++++++++++++++---------------------
> 1 file changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 0acf080..ac8ca5c 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -217,29 +217,30 @@ int drm_getclient(struct drm_device *dev, void *data,
> struct drm_file *file_priv)
> {
> struct drm_client *client = data;
> - struct drm_file *pt;
> - int idx;
> - int i;
> -
> - idx = client->idx;
> - i = 0;
>
> - mutex_lock(&dev->struct_mutex);
> - list_for_each_entry(pt, &dev->filelist, lhead) {
> - if (i++ >= idx) {
> - client->auth = pt->authenticated;
> - client->pid = pid_vnr(pt->pid);
> - client->uid = from_kuid_munged(current_user_ns(),
> pt->uid);
> - client->magic = pt->magic;
> - client->iocs = pt->ioctl_count;
> - mutex_unlock(&dev->struct_mutex);
> -
> - return 0;
> - }
> + /*
> + * Hollowed-out getclient ioctl to keep some dead old drm tests/tools
> + * not breaking completely. Userspace tools stop enumerating one they
> + * get -EINVAL, hence this is the return value we need to hand back
> for
> + * no clients tracked.
> + *
> + * Unfortunately some clients (*cough* libva *cough*) use this in a
> fun
> + * attempt to figure out whether they're authenticated or not. Since
> + * that's the only thing they care about, give it to the directly
> + * instead of walking one giant list.
> + */
> + if (client->idx == 0) {
> + client->auth = file_priv->authenticated;
> + client->pid = pid_vnr(file_priv->pid);
> + client->uid = from_kuid_munged(current_user_ns(),
> + file_priv->uid);
> + client->magic = 0;
> + client->iocs = 0;
magic = 0 is the only thing that bugs me as "0" is an invalid
magic-number for drmAuth. I hope no-one uses it. But we can fix that
if we see any regression. I cannot imagine anyone really using it.
Reviewed-by: David Herrmann <dh.herrmann at gmail.com>
> +
> + return 0;
> + } else {
> + return -EINVAL;
> }
> - mutex_unlock(&dev->struct_mutex);
> -
> - return -EINVAL;
> }
>
> /**
> --
> 1.8.3.2
>
