On Tue, Jan 18, 2022 at 03:05:39PM -0800, Peter Collingbourne wrote:
After submitting a patch with a compare-exchange loop similar to this one to set the KASAN tag in the page flags, Andrey Konovalov pointed out that we should be using READ_ONCE() to read the page flags. Fix it here.
What does it actually fix? If it manages to split the read and read garbage the cmpxchg will fail and we go another round, no harm done.
Fixes: 75980e97dacc ("mm: fold page->_last_nid into page->flags where possible")
As per the above argument, I don't think this rates a Fixes tag, there is no actual fix.
Signed-off-by: Peter Collingbourne pcc@google.com Link: https://linux-review.googlesource.com/id/I2e1f5b5b080ac9c4e0eb7f98768dba6fd7...
That's that doing here?
Cc: stable@vger.kernel.org
That's massively over-selling things.
mm/mmzone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/mmzone.c b/mm/mmzone.c index eb89d6e018e2..f84b84b0d3fc 100644 --- a/mm/mmzone.c +++ b/mm/mmzone.c @@ -90,7 +90,7 @@ int page_cpupid_xchg_last(struct page *page, int cpupid) int last_cpupid; do {
old_flags = flags = page->flags;
old_flags = flags = READ_ONCE(page->flags);flags &= ~(LAST_CPUPID_MASK << LAST_CPUPID_PGSHIFT);
I think that if you want to touch that code, something like the below makes more sense...
diff --git a/mm/mmzone.c b/mm/mmzone.c index eb89d6e018e2..ed9f4bcdc9ee 100644 --- a/mm/mmzone.c +++ b/mm/mmzone.c @@ -89,13 +89,14 @@ int page_cpupid_xchg_last(struct page *page, int cpupid) unsigned long old_flags, flags; int last_cpupid;
+ old_flags = READ_ONCE(page->flags); do { - old_flags = flags = page->flags; - last_cpupid = page_cpupid_last(page); + flags = old_flags; + last_cpupid = (flags >> LAST_CPUPID_PGSHIFT) & LAST_CPUPID_MASK;
flags &= ~(LAST_CPUPID_MASK << LAST_CPUPID_PGSHIFT); flags |= (cpupid & LAST_CPUPID_MASK) << LAST_CPUPID_PGSHIFT; - } while (unlikely(cmpxchg(&page->flags, old_flags, flags) != old_flags)); + } while (unlikely(!try_cmpxchg(&page->flags, &old_flags, flags)));
return last_cpupid; }