Messages in this thread | | | Date | Mon, 11 May 2020 17:54:42 +0100 | From | Will Deacon <> | Subject | Re: [PATCH -next v2] locking/osq_lock: annotate a data race in osq_lock |
| |
On Mon, May 11, 2020 at 12:44:26PM -0400, Qian Cai wrote: > > > > On May 11, 2020, at 11:58 AM, Will Deacon <will@kernel.org> 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.
Hmm, I don't see how it can remove the cmpxchg(). Do you have a link to that discussion, please?
Will
| |