On 06/10/2014 12:25 AM, David Rientjes wrote:
On Mon, 9 Jun 2014, Vlastimil Babka wrote:
Sorry, I meant ACCESS_ONCE(page_private(page)) in the migration scanner
Hm but that's breaking the abstraction of page_order(). I don't know if
it's
worse to create a new variant of page_order() or to do this. BTW, seems
like
next_active_pageblock() in memory-hotplug.c should use this variant too.
The compiler seems free to disregard the access of a volatile object above
because the return value of the inline function is unsigned long. What's
the difference between unsigned long order = page_order_unsafe(page) and
unsigned long order = (unsigned long)ACCESS_ONCE(page_private(page)) and
I think there's none functionally, but one is abstraction layer violation and
the other imply the context of usage as you say (but is that so uncommon?).
the compiler being able to reaccess page_private() because the result is
no longer volatile qualified?
You think it will reaccess? That would defeat all current ACCESS_ONCE usages,
no?
I think the compiler is allowed to turn this into
if (ACCESS_ONCE(page_private(page)) > 0 &&
ACCESS_ONCE(page_private(page)) < MAX_ORDER)
low_pfn += (1UL << ACCESS_ONCE(page_private(page))) - 1;
since the inline function has a return value of unsigned long but gcc may
not do this. I think
/*
* Big fat comment describing why we're using ACCESS_ONCE(), that
* we're ok to race, and that this is meaningful only because of
* the previous PageBuddy() check.
*/
unsigned long pageblock_order = ACCESS_ONCE(page_private(page));
is better.
I've talked about it with a gcc guy and (although he didn't actually see
the code so it might be due to me not explaining it perfectly), the
compiler will inline page_order_unsafe() so that there's effectively.
unsigned long freepage_order = ACCESS_ONCE(page_private(page));
and now it cannot just replace all freepage_order occurences with new
page_private() accesses. So thanks to the inlining, the volatile
qualification propagates to where it matters. It makes sense to me, but
if it's according to standard or gcc specific, I don't know.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>