On Wed, Jul 23, 2014 at 05:26:42PM +0200, David Herrmann wrote:
> The drm_file->is_master field is redundant as it's equivalent to:
> drm_file->master && drm_file->master == drm_file->minor->master
>
> 1) "=>"
> Whenever we set drm_file->is_master, we also set:
> drm_file->minor->master = drm_file->master;
>
> Whenever we clear drm_file->is_master, we also call:
> drm_master_put(&drm_file->minor->master);
> which implicitly clears it to NULL.
>
> 2) "<="
> minor->master cannot be set if it is non-NULL. Therefore, it stays as
> is unless a file drops it.
>
> If minor->master is NULL, it is only set by places that also adjust
> drm_file->is_master.
>
> Therefore, we can safely drop is_master and replace it by an inline helper
> that matches:
> drm_file->master && drm_file->master == drm_file->minor->master
>
> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
Docbook for drm_is_master is missing, otherwise Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
And one question below which doesn't really matter for this patch here.
See below
-Daniel
[snip]
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index d91e09f..e1bb585 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -387,8 +387,6 @@ struct drm_prime_file_private {
> struct drm_file {
> unsigned always_authenticated :1;
> unsigned authenticated :1;
> - /* Whether we're master for a minor. Protected by master_mutex */
> - unsigned is_master :1;
> /* true when the client has asked us to expose stereo 3D mode flags */
> unsigned stereo_allowed :1;
> /*
> @@ -1034,7 +1032,7 @@ struct drm_device {
> /** \name Locks */
> /*@{ */
> struct mutex struct_mutex; /**< For others */
> - struct mutex master_mutex; /**< For drm_minor::master and drm_file::is_master */
> + struct mutex master_mutex; /**< For drm_minor::master */
> /*@} */
>
> /** \name Usage Counters */
> @@ -1172,6 +1170,11 @@ static inline bool drm_is_primary_client(const struct drm_file *file_priv)
> return file_priv->minor->type == DRM_MINOR_LEGACY;
> }
>
Docbook here please ...
> +static inline bool drm_is_master(const struct drm_file *file)
> +{
Hm, we don't have any means to synchronize is_master checks with
concurrent ioctls and stuff. Do we care? Orthogonal issue really.
> + return file->master && file->master == file->minor->master;
> +}
> +
> /******************************************************************/
> /** \name Internal function definitions */
> /*@{*/
> --
> 2.0.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
