[PATCH] drm: Restore drm_file->is_master

[David Herrmann]

Hi

On Thu, Aug 7, 2014 at 3:04 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> Despite the claims of
>
> commit 48ba813701eb14b3008edefef4a0789b328e278c
> Author: David Herrmann <dh.herrmann at gmail.com>
> Date:   Tue Jul 22 18:46:09 2014 +0200
>
>     drm: drop redundant drm_file->is_master
>
> drm_file->is_master is not synomous with having drm_file->master ==
> drm_file->minor->master. This is because drm_file->master is the same
> for all drm_files of the same generation and so when there is a master,
> every drm_file believes itself to be the master. Confusion ensues and
> things go pear shaped when one file is closed and there is no master
> anymore.

Uagh, embarrassing. A revert is fine with me, but I'll try to review
your patch once I get home.

Thanks
David

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82283
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: David Herrmann <dh.herrmann at gmail.com>
> ---
>  drivers/gpu/drm/drm_drv.c  |  5 ++---
>  drivers/gpu/drm/drm_fops.c | 12 ++++++------
>  include/drm/drmP.h         |  3 ++-
>  3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index a1863d8..76cdb51 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -198,6 +198,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
>                 if (unlikely(ret != 0))
>                         drm_master_put(&file_priv->minor->master);
>         }
> +       file_priv->is_master = ret == 0;
>
>  out_unlock:
>         mutex_unlock(&dev->master_mutex);
> @@ -213,13 +214,11 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
>         if (!drm_is_master(file_priv))
>                 goto out_unlock;
>
> -       if (!file_priv->minor->master)
> -               goto out_unlock;
> -
>         ret = 0;
>         if (dev->driver->master_drop)
>                 dev->driver->master_drop(dev, file_priv, false);
>         drm_master_put(&file_priv->minor->master);
> +       file_priv->is_master = false;
>
>  out_unlock:
>         mutex_unlock(&dev->master_mutex);
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index c0166ba..083f7b9 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -196,6 +196,7 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
>
>                 /* take another reference for the copy in the local file priv */
>                 priv->master = drm_master_get(priv->minor->master);
> +               priv->is_master = true;
>                 priv->authenticated = 1;
>
>                 if (dev->driver->master_create) {
> @@ -434,12 +435,11 @@ int drm_release(struct inode *inode, struct file *filp)
>                 }
>                 mutex_unlock(&dev->struct_mutex);
>
> -               if (file_priv->minor->master == file_priv->master) {
> -                       /* drop the reference held my the minor */
> -                       if (dev->driver->master_drop)
> -                               dev->driver->master_drop(dev, file_priv, true);
> -                       drm_master_put(&file_priv->minor->master);
> -               }
> +               /* drop the reference held my the minor */
> +               if (dev->driver->master_drop)
> +                       dev->driver->master_drop(dev, file_priv, true);
> +               drm_master_put(&file_priv->minor->master);
> +               file_priv->is_master = false;
>         }
>
>         /* drop the master reference held by the file priv */
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0ffce5c..70a6598 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -378,6 +378,7 @@ struct drm_prime_file_private {
>
>  /** File private data */
>  struct drm_file {
> +       bool is_master:1;
>         unsigned authenticated :1;
>         /* true when the client has asked us to expose stereo 3D mode flags */
>         unsigned stereo_allowed :1;
> @@ -1177,7 +1178,7 @@ static inline bool drm_is_primary_client(const struct drm_file *file_priv)
>   */
>  static inline bool drm_is_master(const struct drm_file *file)
>  {
> -       return file->master && file->master == file->minor->master;
> +       return file->is_master;
>  }
>
>  /******************************************************************/
> --
> 1.9.1
>