[PATCH 1/2] drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()

[PATCH 1/2] drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()

[Jiasheng Jiang]

Add drmm_alloc_workqueue() and drmm_alloc_ordered_workqueue(), the helpers
that provide managed workqueue cleanup. The workqueue will be destroyed
with the final reference of the DRM device.

Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 drivers/gpu/drm/drm_managed.c | 66 +++++++++++++++++++++++++++++++++++
 include/drm/drm_managed.h     |  8 +++++
 2 files changed, 74 insertions(+)

diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index 4cf214de50c4..d3bd6247eec9 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -271,6 +271,13 @@ static void drmm_mutex_release(struct drm_device *dev, void *res)
 	mutex_destroy(lock);
 }
 
+static void drmm_destroy_workqueue(struct drm_device *dev, void *res)
+{
+	struct workqueue_struct *wq = res;
+
+	destroy_workqueue(wq);
+}
+
 /**
  * drmm_mutex_init - &drm_device-managed mutex_init()
  * @dev: DRM device
@@ -289,3 +296,62 @@ int drmm_mutex_init(struct drm_device *dev, struct mutex *lock)
 	return drmm_add_action_or_reset(dev, drmm_mutex_release, lock);
 }
 EXPORT_SYMBOL(drmm_mutex_init);
+
+/**
+ * drmm_alloc_workqueue - &drm_device-managed alloc_workqueue()
+ * @dev: DRM device
+ * @wq: workqueue to be allocated
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ *
+ * This is a &drm_device-managed version of alloc_workqueue().
+ * The initialized lock is automatically destroyed on the final
+ * drm_dev_put().
+ */
+int drmm_alloc_workqueue(struct drm_device *dev,
+			  struct workqueue_struct *wq, const char *fmt,
+			  unsigned int flags, int max_active, ...)
+{
+	va_list args;
+
+	va_start(args, max_active);
+	wq = alloc_workqueue(fmt, flags, max_active, args);
+	va_end(args);
+
+	if (!wq)
+		return -ENOMEM;
+
+	return drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
+}
+EXPORT_SYMBOL(drmm_alloc_workqueue);
+
+/**
+ * drmm_alloc_ordered_workqueue - &drm_device-managed
+ * alloc_ordered_workqueue()
+ * @dev: DRM device
+ * @wq: workqueue to be allocated
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ *
+ * This is a &drm_device-managed version of alloc_ordered_workqueue().
+ * The initialized lock is automatically destroyed on the final
+ * drm_dev_put().
+ */
+int drmm_alloc_ordered_workqueue(struct drm_device *dev,
+				  struct workqueue_struct *wq,
+				  const char *fmt, unsigned int flags, ...)
+{
+	va_list args;
+
+	va_start(args, flags);
+	wq = alloc_ordered_workqueue(fmt, flags, args);
+	va_end(args);
+
+	if (!wq)
+		return -ENOMEM;
+
+	return drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
+}
+EXPORT_SYMBOL(drmm_alloc_ordered_workqueue);
diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index 359883942612..68cecc14e1af 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -107,4 +107,12 @@ void drmm_kfree(struct drm_device *dev, void *data);
 
 int drmm_mutex_init(struct drm_device *dev, struct mutex *lock);
 
+int drmm_alloc_workqueue(struct drm_device *dev,
+			  struct workqueue_struct *wq, const char *fmt,
+			  unsigned int flags, int max_active, ...);
+
+int drmm_alloc_ordered_workqueue(struct drm_device *dev,
+				  struct workqueue_struct *wq,
+				  const char *fmt, unsigned int flags, ...);
+
 #endif
-- 
2.25.1

Re: [PATCH 1/2] drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()

[Daniel Vetter]

On Tue, Jan 10, 2023 at 11:24:47PM +0800, Jiasheng Jiang wrote:
> Add drmm_alloc_workqueue() and drmm_alloc_ordered_workqueue(), the helpers
> that provide managed workqueue cleanup. The workqueue will be destroyed
> with the final reference of the DRM device.
> 
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

