2024-03-25 18:30:42

by Allen Pais

[permalink] [raw]
Subject: [PATCH v2] kernel: Introduce enable_and_queue_work() convenience function

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(). A successful
enable operation means that the work item was previously disabled
and is now marked as eligible for execution. 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.

Note: This function may lead to unnecessary spurious wake-ups in cases
where the work item is expected to be dormant but enable/disable are called
frequently. Spurious wake-ups refer to the condition where worker threads
are woken up without actual work to be done. Callers should be aware of
this behavior and may need to employ additional synchronization mechanisms
to avoid these overheads if such wake-ups are not desired.

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 <[email protected]>
---
include/linux/workqueue.h | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index aedfb81f9c49..a8d8b015d729 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -678,6 +678,37 @@ 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().
+ * A successful enable operation means that the work item was previously disabled
+ * and is now marked as eligible for execution. After successfully enabling the work,
+ * the work item is then queued on the specified workqueue using queue_work().
+ * It returns %true if the work item was successfully enabled and queued,
+ * and %false otherwise.
+ *
+ * Note: Enabling and queuing work may lead to unnecessary spurious wake-ups if the
+ * work item is already queued or currently being processed. Spurious wake-ups refer
+ * to waking up worker threads without actual work to be done. Callers should be aware
+ * of this behavior and, if it is not desired, take measures to prevent it, such as
+ * checking the state of the work item before attempting to enable and queue it.
+ *
+ * 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



2024-03-25 19:43:31

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v2] kernel: Introduce enable_and_queue_work() convenience function

Hello,

The function comment felt a bit too verbose. I applied the following
compacted version to wq/for-6.10 which hopefully still contains all the
necessary information. Please let me know if you disagree.

Thanks.

--- 8< ----
From 474a549ff4c989427a14fdab851e562c8a63fe24 Mon Sep 17 00:00:00 2001
From: Allen Pais <[email protected]>
Date: Mon, 25 Mar 2024 18:02:01 +0000
Subject: [PATCH] workqueue: Introduce enable_and_queue_work() convenience
function

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(). A successful
enable operation means that the work item was previously disabled
and is now marked as eligible for execution. 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.

Note: This function may lead to unnecessary spurious wake-ups in cases
where the work item is expected to be dormant but enable/disable are called
frequently. Spurious wake-ups refer to the condition where worker threads
are woken up without actual work to be done. Callers should be aware of
this behavior and may need to employ additional synchronization mechanisms
to avoid these overheads if such wake-ups are not desired.

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.

tj: Made the function comment more compact.

Signed-off-by: Allen Pais <[email protected]>
Signed-off-by: Tejun Heo <[email protected]>
---
include/linux/workqueue.h | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index bfcf8d38f4b1..2df1188c0f96 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -682,6 +682,32 @@ 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 combines the operations of enable_work() and queue_work(),
+ * providing a convenient way to enable and queue a work item in a single call.
+ * It invokes enable_work() on @work and then queues it if the disable depth
+ * reached 0. Returns %true if the disable depth reached 0 and @work is queued,
+ * and %false otherwise.
+ *
+ * Note that @work is always queued when disable depth reaches zero. If the
+ * desired behavior is queueing only if certain events took place while @work is
+ * disabled, the user should implement the necessary state tracking and perform
+ * explicit conditional queueing after enable_work().
+ */
+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.44.0


2024-03-25 19:47:16

by Allen

[permalink] [raw]
Subject: Re: [PATCH v2] kernel: Introduce enable_and_queue_work() convenience function

> The function comment felt a bit too verbose. I applied the following
> compacted version to wq/for-6.10 which hopefully still contains all the
> necessary information. Please let me know if you disagree.
>

Looks fine to me. Thanks for fixing it.

- Allen

> Thanks.
>
> --- 8< ----
> From 474a549ff4c989427a14fdab851e562c8a63fe24 Mon Sep 17 00:00:00 2001
> From: Allen Pais <[email protected]>
> Date: Mon, 25 Mar 2024 18:02:01 +0000
> Subject: [PATCH] workqueue: Introduce enable_and_queue_work() convenience
> function
>
> 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(). A successful
> enable operation means that the work item was previously disabled
> and is now marked as eligible for execution. 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.
>
> Note: This function may lead to unnecessary spurious wake-ups in cases
> where the work item is expected to be dormant but enable/disable are called
> frequently. Spurious wake-ups refer to the condition where worker threads
> are woken up without actual work to be done. Callers should be aware of
> this behavior and may need to employ additional synchronization mechanisms
> to avoid these overheads if such wake-ups are not desired.
>
> 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.
>
> tj: Made the function comment more compact.
>
> Signed-off-by: Allen Pais <[email protected]>
> Signed-off-by: Tejun Heo <[email protected]>
> ---
> include/linux/workqueue.h | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index bfcf8d38f4b1..2df1188c0f96 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -682,6 +682,32 @@ 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 combines the operations of enable_work() and queue_work(),
> + * providing a convenient way to enable and queue a work item in a single call.
> + * It invokes enable_work() on @work and then queues it if the disable depth
> + * reached 0. Returns %true if the disable depth reached 0 and @work is queued,
> + * and %false otherwise.
> + *
> + * Note that @work is always queued when disable depth reaches zero. If the
> + * desired behavior is queueing only if certain events took place while @work is
> + * disabled, the user should implement the necessary state tracking and perform
> + * explicit conditional queueing after enable_work().
> + */
> +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.44.0
>
>


--
- Allen