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

[sashiko-bot@kernel.org]

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