Re: [PATCH -next v2] locking/osq_lock: annotate a data race in osq_lock

From: Qian Cai
Date: Mon May 11 2020 - 12:44:32 EST




> On May 11, 2020, at 11:58 AM, Will Deacon <will@xxxxxxxxxx> wrote:
>
> I'm fine with the data_race() placement, but I don't find the comment
> very helpful. We assign the result of a READ_ONCE() to 'prev' in the
> loop, so I don't think that the cpu_relax() is really relevant.
>
> The reason we don't need READ_ONCE() here is because if we race with
> the writer then either we'll go round the loop again after accidentally
> thinking prev->next != node, or we'll erroneously attempt the cmpxchg()
> because we thought they were equal and that will fail.
>
> Make sense?

I think the significant concern from the previous reviews was if compilers could prove that prev->next == node was always true because it had no knowledge of the concurrency, and then took out the whole if statement away resulting in an infinite loop.

The comment tried to explain that the cpu_relax() would save us from the infinite loop in theory here.