Re: [PATCH] locking/osq_lock: annotate a data race in osq_wait_next

[Waiman Long <longman@redhat.com>]

On 6/10/26 10:08 PM, Jiayi Li wrote:
> KCSAN reported a data race in osq_unlock():
>
>    BUG: KCSAN: data-race in osq_unlock+0xa6/0x120
>
>    race at unknown origin, with read to 0xffff8e2c799b7600 of 8 bytes
>    by task 6598 on cpu 11:
>      osq_unlock+0xa6/0x120
>      generic_osq_unlock+0xe/0x20
>      rwsem_down_write_slowpath+0x78a/0xd50
>      down_write+0xef/0x100
>      vma_link_file+0x4c/0xb0
>      mmap_region+0x865/0x12a0
>      do_mmap+0x5b5/0x8d0
>
>    value changed: 0x0000000000000000 -> 0xffff8e2c796b7600
>    Reported by Kernel Concurrency Sanitizer on:
>    CPU: 11 PID: 6598 Comm: modprobe Not tainted 6.6.103+ #588
>
> static inline struct optimistic_spin_node *
> osq_wait_next(struct optimistic_spin_queue *lock,
>                struct optimistic_spin_node *node,
>                struct optimistic_spin_node *prev)
> {
>          struct optimistic_spin_node *next = NULL;
>          int curr = encode_cpu(smp_processor_id());
>          int old;
>
>          old = prev ? prev->cpu : OSQ_UNLOCKED_VAL;
>
>          for (;;) {
>
>                  ......
>
>                  if (node->next) {      <--------------------------
>                          next = xchg(&node->next, NULL);
>                          if (next)
>                                  break;
>                  }
>
>                  cpu_relax();
>          }
>
>          return next;
> }
>
> The read in osq_wait_next() only tests if node->next is non-NULL before
> using xchg() to fetch and clear the actual successor. Even if the load
> observes a transient value, the code is still working correctly. Thus,
> mark it as an intentional data race using the data_race() macro.
>
> Signed-off-by: Jiayi Li <lijiayi@kylinos.cn>
> ---
>   kernel/locking/osq_lock.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c
> index b4233dc2c2b0..ab87d06405f9 100644
> --- a/kernel/locking/osq_lock.c
> +++ b/kernel/locking/osq_lock.c
> @@ -78,7 +78,7 @@ osq_wait_next(struct optimistic_spin_queue *lock,
>   		 * wait for either @lock to point to us, through its Step-B, or
>   		 * wait for a new @node->next from its Step-C.
>   		 */
> -		if (node->next) {
> +		if (data_race(node->next)) { /* xchg() validates the value. */
>   			struct optimistic_spin_node *next;
>   
>   			next = xchg(&node->next, NULL);
Acked-by: Waiman Long <longman@redhat.com>