Messages in this thread | | | Date | Tue, 10 May 2022 16:58:13 -0700 | Subject | Re: [PATCH v4] mm: fix is_pinnable_page against on cma page | From | John Hubbard <> |
| |
On 5/10/22 4:31 PM, Minchan Kim wrote: >>> + int __mt = get_pageblock_migratetype(page); >>> + int mt = __READ_ONCE(__mt); >> >> Although I saw the email discussion about this in v2, that discussion >> didn't go far enough. It started with "don't use volatile", and went >> on to "try __READ_ONCE() instead", but it should have continued on >> to "you don't need this at all". > > That's really what I want to hear from experts so wanted to learn > "Why". How could we prevent refetching of the mt if we don't use > __READ_ONCE or volatile there? > >> >> Because you don't. There is nothing you are racing with, and adding >> __READ_ONCE() in order to avoid a completely not-going-to-happen >> compiler re-invocation of a significant code block is just very wrong. >> >> So let's just let it go entirely. :) > > Yeah, once it's clear for everyone, I am happy to remove the > unnecessary lines. > >> >>> + >>> + if (mt == MIGRATE_CMA || mt == MIGRATE_ISOLATE) >>
With or without __READ_ONCE() or volatile or anything else, this code will do what you want. Which is: loosely check for either of the above.
What functional problem do you think you are preventing with __READ_ONCE()? Because I don't see one.
thanks,
-- John Hubbard NVIDIA
| |