Re: [Intel-gfx] [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout
On Thu, Apr 2, 2020 at 11:39 AM Laurent Pinchart
wrote:
>
> Hi Daniel,
>
> On Thu, Apr 02, 2020 at 07:17:40AM +0200, Daniel Vetter wrote:
> > On Thu, Apr 2, 2020 at 2:50 AM Laurent Pinchart wrote:
> > > On Mon, Mar 23, 2020 at 03:49:18PM +0100, Daniel Vetter wrote:
> > > > A few things:
> > > > - Update the example driver in the documentation.
> > > > - We can drop the old kfree in drm_dev_release.
> > > > - Add a WARN_ON check in drm_dev_register to make sure everyone calls
> > > > drmm_add_final_kfree and there's no leaks.
> > > >
> > > > v2: Restore the full cleanup, I accidentally left some moved code
> > > > behind when fixing the bisectability of the series.
> > > >
> > > > Acked-by: Sam Ravnborg
> > > > Acked-by: Thomas Zimmermann
> > > > Cc: Sam Ravnborg
> > > > Cc: Dan Carpenter
> > > > Signed-off-by: Daniel Vetter
> > > > ---
> > > > drivers/gpu/drm/drm_drv.c | 12 +---
> > > > 1 file changed, 5 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > > index 877ded348b6e..7f9d7ea543a0 100644
> > > > --- a/drivers/gpu/drm/drm_drv.c
> > > > +++ b/drivers/gpu/drm/drm_drv.c
> > > > @@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor)
> > > > *
> > > > * drm_mode_config_cleanup(drm);
> > > > * drm_dev_fini(drm);
> > > > - * kfree(priv->userspace_facing);
> > > > - * kfree(priv);
> > > > * }
> > > > *
> > > > * static struct drm_driver driver_drm_driver = {
> > > > @@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor)
> > > > * kfree(drm);
> > > > * return ret;
> > > > * }
> > > > + * drmm_add_final_kfree(drm, priv);
> > > > *
> > > > * drm_mode_config_init(drm);
> > > > *
> > > > - * priv->userspace_facing = kzalloc(..., GFP_KERNEL);
> > > > + * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
> > > > * if (!priv->userspace_facing)
> > > > * return -ENOMEM;
> > > > *
> > > > @@ -837,10 +836,7 @@ static void drm_dev_release(struct kref *ref)
> > > >
> > > > drm_managed_release(dev);
> > > >
> > > > - if (!dev->driver->release && !dev->managed.final_kfree) {
> > > > - WARN_ON(!list_empty(>managed.resources));
> > > > - kfree(dev);
> > > > - } else if (dev->managed.final_kfree)
> > > > + if (dev->managed.final_kfree)
> > > > kfree(dev->managed.final_kfree);
> > > > }
> > > >
> > > > @@ -961,6 +957,8 @@ int drm_dev_register(struct drm_device *dev,
> > > > unsigned long flags)
> > > > if (!driver->load)
> > > > drm_mode_config_validate(dev);
> > > >
> > > > + WARN_ON(!dev->managed.final_kfree);
> > >
> > > That's too aggressive. Driver freeing their private object in .release()
> > > isn't wrong. I'd even go as far as saying that it should be the norm,
> > > until we manage to find a better way to handle that (which this series
> > > doesn't achieve). Many drivers need to allocate resources at probe time
> > > before they get a chance to init the drm device. Those resources must be
> > > released in the error handling paths of probe. By requiring
> > > drmm_add_final_kfree(), you're making that much more complex. I can't
> > > release those resources in the error path anymore after calling
> > > drmm_add_final_kfree(), or they will be released twice. And I can't rely
> > > on drmm_* to release them in all cases, as the error path may be hit
> > > before touching anything drm-related.
> > >
> > > Until we figure out a good way forward and test it on a significant
> > > number of drivers, let's not add WARN_ON() that will be hit with the
> > > majority of drivers, forcing them to be converted to something that is
> > > clearly half-baked.
> >
> > Hm, is this conjecture, or did you actually hit this WARN_ON with a
> > driver? Because I did audit them all, none should hit this, all are
> > fixed up.
>
> I'm sorry, I should have been clear about that. I hit the issue
> yesterday after rebasing the Xilinx ZynqMP DPSUB driver. I took Sam's
> suggestion to embed struct drm_device instead of allocating it
> dynamically, and then hit the WARN_ON. You're of course not responsible
> for a driver that is still out-of-tree. I then looked at how to convert
> other drivers I work on in a similar way (rcar-du and omapdrm in
> particular), and realized it could actually make cleanup more complex to
> always enforce usage of managed memory for everything.
>
> I apologize for the harsh tone of the previous e-mail, you certainly
> didn't deserve that (even more so as I've only reviewed the initial
> version of the series).
>
> > Also, I'm now actually going through all the places where I've added
> > the drmm_add_final_kfree and remove it again, they are _all_ about 5
> > lines after the kzalloc that allocates the driver structure which has
> >
Re: [Intel-gfx] [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout
Hi Daniel,
On Thu, Apr 02, 2020 at 07:17:40AM +0200, Daniel Vetter wrote:
> On Thu, Apr 2, 2020 at 2:50 AM Laurent Pinchart wrote:
> > On Mon, Mar 23, 2020 at 03:49:18PM +0100, Daniel Vetter wrote:
> > > A few things:
> > > - Update the example driver in the documentation.
> > > - We can drop the old kfree in drm_dev_release.
> > > - Add a WARN_ON check in drm_dev_register to make sure everyone calls
> > > drmm_add_final_kfree and there's no leaks.
> > >
> > > v2: Restore the full cleanup, I accidentally left some moved code
> > > behind when fixing the bisectability of the series.
> > >
> > > Acked-by: Sam Ravnborg
> > > Acked-by: Thomas Zimmermann
> > > Cc: Sam Ravnborg
> > > Cc: Dan Carpenter
> > > Signed-off-by: Daniel Vetter
> > > ---
> > > drivers/gpu/drm/drm_drv.c | 12 +---
> > > 1 file changed, 5 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > index 877ded348b6e..7f9d7ea543a0 100644
> > > --- a/drivers/gpu/drm/drm_drv.c
> > > +++ b/drivers/gpu/drm/drm_drv.c
> > > @@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor)
> > > *
> > > * drm_mode_config_cleanup(drm);
> > > * drm_dev_fini(drm);
> > > - * kfree(priv->userspace_facing);
> > > - * kfree(priv);
> > > * }
> > > *
> > > * static struct drm_driver driver_drm_driver = {
> > > @@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor)
> > > * kfree(drm);
> > > * return ret;
> > > * }
> > > + * drmm_add_final_kfree(drm, priv);
> > > *
> > > * drm_mode_config_init(drm);
> > > *
> > > - * priv->userspace_facing = kzalloc(..., GFP_KERNEL);
> > > + * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
> > > * if (!priv->userspace_facing)
> > > * return -ENOMEM;
> > > *
> > > @@ -837,10 +836,7 @@ static void drm_dev_release(struct kref *ref)
> > >
> > > drm_managed_release(dev);
> > >
> > > - if (!dev->driver->release && !dev->managed.final_kfree) {
> > > - WARN_ON(!list_empty(>managed.resources));
> > > - kfree(dev);
> > > - } else if (dev->managed.final_kfree)
> > > + if (dev->managed.final_kfree)
> > > kfree(dev->managed.final_kfree);
> > > }
> > >
> > > @@ -961,6 +957,8 @@ int drm_dev_register(struct drm_device *dev, unsigned
> > > long flags)
> > > if (!driver->load)
> > > drm_mode_config_validate(dev);
> > >
> > > + WARN_ON(!dev->managed.final_kfree);
> >
> > That's too aggressive. Driver freeing their private object in .release()
> > isn't wrong. I'd even go as far as saying that it should be the norm,
> > until we manage to find a better way to handle that (which this series
> > doesn't achieve). Many drivers need to allocate resources at probe time
> > before they get a chance to init the drm device. Those resources must be
> > released in the error handling paths of probe. By requiring
> > drmm_add_final_kfree(), you're making that much more complex. I can't
> > release those resources in the error path anymore after calling
> > drmm_add_final_kfree(), or they will be released twice. And I can't rely
> > on drmm_* to release them in all cases, as the error path may be hit
> > before touching anything drm-related.
> >
> > Until we figure out a good way forward and test it on a significant
> > number of drivers, let's not add WARN_ON() that will be hit with the
> > majority of drivers, forcing them to be converted to something that is
> > clearly half-baked.
>
> Hm, is this conjecture, or did you actually hit this WARN_ON with a
> driver? Because I did audit them all, none should hit this, all are
> fixed up.
I'm sorry, I should have been clear about that. I hit the issue
yesterday after rebasing the Xilinx ZynqMP DPSUB driver. I took Sam's
suggestion to embed struct drm_device instead of allocating it
dynamically, and then hit the WARN_ON. You're of course not responsible
for a driver that is still out-of-tree. I then looked at how to convert
other drivers I work on in a similar way (rcar-du and omapdrm in
particular), and realized it could actually make cleanup more complex to
always enforce usage of managed memory for everything.
I apologize for the harsh tone of the previous e-mail, you certainly
didn't deserve that (even more so as I've only reviewed the initial
version of the series).
> Also, I'm now actually going through all the places where I've added
> the drmm_add_final_kfree and remove it again, they are _all_ about 5
> lines after the kzalloc that allocates the driver structure which has
> drm_device embedded.
>
> So I'd like to understand where you get your seemingly rather sure
> convinction from that this is a horrible mistake here ...
Overall this features simplifies lots of drivers, and, even more
importantly, remove lots of actual or potential
Re: [Intel-gfx] [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout
On Thu, Apr 2, 2020 at 2:50 AM Laurent Pinchart
wrote:
>
> Hi Daniel,
>
> (On a side note, git-format-patch accepts a -v argument to specify the
> version, I didn't realize you were not aware of it :-))
>
> On Mon, Mar 23, 2020 at 03:49:18PM +0100, Daniel Vetter wrote:
> > A few things:
> > - Update the example driver in the documentation.
> > - We can drop the old kfree in drm_dev_release.
> > - Add a WARN_ON check in drm_dev_register to make sure everyone calls
> > drmm_add_final_kfree and there's no leaks.
> >
> > v2: Restore the full cleanup, I accidentally left some moved code
> > behind when fixing the bisectability of the series.
> >
> > Acked-by: Sam Ravnborg
> > Acked-by: Thomas Zimmermann
> > Cc: Sam Ravnborg
> > Cc: Dan Carpenter
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_drv.c | 12 +---
> > 1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 877ded348b6e..7f9d7ea543a0 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor)
> > *
> > * drm_mode_config_cleanup(drm);
> > * drm_dev_fini(drm);
> > - * kfree(priv->userspace_facing);
> > - * kfree(priv);
> > * }
> > *
> > * static struct drm_driver driver_drm_driver = {
> > @@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor)
> > * kfree(drm);
> > * return ret;
> > * }
> > + * drmm_add_final_kfree(drm, priv);
> > *
> > * drm_mode_config_init(drm);
> > *
> > - * priv->userspace_facing = kzalloc(..., GFP_KERNEL);
> > + * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
> > * if (!priv->userspace_facing)
> > * return -ENOMEM;
> > *
> > @@ -837,10 +836,7 @@ static void drm_dev_release(struct kref *ref)
> >
> > drm_managed_release(dev);
> >
> > - if (!dev->driver->release && !dev->managed.final_kfree) {
> > - WARN_ON(!list_empty(>managed.resources));
> > - kfree(dev);
> > - } else if (dev->managed.final_kfree)
> > + if (dev->managed.final_kfree)
> > kfree(dev->managed.final_kfree);
> > }
> >
> > @@ -961,6 +957,8 @@ int drm_dev_register(struct drm_device *dev, unsigned
> > long flags)
> > if (!driver->load)
> > drm_mode_config_validate(dev);
> >
> > + WARN_ON(!dev->managed.final_kfree);
>
> That's too aggressive. Driver freeing their private object in .release()
> isn't wrong. I'd even go as far as saying that it should be the norm,
> until we manage to find a better way to handle that (which this series
> doesn't achieve). Many drivers need to allocate resources at probe time
> before they get a chance to init the drm device. Those resources must be
> released in the error handling paths of probe. By requiring
> drmm_add_final_kfree(), you're making that much more complex. I can't
> release those resources in the error path anymore after calling
> drmm_add_final_kfree(), or they will be released twice. And I can't rely
> on drmm_* to release them in all cases, as the error path may be hit
> before touching anything drm-related.
>
> Until we figure out a good way forward and test it on a significant
> number of drivers, let's not add WARN_ON() that will be hit with the
> majority of drivers, forcing them to be converted to something that is
> clearly half-baked.
Hm, is this conjecture, or did you actually hit this WARN_ON with a
driver? Because I did audit them all, none should hit this, all are
fixed up.
Also, I'm now actually going through all the places where I've added
the drmm_add_final_kfree and remove it again, they are _all_ about 5
lines after the kzalloc that allocates the driver structure which has
drm_device embedded.
So I'd like to understand where you get your seemingly rather sure
convinction from that this is a horrible mistake here ...
/me confused
-Daniel
>
> > +
> > if (drm_dev_needs_global_mutex(dev))
> > mutex_lock(_global_mutex);
> >
>
> --
> 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 19/51] drm: Cleanups after drmm_add_final_kfree rollout
Hi Daniel,
(On a side note, git-format-patch accepts a -v argument to specify the
version, I didn't realize you were not aware of it :-))
On Mon, Mar 23, 2020 at 03:49:18PM +0100, Daniel Vetter wrote:
> A few things:
> - Update the example driver in the documentation.
> - We can drop the old kfree in drm_dev_release.
> - Add a WARN_ON check in drm_dev_register to make sure everyone calls
> drmm_add_final_kfree and there's no leaks.
>
> v2: Restore the full cleanup, I accidentally left some moved code
> behind when fixing the bisectability of the series.
>
> Acked-by: Sam Ravnborg
> Acked-by: Thomas Zimmermann
> Cc: Sam Ravnborg
> Cc: Dan Carpenter
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_drv.c | 12 +---
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 877ded348b6e..7f9d7ea543a0 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor)
> *
> * drm_mode_config_cleanup(drm);
> * drm_dev_fini(drm);
> - * kfree(priv->userspace_facing);
> - * kfree(priv);
> * }
> *
> * static struct drm_driver driver_drm_driver = {
> @@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor)
> * kfree(drm);
> * return ret;
> * }
> + * drmm_add_final_kfree(drm, priv);
> *
> * drm_mode_config_init(drm);
> *
> - * priv->userspace_facing = kzalloc(..., GFP_KERNEL);
> + * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
> * if (!priv->userspace_facing)
> * return -ENOMEM;
> *
> @@ -837,10 +836,7 @@ static void drm_dev_release(struct kref *ref)
>
> drm_managed_release(dev);
>
> - if (!dev->driver->release && !dev->managed.final_kfree) {
> - WARN_ON(!list_empty(>managed.resources));
> - kfree(dev);
> - } else if (dev->managed.final_kfree)
> + if (dev->managed.final_kfree)
> kfree(dev->managed.final_kfree);
> }
>
> @@ -961,6 +957,8 @@ int drm_dev_register(struct drm_device *dev, unsigned
> long flags)
> if (!driver->load)
> drm_mode_config_validate(dev);
>
> + WARN_ON(!dev->managed.final_kfree);
That's too aggressive. Driver freeing their private object in .release()
isn't wrong. I'd even go as far as saying that it should be the norm,
until we manage to find a better way to handle that (which this series
doesn't achieve). Many drivers need to allocate resources at probe time
before they get a chance to init the drm device. Those resources must be
released in the error handling paths of probe. By requiring
drmm_add_final_kfree(), you're making that much more complex. I can't
release those resources in the error path anymore after calling
drmm_add_final_kfree(), or they will be released twice. And I can't rely
on drmm_* to release them in all cases, as the error path may be hit
before touching anything drm-related.
Until we figure out a good way forward and test it on a significant
number of drivers, let's not add WARN_ON() that will be hit with the
majority of drivers, forcing them to be converted to something that is
clearly half-baked.
> +
> if (drm_dev_needs_global_mutex(dev))
> mutex_lock(_global_mutex);
>
--
Regards,
Laurent Pinchart
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout
A few things:
- Update the example driver in the documentation.
- We can drop the old kfree in drm_dev_release.
- Add a WARN_ON check in drm_dev_register to make sure everyone calls
drmm_add_final_kfree and there's no leaks.
v2: Restore the full cleanup, I accidentally left some moved code
behind when fixing the bisectability of the series.
Acked-by: Sam Ravnborg
Acked-by: Thomas Zimmermann
Cc: Sam Ravnborg
Cc: Dan Carpenter
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_drv.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 877ded348b6e..7f9d7ea543a0 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor)
*
* drm_mode_config_cleanup(drm);
* drm_dev_fini(drm);
- * kfree(priv->userspace_facing);
- * kfree(priv);
* }
*
* static struct drm_driver driver_drm_driver = {
@@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor)
* kfree(drm);
* return ret;
* }
+ * drmm_add_final_kfree(drm, priv);
*
* drm_mode_config_init(drm);
*
- * priv->userspace_facing = kzalloc(..., GFP_KERNEL);
+ * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
* if (!priv->userspace_facing)
* return -ENOMEM;
*
@@ -837,10 +836,7 @@ static void drm_dev_release(struct kref *ref)
drm_managed_release(dev);
- if (!dev->driver->release && !dev->managed.final_kfree) {
- WARN_ON(!list_empty(>managed.resources));
- kfree(dev);
- } else if (dev->managed.final_kfree)
+ if (dev->managed.final_kfree)
kfree(dev->managed.final_kfree);
}
@@ -961,6 +957,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long
flags)
if (!driver->load)
drm_mode_config_validate(dev);
+ WARN_ON(!dev->managed.final_kfree);
+
if (drm_dev_needs_global_mutex(dev))
mutex_lock(_global_mutex);
--
2.25.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout
Am 02.03.20 um 23:25 schrieb Daniel Vetter:
> A few things:
> - Update the example driver in the documentation.
> - We can drop the old kfree in drm_dev_release.
> - Add a WARN_ON check in drm_dev_register to make sure everyone calls
> drmm_add_final_kfree and there's no leaks.
>
> Signed-off-by: Daniel Vetter
Acked-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_drv.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7b84ee8a5eb5..1a048325f30e 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor)
> *
> * drm_mode_config_cleanup(drm);
> * drm_dev_fini(drm);
> - * kfree(priv->userspace_facing);
> - * kfree(priv);
> * }
> *
> * static struct drm_driver driver_drm_driver = {
> @@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor)
> * kfree(drm);
> * return ret;
> * }
> + * drmm_add_final_kfree(drm, priv);
> *
> * drm_mode_config_init(drm);
> *
> - * priv->userspace_facing = kzalloc(..., GFP_KERNEL);
> + * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
> * if (!priv->userspace_facing)
> * return -ENOMEM;
> *
> @@ -961,6 +960,8 @@ int drm_dev_register(struct drm_device *dev, unsigned
> long flags)
> struct drm_driver *driver = dev->driver;
> int ret;
>
> + WARN_ON(!dev->managed.final_kfree);
> +
> if (drm_dev_needs_global_mutex(dev))
> mutex_lock(_global_mutex);
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout
On Mon, Mar 02, 2020 at 11:25:59PM +0100, Daniel Vetter wrote:
> A few things:
> - Update the example driver in the documentation.
> - We can drop the old kfree in drm_dev_release.
> - Add a WARN_ON check in drm_dev_register to make sure everyone calls
> drmm_add_final_kfree and there's no leaks.
>
> Signed-off-by: Daniel Vetter
Acked-by: Sam Ravnborg
> ---
> drivers/gpu/drm/drm_drv.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7b84ee8a5eb5..1a048325f30e 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor)
> *
> * drm_mode_config_cleanup(drm);
> * drm_dev_fini(drm);
> - * kfree(priv->userspace_facing);
> - * kfree(priv);
> * }
> *
> * static struct drm_driver driver_drm_driver = {
> @@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor)
> * kfree(drm);
> * return ret;
> * }
> + * drmm_add_final_kfree(drm, priv);
> *
> * drm_mode_config_init(drm);
> *
> - * priv->userspace_facing = kzalloc(..., GFP_KERNEL);
> + * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
> * if (!priv->userspace_facing)
> * return -ENOMEM;
> *
> @@ -961,6 +960,8 @@ int drm_dev_register(struct drm_device *dev, unsigned
> long flags)
> struct drm_driver *driver = dev->driver;
> int ret;
>
> + WARN_ON(!dev->managed.final_kfree);
> +
> if (drm_dev_needs_global_mutex(dev))
> mutex_lock(_global_mutex);
>
> --
> 2.24.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout
A few things:
- Update the example driver in the documentation.
- We can drop the old kfree in drm_dev_release.
- Add a WARN_ON check in drm_dev_register to make sure everyone calls
drmm_add_final_kfree and there's no leaks.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_drv.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7b84ee8a5eb5..1a048325f30e 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor)
*
* drm_mode_config_cleanup(drm);
* drm_dev_fini(drm);
- * kfree(priv->userspace_facing);
- * kfree(priv);
* }
*
* static struct drm_driver driver_drm_driver = {
@@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor)
* kfree(drm);
* return ret;
* }
+ * drmm_add_final_kfree(drm, priv);
*
* drm_mode_config_init(drm);
*
- * priv->userspace_facing = kzalloc(..., GFP_KERNEL);
+ * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
* if (!priv->userspace_facing)
* return -ENOMEM;
*
@@ -961,6 +960,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long
flags)
struct drm_driver *driver = dev->driver;
int ret;
+ WARN_ON(!dev->managed.final_kfree);
+
if (drm_dev_needs_global_mutex(dev))
mutex_lock(_global_mutex);
--
2.24.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout
A few things:
- Update the example driver in the documentation.
- We can drop the old kfree in drm_dev_release.
- Add a WARN_ON check in drm_dev_register to make sure everyone calls
drmm_add_final_kfree and there's no leaks.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_drv.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9e62e28bbc62..1ee606b4a4f9 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor)
*
* drm_mode_config_cleanup(drm);
* drm_dev_fini(drm);
- * kfree(priv->userspace_facing);
- * kfree(priv);
* }
*
* static struct drm_driver driver_drm_driver = {
@@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor)
* kfree(drm);
* return ret;
* }
+ * drmm_add_final_kfree(drm, priv);
*
* drm_mode_config_init(drm);
*
- * priv->userspace_facing = kzalloc(..., GFP_KERNEL);
+ * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
* if (!priv->userspace_facing)
* return -ENOMEM;
*
@@ -834,10 +833,6 @@ static void drm_dev_release(struct kref *ref)
dev->driver->release(dev);
} else {
drm_dev_fini(dev);
- if (!dev->managed.final_kfree) {
- WARN_ON(!list_empty(>managed.resources));
- kfree(dev);
- }
}
drm_managed_release(dev);
@@ -960,6 +955,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long
flags)
struct drm_driver *driver = dev->driver;
int ret;
+ WARN_ON(!dev->managed.final_kfree);
+
if (drm_dev_needs_global_mutex(dev))
mutex_lock(_global_mutex);
--
2.24.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/51] drm: Cleanups after drmm_add_final_kfree rollout
A few things:
- Update the example driver in the documentation.
- We can drop the old kfree in drm_dev_release.
- Add a WARN_ON check in drm_dev_register to make sure everyone calls
drmm_add_final_kfree and there's no leaks.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_drv.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9e62e28bbc62..1ee606b4a4f9 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -297,8 +297,6 @@ void drm_minor_release(struct drm_minor *minor)
*
* drm_mode_config_cleanup(drm);
* drm_dev_fini(drm);
- * kfree(priv->userspace_facing);
- * kfree(priv);
* }
*
* static struct drm_driver driver_drm_driver = {
@@ -326,10 +324,11 @@ void drm_minor_release(struct drm_minor *minor)
* kfree(drm);
* return ret;
* }
+ * drmm_add_final_kfree(drm, priv);
*
* drm_mode_config_init(drm);
*
- * priv->userspace_facing = kzalloc(..., GFP_KERNEL);
+ * priv->userspace_facing = drmm_kzalloc(..., GFP_KERNEL);
* if (!priv->userspace_facing)
* return -ENOMEM;
*
@@ -834,10 +833,6 @@ static void drm_dev_release(struct kref *ref)
dev->driver->release(dev);
} else {
drm_dev_fini(dev);
- if (!dev->managed.final_kfree) {
- WARN_ON(!list_empty(>managed.resources));
- kfree(dev);
- }
}
drm_managed_release(dev);
@@ -960,6 +955,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long
flags)
struct drm_driver *driver = dev->driver;
int ret;
+ WARN_ON(!dev->managed.final_kfree);
+
if (drm_dev_needs_global_mutex(dev))
mutex_lock(_global_mutex);
--
2.24.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
