[RFC PATCH 1/2] drm: store connector name in connector struct

[David Herrmann]

Hi

On Wed, May 14, 2014 at 3:58 PM, Jani Nikula <jani.nikula at intel.com> wrote:
> This makes drm_get_connector_name() thread safe.
>
> Reference: http://lkml.kernel.org/r/645ee6e22cad47d38a2b35c21c8d5fe3@DC1-MBX-01.ptsecurity.ru
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>

I like that approach, but we should also kill drm_get_connector_name()
in a follow-up. I don't see why that wrapper is needed.

Reviewed-by: David Herrmann <dh.herrmann at gmail.com>

Thanks
David

> ---
>  drivers/gpu/drm/drm_crtc.c | 36 ++++++++++++++++++++----------------
>  include/drm/drm_crtc.h     |  2 ++
>  2 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 461d19bd14ee..5781130b4126 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -277,21 +277,11 @@ EXPORT_SYMBOL(drm_get_encoder_name);
>
>  /**
>   * drm_get_connector_name - return a string for connector
> - * @connector: connector to compute name of
> - *
> - * Note that the buffer used by this function is globally shared and owned by
> - * the function itself.
> - *
> - * FIXME: This isn't really multithreading safe.
> + * @connector: the connector to get name for
>   */
>  const char *drm_get_connector_name(const struct drm_connector *connector)
>  {
> -       static char buf[32];
> -
> -       snprintf(buf, 32, "%s-%d",
> -                drm_connector_enum_list[connector->connector_type].name,
> -                connector->connector_type_id);
> -       return buf;
> +       return connector->name;
>  }
>  EXPORT_SYMBOL(drm_get_connector_name);
>
> @@ -824,7 +814,7 @@ int drm_connector_init(struct drm_device *dev,
>
>         ret = drm_mode_object_get(dev, &connector->base, DRM_MODE_OBJECT_CONNECTOR);
>         if (ret)
> -               goto out;
> +               goto out_unlock;
>
>         connector->base.properties = &connector->properties;
>         connector->dev = dev;
> @@ -834,9 +824,17 @@ int drm_connector_init(struct drm_device *dev,
>                 ida_simple_get(connector_ida, 1, 0, GFP_KERNEL);
>         if (connector->connector_type_id < 0) {
>                 ret = connector->connector_type_id;
> -               drm_mode_object_put(dev, &connector->base);
> -               goto out;
> +               goto out_out;
>         }
> +       connector->name =
> +               kasprintf(GFP_KERNEL, "%s-%d",
> +                         drm_connector_enum_list[connector_type].name,
> +                         connector->connector_type_id);
> +       if (!connector->name) {
> +               ret = -ENOMEM;
> +               goto out_put;
> +       }
> +
>         INIT_LIST_HEAD(&connector->probed_modes);
>         INIT_LIST_HEAD(&connector->modes);
>         connector->edid_blob_ptr = NULL;
> @@ -853,7 +851,11 @@ int drm_connector_init(struct drm_device *dev,
>         drm_object_attach_property(&connector->base,
>                                       dev->mode_config.dpms_property, 0);
>
> - out:
> +out_put:
> +       if (ret)
> +               drm_mode_object_put(dev, &connector->base);
> +
> +out_unlock:
>         drm_modeset_unlock_all(dev);
>
>         return ret;
> @@ -881,6 +883,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
>                    connector->connector_type_id);
>
>         drm_mode_object_put(dev, &connector->base);
> +       kfree(connector->name);
> +       connector->name = NULL;
>         list_del(&connector->head);
>         dev->mode_config.num_connector--;
>  }
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c061bb372199..d4cd7e513280 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -444,6 +444,7 @@ struct drm_encoder {
>   * @attr: sysfs attributes
>   * @head: list management
>   * @base: base KMS object
> + * @name: connector name
>   * @connector_type: one of the %DRM_MODE_CONNECTOR_<foo> types from drm_mode.h
>   * @connector_type_id: index into connector type enum
>   * @interlace_allowed: can this connector handle interlaced modes?
> @@ -482,6 +483,7 @@ struct drm_connector {
>
>         struct drm_mode_object base;
>
> +       char *name;
>         int connector_type;
>         int connector_type_id;
>         bool interlace_allowed;
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel