Re: [PATCH v1 1/1] workqueue: make the increment pwq->stats[] increment atomic

[Mirsad Todorovac]

On 26. 08. 2023. 03:13, Lai Jiangshan wrote:
> On Sat, Aug 19, 2023 at 3:45 AM Mirsad Goran Todorovac
> <mirsad.todorovac@xxxxxxxxxxxx> wrote:
>>
>> KCSAN has discovered a data race in kernel/workqueue.c:2598:
>>
>> [ 1863.554079] ==================================================================
>> [ 1863.554118] BUG: KCSAN: data-race in process_one_work / process_one_work
>>
>> [ 1863.554142] write to 0xffff963d99d79998 of 8 bytes by task 5394 on cpu 27:
>> [ 1863.554154] process_one_work (kernel/workqueue.c:2598)
>> [ 1863.554166] worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2752)
>> [ 1863.554177] kthread (kernel/kthread.c:389)
>> [ 1863.554186] ret_from_fork (arch/x86/kernel/process.c:145)
>> [ 1863.554197] ret_from_fork_asm (arch/x86/entry/entry_64.S:312)
>>
>> [ 1863.554213] read to 0xffff963d99d79998 of 8 bytes by task 5450 on cpu 12:
>> [ 1863.554224] process_one_work (kernel/workqueue.c:2598)
>> [ 1863.554235] worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2752)
>> [ 1863.554247] kthread (kernel/kthread.c:389)
>> [ 1863.554255] ret_from_fork (arch/x86/kernel/process.c:145)
>> [ 1863.554266] ret_from_fork_asm (arch/x86/entry/entry_64.S:312)
>>
>> [ 1863.554280] value changed: 0x0000000000001766 -> 0x000000000000176a
>>
>> [ 1863.554295] Reported by Kernel Concurrency Sanitizer on:
>> [ 1863.554303] CPU: 12 PID: 5450 Comm: kworker/u64:1 Tainted: G             L     6.5.0-rc6+ #44
>> [ 1863.554314] Hardware name: ASRock X670E PG Lightning/X670E PG Lightning, BIOS 1.21 04/26/2023
>> [ 1863.554322] Workqueue: btrfs-endio btrfs_end_bio_work [btrfs]
>> [ 1863.554941] ==================================================================
>>
>>     lockdep_invariant_state(true);
>> →   pwq->stats[PWQ_STAT_STARTED]++;
>>     trace_workqueue_execute_start(work);
>>     worker->current_func(work);
>>
>> The quick and dirty solution is to use atomic_inc():
> 
> Great thanks for reporting the problem.
> 
> IMO, the best way to fix it is just to move the line
> "pwq->stats[PWQ_STAT_STARTED]++;" up and above the line
> "raw_spin_unlock_irq(&pool->lock);"
> 
> Thanks
> Lai

Great news, no problem.

Kind regards,
Mirsad

-- 
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
 
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
The European Union

"I see something approaching fast ... Will it be friends with me?"