kernel: Introduce enable_and_queue_work() convenience function

Message ID 20240228181850.5895-1-apais@linux.microsoft.com
State New
Headers
Series kernel: Introduce enable_and_queue_work() convenience function |

Commit Message

Allen Pais Feb. 28, 2024, 6:18 p.m. UTC
  The enable_and_queue_work() function is introduced to streamline
the process of enabling and queuing a work item on a specific
workqueue. This function combines the functionalities of
enable_work() and queue_work() in a single call, providing a
concise and convenient API for enabling and queuing work items.

The function accepts a target workqueue and a work item as parameters.
It first attempts to enable the work item using enable_work(). If the
enable operation is successful, the work item is then queued on the
specified workqueue using queue_work(). The function returns true if
the work item was successfully enabled and queued, and false otherwise.

This addition aims to enhance code readability and maintainability by
providing a unified interface for the common use case of enabling and
queuing work items on a workqueue.

Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 include/linux/workqueue.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
  

Comments

Tejun Heo Feb. 28, 2024, 6:24 p.m. UTC | #1
On Wed, Feb 28, 2024 at 06:18:50PM +0000, Allen Pais wrote:
> The enable_and_queue_work() function is introduced to streamline
> the process of enabling and queuing a work item on a specific
> workqueue. This function combines the functionalities of
> enable_work() and queue_work() in a single call, providing a
> concise and convenient API for enabling and queuing work items.
> 
> The function accepts a target workqueue and a work item as parameters.
> It first attempts to enable the work item using enable_work(). If the
> enable operation is successful, the work item is then queued on the
> specified workqueue using queue_work(). The function returns true if
> the work item was successfully enabled and queued, and false otherwise.
> 
> This addition aims to enhance code readability and maintainability by
> providing a unified interface for the common use case of enabling and
> queuing work items on a workqueue.
> 
> Signed-off-by: Allen Pais <allen.lkml@gmail.com>

I'll apply this together with the rest of the series once v6.10-rc1 opens.

Thanks.
  
Allen Pais Feb. 28, 2024, 6:29 p.m. UTC | #2
>> The enable_and_queue_work() function is introduced to streamline
>> the process of enabling and queuing a work item on a specific
>> workqueue. This function combines the functionalities of
>> enable_work() and queue_work() in a single call, providing a
>> concise and convenient API for enabling and queuing work items.
>> 
>> The function accepts a target workqueue and a work item as parameters.
>> It first attempts to enable the work item using enable_work(). If the
>> enable operation is successful, the work item is then queued on the
>> specified workqueue using queue_work(). The function returns true if
>> the work item was successfully enabled and queued, and false otherwise.
>> 
>> This addition aims to enhance code readability and maintainability by
>> providing a unified interface for the common use case of enabling and
>> queuing work items on a workqueue.
>> 
>> Signed-off-by: Allen Pais <allen.lkml@gmail.com>
> 
> I'll apply this together with the rest of the series once v6.10-rc1 opens.

 Thank you. Meanwhile, I will go ahead and prepare the conversion with
Based on this change.

Thanks.
> 
> Thanks.
> 
> -- 
> tejun
  
