diff mbox series

[21/52] drm: Handle dev->unique with drmm_

Message ID 20200219102122.1607365-22-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show
Series drm_device managed resources | expand

Commit Message

Daniel Vetter Feb. 19, 2020, 10:20 a.m. UTC 
We need to add a drmm_kstrdup for this, but let's start somewhere.

This is not exactly perfect onion unwinding, but it's jsut a kfree so
doesn't really matter at all.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c     |  5 ++---
 drivers/gpu/drm/drm_managed.c | 16 ++++++++++++++++
 include/drm/drm_managed.h     |  1 +
 3 files changed, 19 insertions(+), 3 deletions(-)

Comments

Laurent Pinchart Feb. 19, 2020, 2:28 p.m. UTC | #1 
Hi Daniel,

Thank you for the patch.

On Wed, Feb 19, 2020 at 11:20:51AM +0100, Daniel Vetter wrote:
> We need to add a drmm_kstrdup for this, but let's start somewhere.
> 
> This is not exactly perfect onion unwinding, but it's jsut a kfree so
> doesn't really matter at all.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_drv.c     |  5 ++---
>  drivers/gpu/drm/drm_managed.c | 16 ++++++++++++++++
>  include/drm/drm_managed.h     |  1 +
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 1ee606b4a4f9..782fd5d6f8b2 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -777,7 +777,6 @@ void drm_dev_fini(struct drm_device *dev)
>  	mutex_destroy(&dev->filelist_mutex);
>  	mutex_destroy(&dev->struct_mutex);
>  	drm_legacy_destroy_members(dev);
> -	kfree(dev->unique);
>  }
>  EXPORT_SYMBOL(drm_dev_fini);
>  
> @@ -1063,8 +1062,8 @@ EXPORT_SYMBOL(drm_dev_unregister);
>   */
>  int drm_dev_set_unique(struct drm_device *dev, const char *name)
>  {
> -	kfree(dev->unique);
> -	dev->unique = kstrdup(name, GFP_KERNEL);
> +	drmm_kfree(dev, dev->unique);
> +	dev->unique = drmm_kstrdup(dev, name, GFP_KERNEL);
>  
>  	return dev->unique ? 0 : -ENOMEM;
>  }
> diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> index ee7c7253af61..d8a484e19830 100644
> --- a/drivers/gpu/drm/drm_managed.c
> +++ b/drivers/gpu/drm/drm_managed.c
> @@ -147,6 +147,22 @@ void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp)
>  }
>  EXPORT_SYMBOL(drmm_kmalloc);
>  
> +char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp)

Do we need support for gfp_t other than GFP_KERNEL ? Given that the
memory will be released when the drm_device is destroyed, GFP_ATOMIC
would seem of dubious use to me, and we may want to not make it possible
to use it.

> +{
> +	size_t size;
> +	char *buf;
> +
> +	if (!s)
> +		return NULL;
> +
> +	size = strlen(s) + 1;
> +	buf = drmm_kmalloc(dev, size, gfp);
> +	if (buf)
> +		memcpy(buf, s, size);
> +	return buf;
> +}
> +EXPORT_SYMBOL_GPL(drmm_kstrdup);
> +
>  void drmm_kfree(struct drm_device *dev, void *data)
>  {
>  	struct drmres *dr = NULL, *tmp;
> diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
> index 75f2c8932c69..240edd395e88 100644
> --- a/include/drm/drm_managed.h
> +++ b/include/drm/drm_managed.h
> @@ -21,5 +21,6 @@ static inline void *drmm_kzalloc(struct drm_device *dev, size_t size, gfp_t gfp)
>  {
>  	return drmm_kmalloc(dev, size, gfp | __GFP_ZERO);
>  }
> +char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp);
>  
>  void drmm_kfree(struct drm_device *dev, void *data);
Daniel Vetter Feb. 19, 2020, 3:24 p.m. UTC | #2 
On Wed, Feb 19, 2020 at 3:28 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Daniel,
>
> Thank you for the patch.
>
> On Wed, Feb 19, 2020 at 11:20:51AM +0100, Daniel Vetter wrote:
> > We need to add a drmm_kstrdup for this, but let's start somewhere.
> >
> > This is not exactly perfect onion unwinding, but it's jsut a kfree so
> > doesn't really matter at all.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_drv.c     |  5 ++---
> >  drivers/gpu/drm/drm_managed.c | 16 ++++++++++++++++
> >  include/drm/drm_managed.h     |  1 +
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 1ee606b4a4f9..782fd5d6f8b2 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -777,7 +777,6 @@ void drm_dev_fini(struct drm_device *dev)
> >       mutex_destroy(&dev->filelist_mutex);
> >       mutex_destroy(&dev->struct_mutex);
> >       drm_legacy_destroy_members(dev);
> > -     kfree(dev->unique);
> >  }
> >  EXPORT_SYMBOL(drm_dev_fini);
> >
> > @@ -1063,8 +1062,8 @@ EXPORT_SYMBOL(drm_dev_unregister);
> >   */
> >  int drm_dev_set_unique(struct drm_device *dev, const char *name)
> >  {
> > -     kfree(dev->unique);
> > -     dev->unique = kstrdup(name, GFP_KERNEL);
> > +     drmm_kfree(dev, dev->unique);
> > +     dev->unique = drmm_kstrdup(dev, name, GFP_KERNEL);
> >
> >       return dev->unique ? 0 : -ENOMEM;
> >  }
> > diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> > index ee7c7253af61..d8a484e19830 100644
> > --- a/drivers/gpu/drm/drm_managed.c
> > +++ b/drivers/gpu/drm/drm_managed.c
> > @@ -147,6 +147,22 @@ void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp)
> >  }
> >  EXPORT_SYMBOL(drmm_kmalloc);
> >
> > +char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp)
>
> Do we need support for gfp_t other than GFP_KERNEL ? Given that the
> memory will be released when the drm_device is destroyed, GFP_ATOMIC
> would seem of dubious use to me, and we may want to not make it possible
> to use it.

