| Message ID | 20250114-google-vkms-managed-v8-3-da4254aa3dd5@bootlin.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/vkms: Switch all vkms object to DRM managed objects | expand |
Hi Am 14.01.25 um 15:05 schrieb Louis Chauvet: > Add drmm_alloc_ordered_workqueue(), a helper that provides managed ordered > workqueue cleanup. The workqueue will be destroyed with the final > reference of the DRM device. > > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> > > --- > > Hi Thomas, > > I noted that you wanted to mark this as Reviewed, but as this was not > the complete patch, I prefered to have a confirmation before merging the > patch. > > Thanks for your time, > Louis Chauvet > --- > drivers/gpu/drm/drm_managed.c | 8 ++++++++ > include/drm/drm_managed.h | 12 ++++++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c > index 79ce86a5bd67a7201f5d60550a12364f2628e0ec..1589c28410f23b5a1e93dd1638420b3ae5c4b854 100644 > --- a/drivers/gpu/drm/drm_managed.c > +++ b/drivers/gpu/drm/drm_managed.c > @@ -310,3 +310,11 @@ void __drmm_mutex_release(struct drm_device *dev, void *res) > mutex_destroy(lock); > } > EXPORT_SYMBOL(__drmm_mutex_release); > + > +void __drmm_destroy_workqueue(struct drm_device *device, void *res) > +{ > + struct workqueue_struct *wq = res; > + > + destroy_workqueue(wq); > +} > +EXPORT_SYMBOL(__drmm_destroy_workqueue); > diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h > index f547b09ca0239dd7c4fb734038bae4438321395c..96d9b0e811675fe66d5d48f02b17bc7de2ac437e 100644 > --- a/include/drm/drm_managed.h > +++ b/include/drm/drm_managed.h > @@ -127,4 +127,16 @@ void __drmm_mutex_release(struct drm_device *dev, void *res); > drmm_add_action_or_reset(dev, __drmm_mutex_release, lock); \ > }) \ > > +void __drmm_destroy_workqueue(struct drm_device *device, void *wq); A comment on the naming. This is the release helper for the workqueue. Rather call it __drm_workqueue_release(). The current name sounds like it is a drmm wrapper around destroy_workqueue(), which it isn't. Apart from that, Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> Best regards Thomas > + > +#define drmm_alloc_ordered_workqueue(dev, fmt, flags, args...) \ > + ({ \ > + struct workqueue_struct *wq = alloc_ordered_workqueue(fmt, flags, ##args); \ > + wq ? ({ \ > + int ret = drmm_add_action_or_reset(dev, __drmm_destroy_workqueue, wq); \ > + ret ? ERR_PTR(ret) : wq; \ > + }) : \ > + wq; \ > + }) > + > #endif >
On Tue, Jan 14, 2025 at 03:05:45PM +0100, Louis Chauvet wrote: > Add drmm_alloc_ordered_workqueue(), a helper that provides managed ordered > workqueue cleanup. The workqueue will be destroyed with the final > reference of the DRM device. > > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> > > --- > > Hi Thomas, > > I noted that you wanted to mark this as Reviewed, but as this was not > the complete patch, I prefered to have a confirmation before merging the > patch. > > Thanks for your time, > Louis Chauvet > --- > drivers/gpu/drm/drm_managed.c | 8 ++++++++ > include/drm/drm_managed.h | 12 ++++++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c > index 79ce86a5bd67a7201f5d60550a12364f2628e0ec..1589c28410f23b5a1e93dd1638420b3ae5c4b854 100644 > --- a/drivers/gpu/drm/drm_managed.c > +++ b/drivers/gpu/drm/drm_managed.c > @@ -310,3 +310,11 @@ void __drmm_mutex_release(struct drm_device *dev, void *res) > mutex_destroy(lock); > } > EXPORT_SYMBOL(__drmm_mutex_release); > + > +void __drmm_destroy_workqueue(struct drm_device *device, void *res) > +{ > + struct workqueue_struct *wq = res; > + > + destroy_workqueue(wq); > +} > +EXPORT_SYMBOL(__drmm_destroy_workqueue); > diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h > index f547b09ca0239dd7c4fb734038bae4438321395c..96d9b0e811675fe66d5d48f02b17bc7de2ac437e 100644 > --- a/include/drm/drm_managed.h > +++ b/include/drm/drm_managed.h > @@ -127,4 +127,16 @@ void __drmm_mutex_release(struct drm_device *dev, void *res); > drmm_add_action_or_reset(dev, __drmm_mutex_release, lock); \ > }) \ > > +void __drmm_destroy_workqueue(struct drm_device *device, void *wq); > + > +#define drmm_alloc_ordered_workqueue(dev, fmt, flags, args...) \ > + ({ \ > + struct workqueue_struct *wq = alloc_ordered_workqueue(fmt, flags, ##args); \ > + wq ? ({ \ > + int ret = drmm_add_action_or_reset(dev, __drmm_destroy_workqueue, wq); \ > + ret ? ERR_PTR(ret) : wq; \ > + }) : \ > + wq; \ > + }) > + > #endif Is there a reason to do a macro and not a proper function? Maxime
On 14/01/25 - 16:29, Maxime Ripard wrote: > On Tue, Jan 14, 2025 at 03:05:45PM +0100, Louis Chauvet wrote: > > Add drmm_alloc_ordered_workqueue(), a helper that provides managed ordered > > workqueue cleanup. The workqueue will be destroyed with the final > > reference of the DRM device. > > > > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> > > > > --- > > > > Hi Thomas, > > > > I noted that you wanted to mark this as Reviewed, but as this was not > > the complete patch, I prefered to have a confirmation before merging the > > patch. > > > > Thanks for your time, > > Louis Chauvet > > --- > > drivers/gpu/drm/drm_managed.c | 8 ++++++++ > > include/drm/drm_managed.h | 12 ++++++++++++ > > 2 files changed, 20 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c > > index 79ce86a5bd67a7201f5d60550a12364f2628e0ec..1589c28410f23b5a1e93dd1638420b3ae5c4b854 100644 > > --- a/drivers/gpu/drm/drm_managed.c > > +++ b/drivers/gpu/drm/drm_managed.c > > @@ -310,3 +310,11 @@ void __drmm_mutex_release(struct drm_device *dev, void *res) > > mutex_destroy(lock); > > } > > EXPORT_SYMBOL(__drmm_mutex_release); > > + > > +void __drmm_destroy_workqueue(struct drm_device *device, void *res) > > +{ > > + struct workqueue_struct *wq = res; > > + > > + destroy_workqueue(wq); > > +} > > +EXPORT_SYMBOL(__drmm_destroy_workqueue); > > diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h > > index f547b09ca0239dd7c4fb734038bae4438321395c..96d9b0e811675fe66d5d48f02b17bc7de2ac437e 100644 > > --- a/include/drm/drm_managed.h > > +++ b/include/drm/drm_managed.h > > @@ -127,4 +127,16 @@ void __drmm_mutex_release(struct drm_device *dev, void *res); > > drmm_add_action_or_reset(dev, __drmm_mutex_release, lock); \ > > }) \ > > > > +void __drmm_destroy_workqueue(struct drm_device *device, void *wq); > > + > > +#define drmm_alloc_ordered_workqueue(dev, fmt, flags, args...) \ > > + ({ \ > > + struct workqueue_struct *wq = alloc_ordered_workqueue(fmt, flags, ##args); \ > > + wq ? ({ \ > > + int ret = drmm_add_action_or_reset(dev, __drmm_destroy_workqueue, wq); \ > > + ret ? ERR_PTR(ret) : wq; \ > > + }) : \ > > + wq; \ > > + }) > > + > > #endif > > Is there a reason to do a macro and not a proper function? I wrote a macro because creating a proper function is more complex. The function would require a va_list variant for alloc_ordered_workqueue, which does not currently exist. Louis Chauvet --- Sorry if you received this message twice. The first attempt was rejected by most of the recipients, including mailing lists. > Maxime
diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c index 79ce86a5bd67a7201f5d60550a12364f2628e0ec..1589c28410f23b5a1e93dd1638420b3ae5c4b854 100644 --- a/drivers/gpu/drm/drm_managed.c +++ b/drivers/gpu/drm/drm_managed.c @@ -310,3 +310,11 @@ void __drmm_mutex_release(struct drm_device *dev, void *res) mutex_destroy(lock); } EXPORT_SYMBOL(__drmm_mutex_release); + +void __drmm_destroy_workqueue(struct drm_device *device, void *res) +{ + struct workqueue_struct *wq = res; + + destroy_workqueue(wq); +} +EXPORT_SYMBOL(__drmm_destroy_workqueue); diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h index f547b09ca0239dd7c4fb734038bae4438321395c..96d9b0e811675fe66d5d48f02b17bc7de2ac437e 100644 --- a/include/drm/drm_managed.h +++ b/include/drm/drm_managed.h @@ -127,4 +127,16 @@ void __drmm_mutex_release(struct drm_device *dev, void *res); drmm_add_action_or_reset(dev, __drmm_mutex_release, lock); \ }) \ +void __drmm_destroy_workqueue(struct drm_device *device, void *wq); + +#define drmm_alloc_ordered_workqueue(dev, fmt, flags, args...) \ + ({ \ + struct workqueue_struct *wq = alloc_ordered_workqueue(fmt, flags, ##args); \ + wq ? ({ \ + int ret = drmm_add_action_or_reset(dev, __drmm_destroy_workqueue, wq); \ + ret ? ERR_PTR(ret) : wq; \ + }) : \ + wq; \ + }) + #endif
Add drmm_alloc_ordered_workqueue(), a helper that provides managed ordered workqueue cleanup. The workqueue will be destroyed with the final reference of the DRM device. Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> --- Hi Thomas, I noted that you wanted to mark this as Reviewed, but as this was not the complete patch, I prefered to have a confirmation before merging the patch. Thanks for your time, Louis Chauvet --- drivers/gpu/drm/drm_managed.c | 8 ++++++++ include/drm/drm_managed.h | 12 ++++++++++++ 2 files changed, 20 insertions(+)