Messages in this thread | | | Date | Sun, 9 Feb 2020 18:20:08 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH -next v2] mm: mark an intentional data race in page_zonenum |
| |
On Thu, 6 Feb 2020 15:25:41 -0500 Qian Cai <cai@lca.pw> wrote:
> > > > On Feb 6, 2020, at 3:20 PM, Matthew Wilcox <willy@infradead.org> 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.
| |