Hm .. I just copied the devm_ functions with struct drm_device
substituting struct device. I agree there's not going to be a use-case
for anything else than GFP_KERNEL. If there ever will be, I guess we
could add a __drm_kmallock for that case.

The downside of dropping the gfp argument is that drmm_ wont be a
drop-in replacament for devm_k*alloc functions anymore. Adding Greg,
maybe he has a good idea about what we should be doing here?

Personally I'd be happy to drop the gfp argument, it feels somewhat
pointless for these.
-Daniel


> > +{
> > +     size_t size;
> > +     char *buf;
> > +
> > +     if (!s)
> > +             return NULL;
> > +
> > +     size = strlen(s) + 1;
> > +     buf = drmm_kmalloc(dev, size, gfp);
> > +     if (buf)
> > +             memcpy(buf, s, size);
> > +     return buf;
> > +}
> > +EXPORT_SYMBOL_GPL(drmm_kstrdup);
> > +
> >  void drmm_kfree(struct drm_device *dev, void *data)
> >  {
> >       struct drmres *dr = NULL, *tmp;
> > diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
> > index 75f2c8932c69..240edd395e88 100644
> > --- a/include/drm/drm_managed.h
> > +++ b/include/drm/drm_managed.h
> > @@ -21,5 +21,6 @@ static inline void *drmm_kzalloc(struct drm_device *dev, size_t size, gfp_t gfp)
> >  {
> >       return drmm_kmalloc(dev, size, gfp | __GFP_ZERO);
> >  }
> > +char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp);
> >
> >  void drmm_kfree(struct drm_device *dev, void *data);
>
> --
> Regards,
>
> Laurent Pinchart
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 1ee606b4a4f9..782fd5d6f8b2 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -777,7 +777,6 @@  void drm_dev_fini(struct drm_device *dev)
 	mutex_destroy(&dev->filelist_mutex);
 	mutex_destroy(&dev->struct_mutex);
 	drm_legacy_destroy_members(dev);
-	kfree(dev->unique);
 }
 EXPORT_SYMBOL(drm_dev_fini);
 
@@ -1063,8 +1062,8 @@  EXPORT_SYMBOL(drm_dev_unregister);
  */
 int drm_dev_set_unique(struct drm_device *dev, const char *name)
 {
-	kfree(dev->unique);
-	dev->unique = kstrdup(name, GFP_KERNEL);
+	drmm_kfree(dev, dev->unique);
+	dev->unique = drmm_kstrdup(dev, name, GFP_KERNEL);
 
 	return dev->unique ? 0 : -ENOMEM;
 }
diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index ee7c7253af61..d8a484e19830 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -147,6 +147,22 @@  void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp)
 }
 EXPORT_SYMBOL(drmm_kmalloc);
 
+char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp)
+{
+	size_t size;
+	char *buf;
+
+	if (!s)
+		return NULL;
+
+	size = strlen(s) + 1;
+	buf = drmm_kmalloc(dev, size, gfp);
+	if (buf)
+		memcpy(buf, s, size);
+	return buf;
+}
+EXPORT_SYMBOL_GPL(drmm_kstrdup);
+
 void drmm_kfree(struct drm_device *dev, void *data)
 {
 	struct drmres *dr = NULL, *tmp;
diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index 75f2c8932c69..240edd395e88 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -21,5 +21,6 @@  static inline void *drmm_kzalloc(struct drm_device *dev, size_t size, gfp_t gfp)
 {
 	return drmm_kmalloc(dev, size, gfp | __GFP_ZERO);
 }
+char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp);
 
 void drmm_kfree(struct drm_device *dev, void *data);