[PATCH] kernel: Introduce enable_and_queue_work() convenience function

[PATCH] kernel: Introduce enable_and_queue_work() convenience function

[Allen Pais]

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
+ * 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

Re: [PATCH] kernel: Introduce enable_and_queue_work() convenience function

[Tejun Heo]

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.

-- 
tejun

Re: [PATCH] kernel: Introduce enable_and_queue_work() convenience function

[Allen Pais]

>> 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

Re: [PATCH] kernel: Introduce enable_and_queue_work() convenience function

[Allen]

Tejun
> > 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.
>

 Could you please let me know once this is applied or point me to the branch.
I have all the conversion based on this.

 I have a series ready for review(a set of 10 patches):
https://github.com/allenpais/for-6.9-bh-conversions/tree/for-6.9

 A series that still needs some work(testing and cleanup):
https:/github.com/allenpais/for-6.9-bh-conversions/tree/for-6.9-wip (This
series contains changes to all the files that use tasklets in drivers/*)
I am working on cleaning it up and also testing. They should be ready
for review soon.

Thanks.

Re: [PATCH] kernel: Introduce enable_and_queue_work() convenience function

[Tejun Heo]

On Mon, Mar 11, 2024 at 03:50:54PM -0700, Allen wrote:
> Tejun
> > > 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.
> >
> 
>  Could you please let me know once this is applied or point me to the branch.
> I have all the conversion based on this.
> 
>  I have a series ready for review(a set of 10 patches):
> https://github.com/allenpais/for-6.9-bh-conversions/tree/for-6.9
> 
>  A series that still needs some work(testing and cleanup):
> https:/github.com/allenpais/for-6.9-bh-conversions/tree/for-6.9-wip (This
> series contains changes to all the files that use tasklets in drivers/*)
> I am working on cleaning it up and also testing. They should be ready
> for review soon.

Great, I'll open wq/for-6.10 branch once 6.9-rc1 is cut and apply the
patches there.

Thanks.

-- 
tejun

Re: [PATCH] kernel: Introduce enable_and_queue_work() convenience function

[Tejun Heo]

Hello,

On Wed, Mar 13, 2024 at 10:10:59AM -1000, Tejun Heo wrote:
> > > I'll apply this together with the rest of the series once v6.10-rc1 opens.
> > >
> > 
> >  Could you please let me know once this is applied or point me to the branch.
> > I have all the conversion based on this.

The enable/disable patchset is applied to wq/for-6.10. I was going to apply
the enable_and_queue_work() patch but you wrote that you were gonna update
the documentation but I can't find the updated version. Can you please send
the updated one?

Thanks.

-- 
tejun

Re: [PATCH] kernel: Introduce enable_and_queue_work() convenience function

[Allen]

> > >
> > >  Could you please let me know once this is applied or point me to the branch.
> > > I have all the conversion based on this.
>
> The enable/disable patchset is applied to wq/for-6.10. I was going to apply
> the enable_and_queue_work() patch but you wrote that you were gonna update
> the documentation but I can't find the updated version. Can you please send
> the updated one?
>

 Sure, I will send out v2 right away.

Thanks.

Re: [PATCH] kernel: Introduce enable_and_queue_work() convenience function

[Lai Jiangshan]

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
>

Re: [PATCH] kernel: Introduce enable_and_queue_work() convenience function

[Allen]

> >
> > +/**
> > + * 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
> >
>


-- 
       - Allen