Lai Jiangshan Feb. 29, 2024, 2:30 a.m. UTC | #3
On Thu, Feb 29, 2024 at 2:18 AM Allen Pais <apais@linux.microsoft.com> wrote:
>
> The enable_and_queue_work() function is introduced to streamline
> the process of enabling and queuing a work item on a specific
> workqueue. This function combines the functionalities of
> enable_work() and queue_work() in a single call, providing a
> concise and convenient API for enabling and queuing work items.
>
> The function accepts a target workqueue and a work item as parameters.
> It first attempts to enable the work item using enable_work(). If the
> enable operation is successful, the work item is then queued on the
> specified workqueue using queue_work(). The function returns true if
> the work item was successfully enabled and queued, and false otherwise.
>
> This addition aims to enhance code readability and maintainability by
> providing a unified interface for the common use case of enabling and
> queuing work items on a workqueue.
>
> Signed-off-by: Allen Pais <allen.lkml@gmail.com>
> ---
>  include/linux/workqueue.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index aedfb81f9c49..31bbd38ef8c8 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -678,6 +678,29 @@ static inline bool schedule_work(struct work_struct *work)
>         return queue_work(system_wq, work);
>  }
>
> +/**
> + * enable_and_queue_work - Enable and queue a work item on a specific workqueue
> + * @wq: The target workqueue
> + * @work: The work item to be enabled and queued
> + *
> + * This function attempts to enable the specified work item using enable_work().
> + * If the enable operation is successful, the work item is then queued on the

Could you please specify what "successful" means and also please
document it that it might cause unnecessary spurious wake-ups and
the caller should prepare for it if it is not desired.

Thanks
Lai

PS:

I'm afraid it can cause unnecessary spurious wake-ups in cases
where the work item is expected to be dormant ordinarily but disable/enable()
are called often for maintenance.  However, the user can resort to other
synchronization in this case rather than just disable/enable() only to avoid the
wake-ups overheads.



> + * provided workqueue using queue_work(). It returns %true if the work item was
> + * successfully enabled and queued, and %false otherwise.
> + *
> + * This function combines the operations of enable_work() and queue_work(),
> + * providing a convenient way to enable and queue a work item in a single call.
> + */
> +static inline bool enable_and_queue_work(struct workqueue_struct *wq,
> +                                        struct work_struct *work)
> +{
> +       if (enable_work(work)) {
> +               queue_work(wq, work);
> +               return true;
> +       }
> +       return false;
> +}
> +
>  /*
>   * Detect attempt to flush system-wide workqueues at compile time when possible.
>   * Warn attempt to flush system-wide workqueues at runtime.
> --
> 2.17.1
>
  
Allen Feb. 29, 2024, 6:24 p.m. UTC | #4
> >
> > +/**
> > + * enable_and_queue_work - Enable and queue a work item on a specific workqueue
> > + * @wq: The target workqueue
> > + * @work: The work item to be enabled and queued
> > + *
> > + * This function attempts to enable the specified work item using enable_work().
> > + * If the enable operation is successful, the work item is then queued on the
>
> Could you please specify what "successful" means and also please
> document it that it might cause unnecessary spurious wake-ups and
> the caller should prepare for it if it is not desired.

 Thank you for the review. I will work on documenting it.

Thanks.
> Thanks
> Lai
>
> PS:
>
> I'm afraid it can cause unnecessary spurious wake-ups in cases
> where the work item is expected to be dormant ordinarily but disable/enable()
> are called often for maintenance.  However, the user can resort to other
> synchronization in this case rather than just disable/enable() only to avoid the
> wake-ups overheads.
>
>
>
> > + * provided workqueue using queue_work(). It returns %true if the work item was
> > + * successfully enabled and queued, and %false otherwise.
> > + *
> > + * This function combines the operations of enable_work() and queue_work(),
> > + * providing a convenient way to enable and queue a work item in a single call.
> > + */
> > +static inline bool enable_and_queue_work(struct workqueue_struct *wq,
> > +                                        struct work_struct *work)
> > +{
> > +       if (enable_work(work)) {
> > +               queue_work(wq, work);
> > +               return true;
> > +       }
> > +       return false;
> > +}
> > +
> >  /*
> >   * Detect attempt to flush system-wide workqueues at compile time when possible.
> >   * Warn attempt to flush system-wide workqueues at runtime.
> > --
> > 2.17.1
> >
>
  

Patch

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index aedfb81f9c49..31bbd38ef8c8 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -678,6 +678,29 @@  static inline bool schedule_work(struct work_struct *work)
 	return queue_work(system_wq, work);
 }
 
+/**
+ * enable_and_queue_work - Enable and queue a work item on a specific workqueue
+ * @wq: The target workqueue
+ * @work: The work item to be enabled and queued
+ *
+ * This function attempts to enable the specified work item using enable_work().
+ * If the enable operation is successful, the work item is then queued on the
+ * provided workqueue using queue_work(). It returns %true if the work item was
+ * successfully enabled and queued, and %false otherwise.
+ *
+ * This function combines the operations of enable_work() and queue_work(),
+ * providing a convenient way to enable and queue a work item in a single call.
+ */
+static inline bool enable_and_queue_work(struct workqueue_struct *wq,
+					 struct work_struct *work)
+{
+	if (enable_work(work)) {
+		queue_work(wq, work);
+		return true;
+	}
+	return false;
+}
+
 /*
  * Detect attempt to flush system-wide workqueues at compile time when possible.
  * Warn attempt to flush system-wide workqueues at runtime.