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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 6/10/22 13: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>

Thanks.

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans


> 
>>   .../platform/surface/surface_acpi_notify.c    | 27 ++++++++++++++++---
>>   1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/platform/surface/surface_acpi_notify.c b/drivers/platform/surface/surface_acpi_notify.c
>> index 7b758f8cc137..c0e12f0b9b79 100644
>> --- a/drivers/platform/surface/surface_acpi_notify.c
>> +++ b/drivers/platform/surface/surface_acpi_notify.c
>> @@ -37,6 +37,7 @@ struct san_data {
>>   #define to_san_data(ptr, member) \
>>       container_of(ptr, struct san_data, member)
>>   +static struct workqueue_struct *san_wq;
>>     /* -- dGPU notifier interface. ---------------------------------------------- */
>>   @@ -356,7 +357,7 @@ static u32 san_evt_bat_nf(struct ssam_event_notifier *nf,
>>         memcpy(&work->event, event, sizeof(struct ssam_event) + event->length);
>>   -    schedule_delayed_work(&work->work, delay);
>> +    queue_delayed_work(san_wq, &work->work, delay);
>>       return SSAM_NOTIF_HANDLED;
>>   }
>>   @@ -861,7 +862,7 @@ static int san_remove(struct platform_device *pdev)
>>        * We have unregistered our event sources. Now we need to ensure that
>>        * all delayed works they may have spawned are run to completion.
>>        */
>> -    flush_scheduled_work();
>> +    flush_workqueue(san_wq);
>>         return 0;
>>   }
>> @@ -881,7 +882,27 @@ static struct platform_driver surface_acpi_notify = {
>>           .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>>       },
>>   };
>> -module_platform_driver(surface_acpi_notify);
>> +
>> +static int __init san_init(void)
>> +{
>> +    int ret;
>> +
>> +    san_wq = alloc_workqueue("san_wq", 0, 0);
>> +    if (!san_wq)
>> +        return -ENOMEM;
>> +    ret = platform_driver_register(&surface_acpi_notify);
>> +    if (ret)
>> +        destroy_workqueue(san_wq);
>> +    return ret;
>> +}
>> +module_init(san_init);
>> +
>> +static void __exit san_exit(void)
>> +{
>> +    platform_driver_unregister(&surface_acpi_notify);
>> +    destroy_workqueue(san_wq);
>> +}
>> +module_exit(san_exit);
>>     MODULE_AUTHOR("Maximilian Luz <luzmaximilian@gmail.com>");
>>   MODULE_DESCRIPTION("Surface ACPI Notify driver for Surface System Aggregator Module");
>