Later, that code was modified (for performance reasons) in commit 0fa5bc4023c18 to do a single try_grab_compound_page() instead of multiple
calls to try_grab_page(). At the time it was not noticed that
try_grab_compound_page had a check for 'pinnable' when try_grab_page did
not. So, I think this commit actually changed the behavior.
try_grab_folio()
/*
* Can't do FOLL_LONGTERM + FOLL_PIN gup fast path if not in a
* right zone, so fail and let the caller fall back to the slow
* path.
*/
if (unlikely((flags & FOLL_LONGTERM) &&
!is_pinnable_page(page))) /* which we just changed */
return NULL;
...and now I'm starting to think that this warning might fire even with
the corrected check for MIGRATE_CMA || MIGRATE_ISOLATE. Because
try_grab_folio() didn't always have this early exit and it is starting
to look wrong.
Simply attempting to pin a non-pinnable huge page would hit this
warning. Adding additional reasons that a page is not pinnable (which
the patch does) could make this more likely to fire.
Yes, that is correct. One could easily allocate a hugetlb page from
CMA and trigger this warning.
I need to look at this a little more closely, it is making me wonder
whether the is_pinnable_page() check is a problem in this path. The
comment in try_grab_folio() indicates that the early return is a hack
(it assumes that the caller is in the gup fast path), and maybe the hack
is just wrong here--I think we're actually on the slow gup path. Not
good.
Mike, any thoughts here?
Do you know why try_grab_compound_page(now try_grab_folio) checks for
pinnable when try_grab_page does not?
Then I guess the next question is 'Should we allow pinning of hugetlb pages
in these areas?'. My first thought would be no. But, recall it was 'allowed'
until that commit which changed try_grab_page to try_grab_compound_page.
In the 'common' case of compaction, we do not attempt to migrate/move hugetlb
pages (last time I looked), so long term pinning should not be an issue.
However, for things like memory offline or alloc_contig_range() we want to
migrate hugetlb pages, so this would be an issue there.
At a minimum, I think the warning should go.