Re: [PATCH] ath6kl: avoid flush_scheduled_work() usage

[Kalle Valo <kvalo@kernel.org>]

Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:

> On 2022/06/11 7:51, Tetsuo Handa wrote:
>> On 2022/06/11 4:10, Jeff Johnson wrote:
>>> On 6/10/2022 12:05 PM, Jeff Johnson wrote:
>>>>> +static int __init ath6kl_init(void)
>>>>> +{
>>>>> +    int ret;
>>>>> +
>>>>> +    ath6kl_wq = alloc_workqueue("ath6kl_wq", 0, 0);
>>>>> +    if (!ath6kl_wq)
>>>>> +        return -ENOMEM;
>>>>
>>>> this approach means the driver will always allocate a workqueue even if the associated hardware is never present.
>> 
>> Creating a WQ without WQ_MEM_RECLAIM flag consumes little resource.
>
> My understanding is that a loadable kernel module file is modprobe'd when
> the kernel detected a hardware and some userspace program determined a .ko
> file to use based on device IDs database). Thus, I think it is very likely
> that an associated hardware presents if module's __init function is called.
>
> If a module is built-in, that user is ready to tolerate memory footprint
> wasted by non-present hardware. And I think memory wasted by keeping an
> unused !WQ_MEM_RECLAIM workqueue is much smaller than memory wasted by
> keeping that module.
>
> Thus, I wonder who complains about creating possibly unused !WQ_MEM_RECLAIM
> workqueue, unless that module is designed for tiny embedded
> environments.

I complain, it's still wrong. We have patches which save few bytes
everywhere we can, we shouldn't deliberately increase the kernel size.

>>From 00c560307d72abffea29409328be8cd69abecc95 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Sun, 12 Jun 2022 10:38:03 +0900
> Subject: [PATCH v2] ath6kl: avoid flush_scheduled_work() usage
>
> Use local wq in order to avoid flush_scheduled_work() usage.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> Changes in v2:
>   Use per "struct ath6kl_usb" workqueue instead of per module workqueue.

Please don't embed patches into email, patchwork doesn't see those:

https://patchwork.kernel.org/project/linux-wireless/list/

Please submit v3.

> Please see commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue()
> using a macro") for background.
>
> This is a blind conversion, and is only compile tested.

This is good information to have, please include that to the commit log
so that it's stored to git.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches