Re: [PATCH] platform/surface: avoid flush_scheduled_work() usage

[Maximilian Luz <luzmaximilian@gmail.com>]

On 6/17/22 12:40, Tetsuo Handa wrote:
> If you are fine with per a module WQ, please apply this patch to your tree.
> 
> If you prefer per a "struct san_data" WQ like
> https://lkml.kernel.org/r/f78ddbdc-8989-a1a7-2234-ce9ec3894625@I-love.SAKURA.ne.jp
> does, you can replace this patch with yours.

I think a per-module workqueue is perfectly adequate, unless there are
some considerations I'm missing. We generally expect there to only ever
be one device instance present that this driver binds to, and even if
there were multiple instances, it's fairly low-use and we only flush in
the remove function (this is to say that I see virtually no potential
for this to live-lock or something similar).

I guess one could make the case that having it in the module as opposed
to the driver may lead to an unused workqueue if the module is loaded
(or built in) but the driver is never used. However, generally the
driver should be built as module (unless you're very specifically
targeting Surface devices) and the module is only ever loaded if the
driver will actually be used.

So, in the end, both ways should lead to virtually the same result. And
I don't really have any strong preferences either way.

> On 2022/06/10 20:29, Maximilian Luz wrote:
>> On 6/10/22 07:41, Tetsuo Handa wrote:
>>> Use local wq in order to avoid flush_scheduled_work() usage.
>>>
>>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>>> ---
>>> Please see commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue()
>>> using a macro") for background.
>>>
>>> This is a blind conversion, and is only compile tested.
>>
>> Looks good to me, thanks!
>>
>> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
>> Tested-by: Maximilian Luz <luzmaximilian@gmail.com>
>>
>>>    .../platform/surface/surface_acpi_notify.c    | 27 ++++++++++++++++---
>>>    1 file changed, 24 insertions(+), 3 deletions(-)