Yeah I think this looks nice.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'm assuming driver maintainers will pick this up, if not please holler.

Also the threading seems broken, it's not a patch series. The b4 tool or
git send-email (of all the patches of the entire series at once, not each
individually) should get this right.

Unfortunately I did't find the right link in the kernel docs, or at least
they're not as detailed as I hoped.

Also your previous submission had iirc a bunch more patches, do you plan
to include them all in the next patch set?
-Daniel


> ---
>  drivers/gpu/drm/drm_managed.c | 66 +++++++++++++++++++++++++++++++++++
>  include/drm/drm_managed.h     |  8 +++++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> index 4cf214de50c4..d3bd6247eec9 100644
> --- a/drivers/gpu/drm/drm_managed.c
> +++ b/drivers/gpu/drm/drm_managed.c
> @@ -271,6 +271,13 @@ static void drmm_mutex_release(struct drm_device *dev, void *res)
>  	mutex_destroy(lock);
>  }
>  
> +static void drmm_destroy_workqueue(struct drm_device *dev, void *res)
> +{
> +	struct workqueue_struct *wq = res;
> +
> +	destroy_workqueue(wq);
> +}
> +
>  /**
>   * drmm_mutex_init - &drm_device-managed mutex_init()
>   * @dev: DRM device
> @@ -289,3 +296,62 @@ int drmm_mutex_init(struct drm_device *dev, struct mutex *lock)
>  	return drmm_add_action_or_reset(dev, drmm_mutex_release, lock);
>  }
>  EXPORT_SYMBOL(drmm_mutex_init);
> +
> +/**
> + * drmm_alloc_workqueue - &drm_device-managed alloc_workqueue()
> + * @dev: DRM device
> + * @wq: workqueue to be allocated
> + *
> + * Returns:
> + * 0 on success, or a negative errno code otherwise.
> + *
> + * This is a &drm_device-managed version of alloc_workqueue().
> + * The initialized lock is automatically destroyed on the final
> + * drm_dev_put().
> + */
> +int drmm_alloc_workqueue(struct drm_device *dev,
> +			  struct workqueue_struct *wq, const char *fmt,
> +			  unsigned int flags, int max_active, ...)
> +{
> +	va_list args;
> +
> +	va_start(args, max_active);
> +	wq = alloc_workqueue(fmt, flags, max_active, args);
> +	va_end(args);
> +
> +	if (!wq)
> +		return -ENOMEM;
> +
> +	return drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
> +}
> +EXPORT_SYMBOL(drmm_alloc_workqueue);
> +
> +/**
> + * drmm_alloc_ordered_workqueue - &drm_device-managed
> + * alloc_ordered_workqueue()
> + * @dev: DRM device
> + * @wq: workqueue to be allocated
> + *
> + * Returns:
> + * 0 on success, or a negative errno code otherwise.
> + *
> + * This is a &drm_device-managed version of alloc_ordered_workqueue().
> + * The initialized lock is automatically destroyed on the final
> + * drm_dev_put().
> + */
> +int drmm_alloc_ordered_workqueue(struct drm_device *dev,
> +				  struct workqueue_struct *wq,
> +				  const char *fmt, unsigned int flags, ...)
> +{
> +	va_list args;
> +
> +	va_start(args, flags);
> +	wq = alloc_ordered_workqueue(fmt, flags, args);
> +	va_end(args);
> +
> +	if (!wq)
> +		return -ENOMEM;
> +
> +	return drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
> +}
> +EXPORT_SYMBOL(drmm_alloc_ordered_workqueue);
> diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
> index 359883942612..68cecc14e1af 100644
> --- a/include/drm/drm_managed.h
> +++ b/include/drm/drm_managed.h
> @@ -107,4 +107,12 @@ void drmm_kfree(struct drm_device *dev, void *data);
>  
>  int drmm_mutex_init(struct drm_device *dev, struct mutex *lock);
>  
> +int drmm_alloc_workqueue(struct drm_device *dev,
> +			  struct workqueue_struct *wq, const char *fmt,
> +			  unsigned int flags, int max_active, ...);
> +
> +int drmm_alloc_ordered_workqueue(struct drm_device *dev,
> +				  struct workqueue_struct *wq,
> +				  const char *fmt, unsigned int flags, ...);
> +
>  #endif
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/2] drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()

