[PATCH 0/3] HID: Replace deprecated create_singlethread_workqueue

[PATCH 0/3] HID: Replace deprecated create_singlethread_workqueue

[George Karagounis]

Hi maintainers,

This patch series replaces the deprecated create_singlethread_workqueue()
with the more efficient alloc_ordered_workqueue() in three HID drivers.

WQ_MEM_RECLAIM is added to ensure the workqueues can process hardware
events under memory pressure.

All three patches have been compile-tested with W=1 and produce no warnings.

Best regards,
George

George Karagounis (3):
  HID: haptic: Replace create_singlethread_workqueue with
    alloc_ordered_workqueue
  HID: elo: Replace create_singlethread_workqueue with
    alloc_ordered_workqueue
  HID: logitech-hidpp: Replace create_singlethread_workqueue with
    alloc_ordered_workqueue

 drivers/hid/hid-elo.c            | 2 +-
 drivers/hid/hid-haptic.c         | 2 +-
 drivers/hid/hid-logitech-hidpp.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.56.0

[PATCH 1/3] HID: haptic: Replace create_singlethread_workqueue with alloc_ordered_workqueue

[George Karagounis]

create_singlethread_workqueue() is deprecated. Replace it with

alloc_ordered_workqueue() for the haptic workqueue. WQ_MEM_RECLAIM

is added to ensure hardware events can be processed under memory

pressure.

