On Thu, 6 Feb 2020 15:25:41 -0500 Qian Cai <cai@xxxxxx> wrote: > > > > On Feb 6, 2020, at 3:20 PM, Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > On Thu, Feb 06, 2020 at 01:30:00PM -0500, Qian Cai wrote: > >> Both the read and write are done only with the non-exclusive mmap_sem > >> held. Since the read only check for a specific bit range (up to 3 bits) > >> in the flag but the write here never change those 3 bits, so load > >> tearing would be harmless here. Thus, just mark it as an intentional > >> data races using the data_race() macro which is designed for those > >> situations [1]. > > > > This changelog makes me think you don't really understand the situation. > > > > A page never changes its zone number. The zone number happens to be > > stored in the same word as other bits which are modified, but the zone > > number bits will never be modified by any other write. So we can accept > > a reload of the zone bits after an intervening write and we don't need > > to use READ_ONCE(). > > Maybe your explanation is better, but I did try to explain the same thing. > I’ll let Andrew to decide if he would like to update the commit log a bit > with your wording. Using data_race() here seems misleading - there is no race, but we're using data_race() to suppress a false positive warning from KCSAN, yes? That's a bit hacky. If this happens rarely then perhaps adding a suitable comment in page_zonenum() which explains this will suffice. But if we keep on abusing data_race() in this fashion then it would be better to add a new macro for this purpose.
