Re: [Intel-gfx] [PATCH 21/52] drm: Handle dev->unique with drmm_
On Wed, Feb 19, 2020 at 3:28 PM Laurent Pinchart
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
> > ---
> > 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(>filelist_mutex);
> > mutex_destroy(>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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 21/52] drm: Handle dev->unique with drmm_
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
> ---
> 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(>filelist_mutex);
> mutex_destroy(>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);
--
Regards,
Laurent Pinchart
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 21/52] drm: Handle dev->unique with drmm_
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
---
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(>filelist_mutex);
mutex_destroy(>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);
--
2.24.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