[Jiasheng Jiang]

> On Tue, Jan 10, 2023 at 11:24:47PM +0800, Jiasheng Jiang wrote:
>> Add drmm_alloc_workqueue() and drmm_alloc_ordered_workqueue(), the helpers
>> that provide managed workqueue cleanup. The workqueue will be destroyed
>> with the final reference of the DRM device.
>> 
>> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> 
> Yeah I think this looks nice.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I'm assuming driver maintainers will pick this up, if not please holler.
> 
> Also the threading seems broken, it's not a patch series. The b4 tool or
> git send-email (of all the patches of the entire series at once, not each
> individually) should get this right.
> 
> Unfortunately I did't find the right link in the kernel docs, or at least
> they're not as detailed as I hoped.
> 
> Also your previous submission had iirc a bunch more patches, do you plan
> to include them all in the next patch set?

I have found that some previous patches have already been applied.
Need I just convert alloc*workqueue into drmm_alloc*workqueue and remove
the destroy_workqueue?
Or need I convert all the alloc*workqueue in the DRM?

Thanks,
Jiang

Re: [PATCH 1/2] drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()

[Jeffrey Hugo]

On 1/17/2023 8:24 PM, Jiasheng Jiang wrote:
>> On Tue, Jan 10, 2023 at 11:24:47PM +0800, Jiasheng Jiang wrote:
>>> Add drmm_alloc_workqueue() and drmm_alloc_ordered_workqueue(), the helpers
>>> that provide managed workqueue cleanup. The workqueue will be destroyed
>>> with the final reference of the DRM device.
>>>
>>> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
>>
>> Yeah I think this looks nice.
>>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> I'm assuming driver maintainers will pick this up, if not please holler.
>>
>> Also the threading seems broken, it's not a patch series. The b4 tool or
>> git send-email (of all the patches of the entire series at once, not each
>> individually) should get this right.
>>
>> Unfortunately I did't find the right link in the kernel docs, or at least
>> they're not as detailed as I hoped.
>>
>> Also your previous submission had iirc a bunch more patches, do you plan
>> to include them all in the next patch set?
> 
> I have found that some previous patches have already been applied.
> Need I just convert alloc*workqueue into drmm_alloc*workqueue and remove
> the destroy_workqueue?
> Or need I convert all the alloc*workqueue in the DRM?
> 
> Thanks,
> Jiang

Unless I'm missing something, I don't see that this ever got applied.  I 
have a use for this interface in the qaic driver.

Jiang, are you planning on posting a v2?

-Jeff

[PATCH 0/2] drm: Add DRM managed workqueues

[Jeffrey Hugo]

Based on work at https://lore.kernel.org/dri-devel/20230118032413.6496-1-jiasheng@iscas.ac.cn/

The API in the origional work seemed to have two issues:
1. The output parameter was not correctly defined
2. The allocating functions did not return the allocated object like the
other drmm functions

I tweaked the implementation to address both of these.

From what I can tell, the i915 change no longer applies to the code
base, likely due to refactoring from merging xe.  I dropped it.

Jeffrey Hugo (1):
  accel/qaic: Use drmm_alloc_workqueue()

Jiasheng Jiang (1):
  drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()

 drivers/accel/qaic/qaic_drv.c | 30 ++-----------
 drivers/gpu/drm/drm_managed.c | 82 +++++++++++++++++++++++++++++++++++
 include/drm/drm_managed.h     |  8 ++++
 3 files changed, 94 insertions(+), 26 deletions(-)

-- 
2.34.1

[PATCH 1/2] drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()

[Jeffrey Hugo]

From: Jiasheng Jiang <jiasheng@iscas.ac.cn>

Add drmm_alloc_workqueue() and drmm_alloc_ordered_workqueue(), the helpers
that provide managed workqueue cleanup. The workqueue will be destroyed
with the final reference of the DRM device.

Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
[jhugo: fix API to return the alloc'd workqueue]
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Reviewed-by: Carl Vanderlip <quic_carlv@quicinc.com>
Reviewed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
---
 drivers/gpu/drm/drm_managed.c | 82 +++++++++++++++++++++++++++++++++++
 include/drm/drm_managed.h     |  8 ++++
 2 files changed, 90 insertions(+)

diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index 7646f67bda4e..d5135a471ff2 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -310,3 +310,85 @@ void __drmm_mutex_release(struct drm_device *dev, void *res)
 	mutex_destroy(lock);
 }
 EXPORT_SYMBOL(__drmm_mutex_release);
+
+static void drmm_destroy_workqueue(struct drm_device *dev, void *res)
+{
+	struct workqueue_struct *wq = res;
+
+	destroy_workqueue(wq);
+}
+
+/**
+ * drmm_alloc_workqueue - &drm_device-managed alloc_workqueue()
+ * @dev: DRM device
+ * @wq: workqueue to be allocated
+ *
+ * Returns:
+ * Valid pointer on success, NULL on error.
+ *
+ * This is a &drm_device-managed version of alloc_workqueue().
+ * The initialized lock is automatically destroyed on the final
+ * drm_dev_put().
+ */
+struct workqueue_struct *drmm_alloc_workqueue(struct drm_device *dev,
+					      const char *fmt, unsigned int flags,
+					      int max_active, ...)
+{
+	struct workqueue_struct *wq;
+	va_list args;
+	int ret;
+
+	va_start(args, max_active);
+	wq = alloc_workqueue(fmt, flags, max_active, args);
+	va_end(args);
+
+	if (!wq)
+		return NULL;
+
+	ret = drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
+	if (ret) {
+		destroy_workqueue(wq);
+		return NULL;
+	}
+
+	return wq;
+}
+EXPORT_SYMBOL(drmm_alloc_workqueue);
+
+/**
+ * drmm_alloc_ordered_workqueue - &drm_device-managed
+ * alloc_ordered_workqueue()
+ * @dev: DRM device
+ * @wq: workqueue to be allocated
+ *
+ * Returns:
+ * Valid pointer on success, NULL on error.
+ *
+ * This is a &drm_device-managed version of alloc_ordered_workqueue().
+ * The initialized lock is automatically destroyed on the final
+ * drm_dev_put().
+ */
+struct workqueue_struct *drmm_alloc_ordered_workqueue(struct drm_device *dev,
+						      const char *fmt,
+						      unsigned int flags, ...)
+{
+	struct workqueue_struct *wq;
+	va_list args;
+	int ret;
+
+	va_start(args, flags);
+	wq = alloc_ordered_workqueue(fmt, flags, args);
+	va_end(args);
+
+	if (!wq)
+		return NULL;
+
+	ret = drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
+	if (ret) {
+		destroy_workqueue(wq);
+		return NULL;
+	}
+
+	return wq;
+}
+EXPORT_SYMBOL(drmm_alloc_ordered_workqueue);
diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index f547b09ca023..cb42fb252648 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -127,4 +127,12 @@ void __drmm_mutex_release(struct drm_device *dev, void *res);
 	drmm_add_action_or_reset(dev, __drmm_mutex_release, lock);	     \
 })									     \
 
+struct workqueue_struct *drmm_alloc_workqueue(struct drm_device *dev,
+					      const char *fmt, unsigned int flags,
+					      int max_active, ...);
+
+struct workqueue_struct *drmm_alloc_ordered_workqueue(struct drm_device *dev,
+						      const char *fmt,
+						      unsigned int flags, ...);
+
 #endif