Signed-off-by: George Karagounis <mail@taterr.org>
---
 drivers/hid/hid-haptic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 8760eeb08b2c..2f4fcb56aba1 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -460,7 +460,7 @@ int hid_haptic_init(struct hid_device *hdev,
 	name = kmalloc(strlen(prefix) + strlen(hdev->name) + 2, GFP_KERNEL);
 	if (name) {
 		sprintf(name, "%s %s", prefix, hdev->name);
-		haptic->wq = create_singlethread_workqueue(name);
+		haptic->wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 		kfree(name);
 	}
 	if (!haptic->wq) {
-- 
2.56.0

Re: [PATCH 1/3] HID: haptic: Replace create_singlethread_workqueue with alloc_ordered_workqueue

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] HID: haptic: format string bug in hid_haptic_init() workqueue allocation
- [Medium] HID: haptic: misuse of WQ_MEM_RECLAIM in hid_haptic_init()
--

commit 7db50def49522def9f10d7cdd2d640a9ac167fa3
Author: George Karagounis <taterraster@gmail.com>

HID: haptic: Replace create_singlethread_workqueue with alloc_ordered_workqueue

This commit replaces the deprecated create_singlethread_workqueue() call in
hid_haptic_init() with alloc_ordered_workqueue() using the WQ_MEM_RECLAIM
flag.

> diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
> index 8760eeb08b2cd..2f4fcb56aba13 100644
> --- a/drivers/hid/hid-haptic.c
> +++ b/drivers/hid/hid-haptic.c
> @@ -460,7 +460,7 @@ int hid_haptic_init(struct hid_device *hdev,
>  	name = kmalloc(strlen(prefix) + strlen(hdev->name) + 2, GFP_KERNEL);
>  	if (name) {
>  		sprintf(name, "%s %s", prefix, hdev->name);
> -		haptic->wq = create_singlethread_workqueue(name);
> +		haptic->wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);

[Severity: Critical]
Does passing name directly as the first argument to alloc_ordered_workqueue()
in hid_haptic_init() introduce a format string regression when hdev->name
contains % characters?

Previously, create_singlethread_workqueue(name) expanded to:

include/linux/workqueue.h:
#define create_singlethread_workqueue(name)				\
	alloc_ordered_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, name)

With alloc_ordered_workqueue(name, WQ_MEM_RECLAIM), name is passed as the
printf format string fmt to alloc_workqueue() -> __alloc_workqueue() ->
vsnprintf(wq->name, sizeof(wq->name), fmt, args) with no variadic format
arguments. Because hdev->name comes from external HID device descriptors or
userspace via uhid, any % specifiers in hdev->name will be interpreted by
vsnprintf().

Since alloc_ordered_workqueue() accepts a format string and variadic
arguments directly, could this pass "%s %s", 0, prefix, hdev->name and avoid
the intermediate kmalloc(), sprintf(), and kfree() of name altogether?

[Severity: Medium]
Also, is WQ_MEM_RECLAIM appropriate for haptic->wq in hid_haptic_init()?

The commit message notes that WQ_MEM_RECLAIM is added to ensure hardware
events can be processed under memory pressure, but WQ_MEM_RECLAIM is intended
for workqueues depended upon during memory reclaim paths and allocates a
dedicated rescuer kthread per device.

haptic->wq only runs haptic_work_handler() -> play_effect() to transmit
force-feedback output reports via hid_hw_output_report() and
hid_hw_raw_request(), which is not part of memory reclaim. Dropping
__WQ_LEGACY while keeping WQ_MEM_RECLAIM also enables check_flush_dependency()
warnings in kernel/workqueue.c if the underlying transport flushes a
non-WQ_MEM_RECLAIM workqueue. Should the flags argument be 0 instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028111853.22429-1-mail@taterr.org?part=1

[PATCH 2/3] HID: elo: Replace create_singlethread_workqueue with alloc_ordered_workqueue

[George Karagounis]

create_singlethread_workqueue() is deprecated. Replace it with

alloc_ordered_workqueue() for the elousb workqueue. WQ_MEM_RECLAIM

is added to ensure hardware events can be processed under memory

pressure.

Signed-off-by: George Karagounis <mail@taterr.org>
---
 drivers/hid/hid-elo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-elo.c b/drivers/hid/hid-elo.c
index 86e8729e1975..72fdc073ae39 100644
--- a/drivers/hid/hid-elo.c
+++ b/drivers/hid/hid-elo.c
@@ -296,7 +296,7 @@ static int __init elo_driver_init(void)
 {
 	int ret;
 
-	wq = create_singlethread_workqueue("elousb");
+	wq = alloc_ordered_workqueue("elousb", WQ_MEM_RECLAIM);
 	if (!wq)
 		return -ENOMEM;
 
-- 
2.56.0

[PATCH 3/3] HID: logitech-hidpp: Replace create_singlethread_workqueue with alloc_ordered_workqueue

[George Karagounis]

create_singlethread_workqueue() is deprecated. Replace it with

alloc_ordered_workqueue() for the force feedback sendqueue.

WQ_MEM_RECLAIM is added to ensure hardware events can be processed

under memory pressure.

Signed-off-by: George Karagounis <mail@taterr.org>
---
 drivers/hid/hid-logitech-hidpp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 27d83b8bbaaf..4efd26450539 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2875,7 +2875,7 @@ static int hidpp_ff_init(struct hidpp_device *hidpp,
 		error = -ENOMEM;
 		goto err_free_data;
 	}
-	data->wq = create_singlethread_workqueue("hidpp-ff-sendqueue");
+	data->wq = alloc_ordered_workqueue("hidpp-ff-sendqueue", WQ_MEM_RECLAIM);
 	if (!data->wq) {
 		error = -ENOMEM;
 		goto err_free_effect_ids;
-- 
2.56.0

Re: [PATCH 3/3] HID: logitech-hidpp: Replace create_singlethread_workqueue with alloc_ordered_workqueue

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] HID: logitech-hidpp: misuse of WQ_MEM_RECLAIM in hidpp_ff_init()
--

commit 652f6e8738d9258b57261902879ab9a6f517f7aa
Author: George Karagounis <taterraster@gmail.com>

HID: logitech-hidpp: Replace create_singlethread_workqueue with alloc_ordered_workqueue

This commit replaces create_singlethread_workqueue() with
alloc_ordered_workqueue() using the WQ_MEM_RECLAIM flag for the
force-feedback sendqueue in hidpp_ff_init().

> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 27d83b8bbaaf3..4efd26450539c 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -2875,7 +2875,7 @@ static int hidpp_ff_init(struct hidpp_device *hidpp,
>  		error = -ENOMEM;
>  		goto err_free_data;
>  	}
> -	data->wq = create_singlethread_workqueue("hidpp-ff-sendqueue");
> +	data->wq = alloc_ordered_workqueue("hidpp-ff-sendqueue", WQ_MEM_RECLAIM);

[Severity: Medium]
Should the flags argument to alloc_ordered_workqueue() in hidpp_ff_init()
be 0 instead of WQ_MEM_RECLAIM?

Per Documentation/core-api/workqueue.rst, WQ_MEM_RECLAIM is intended only
for workqueues that may be depended upon during memory reclaim.
create_singlethread_workqueue() was deprecated because it unconditionally
set __WQ_LEGACY | WQ_MEM_RECLAIM even on workqueues outside the reclaim
path.

Passing WQ_MEM_RECLAIM to alloc_ordered_workqueue() drops __WQ_LEGACY while
keeping WQ_MEM_RECLAIM. This spawns an unnecessary rescuer kthread via
init_rescuer() and enables check_flush_dependency() warnings:

kernel/workqueue.c:check_flush_dependency() {
	...
	WARN_ONCE(worker && worker->current_pwq && ((worker->current_pwq->wq->flags &
			      (WQ_MEM_RECLAIM | __WQ_LEGACY)) == WQ_MEM_RECLAIM),
		  "workqueue: WQ_MEM_RECLAIM %s:%ps is flushing !WQ_MEM_RECLAIM %s:%ps",
	...
}

Furthermore, the commit message states that WQ_MEM_RECLAIM is added to
ensure hardware events can be processed under memory pressure, but
hidpp-ff-sendqueue only sends outgoing force-feedback commands and relies
on GFP_KERNEL allocations both when queueing work in hidpp_ff_queue_work():

drivers/hid/hid-logitech-hidpp.c:hidpp_ff_queue_work() {
	struct hidpp_ff_work_data *wd = kzalloc_obj(*wd);
	...
}

and when executing work via
hidpp_ff_work_handler() -> hidpp_send_fap_command_sync():

drivers/hid/hid-logitech-hidpp.c:hidpp_send_fap_command_sync() {
	struct hidpp_report *message;
	int ret;

	message = kzalloc_obj(struct hidpp_report);
	if (!message)
		return -ENOMEM;
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028111853.22429-1-mail@taterr.org?part=3