| Message ID | 881f57fb7f6a7f5ad45e28afbe9d6b2d147c78c4.1400075481.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi On Wed, May 14, 2014 at 3:58 PM, Jani Nikula <jani.nikula@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@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@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> Reference: http://lkml.kernel.org/r/645ee6e22cad47d38a2b35c21c8d5fe3@DC1-MBX-01.ptsecurity.ru > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > @@ -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; ^^ one of these things is not like the other, > } > + 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; ^^, I've merged this fixed into my tree, I'm sure Daniel won't mind :) Dave.
On Tue, 27 May 2014, Dave Airlie <airlied@gmail.com> wrote: >> Reference: http://lkml.kernel.org/r/645ee6e22cad47d38a2b35c21c8d5fe3@DC1-MBX-01.ptsecurity.ru >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > >> @@ -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; > > ^^ one of these things is not like the other, > > >> } >> + 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; > > ^^, > > I've merged this fixed into my tree, I'm sure Daniel won't mind :) Thanks. I'll hide behind the "RFC PATCH" subject. ;) BR, Jani.
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;
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@intel.com> --- drivers/gpu/drm/drm_crtc.c | 36 ++++++++++++++++++++---------------- include/drm/drm_crtc.h | 2 ++ 2 files changed, 22 insertions(+), 16 deletions(-)