-- 
2.34.1

Re: [PATCH 1/2] drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()

[kernel test robot]

Hi Jeffrey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.8 next-20240315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeffrey-Hugo/drm-Add-DRM-managed-alloc_workqueue-and-alloc_ordered_workqueue/20240315-225330
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20240315145034.3972749-2-quic_jhugo%40quicinc.com
patch subject: [PATCH 1/2] drm: Add DRM-managed alloc_workqueue() and alloc_ordered_workqueue()
config: parisc-defconfig (https://download.01.org/0day-ci/archive/20240316/202403160449.IaCY0Cl5-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/202403160449.IaCY0Cl5-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403160449.IaCY0Cl5-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/drm_managed.c:336: warning: Function parameter or struct member 'fmt' not described in 'drmm_alloc_workqueue'
>> drivers/gpu/drm/drm_managed.c:336: warning: Function parameter or struct member 'flags' not described in 'drmm_alloc_workqueue'
>> drivers/gpu/drm/drm_managed.c:336: warning: Function parameter or struct member 'max_active' not described in 'drmm_alloc_workqueue'
>> drivers/gpu/drm/drm_managed.c:336: warning: Excess function parameter 'wq' description in 'drmm_alloc_workqueue'
>> drivers/gpu/drm/drm_managed.c:374: warning: Function parameter or struct member 'fmt' not described in 'drmm_alloc_ordered_workqueue'
>> drivers/gpu/drm/drm_managed.c:374: warning: Function parameter or struct member 'flags' not described in 'drmm_alloc_ordered_workqueue'
>> drivers/gpu/drm/drm_managed.c:374: warning: Excess function parameter 'wq' description in 'drmm_alloc_ordered_workqueue'


vim +336 drivers/gpu/drm/drm_managed.c

   320	
   321	/**
   322	 * drmm_alloc_workqueue - &drm_device-managed alloc_workqueue()
   323	 * @dev: DRM device
   324	 * @wq: workqueue to be allocated
   325	 *
   326	 * Returns:
   327	 * Valid pointer on success, NULL on error.
   328	 *
   329	 * This is a &drm_device-managed version of alloc_workqueue().
   330	 * The initialized lock is automatically destroyed on the final
   331	 * drm_dev_put().
   332	 */
   333	struct workqueue_struct *drmm_alloc_workqueue(struct drm_device *dev,
   334						      const char *fmt, unsigned int flags,
   335						      int max_active, ...)
 > 336	{
   337		struct workqueue_struct *wq;
   338		va_list args;
   339		int ret;
   340	
   341		va_start(args, max_active);
   342		wq = alloc_workqueue(fmt, flags, max_active, args);
   343		va_end(args);
   344	
   345		if (!wq)
   346			return NULL;
   347	
   348		ret = drmm_add_action_or_reset(dev, drmm_destroy_workqueue, wq);
   349		if (ret) {
   350			destroy_workqueue(wq);
   351			return NULL;
   352		}
   353	
   354		return wq;
   355	}
   356	EXPORT_SYMBOL(drmm_alloc_workqueue);
   357	
   358	/**
   359	 * drmm_alloc_ordered_workqueue - &drm_device-managed
   360	 * alloc_ordered_workqueue()
   361	 * @dev: DRM device
   362	 * @wq: workqueue to be allocated
   363	 *
   364	 * Returns:
   365	 * Valid pointer on success, NULL on error.
   366	 *
   367	 * This is a &drm_device-managed version of alloc_ordered_workqueue().
   368	 * The initialized lock is automatically destroyed on the final
   369	 * drm_dev_put().
   370	 */
   371	struct workqueue_struct *drmm_alloc_ordered_workqueue(struct drm_device *dev,
   372							      const char *fmt,
   373							      unsigned int flags, ...)
 > 374	{

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki