On 14.04.23 12:19, Michal Hocko wrote:
On Fri 14-04-23 10:52:04, Mel Gorman wrote:
On Fri, Apr 14, 2023 at 10:55:04AM +0200, Michal Hocko wrote:
On Fri 14-04-23 09:22:22, Mel Gorman wrote:
[...]
+
+ /*
+ * Do not migrate huge pages that span the size of the region
+ * being allocated contiguous. e.g. Do not migrate a 1G page
+ * for a 1G allocation request. CMA is an exception as the
+ * region may be reserved for hardware that requires physical
+ * memory without a MMU or scatter/gather capability.
+ *
+ * Note that the compound check is race-prone versus
+ * free/split/collapse but it should be safe and result in
+ * a premature skip or a useless migration attempt.
+ */
+ if (PageHuge(page) && compound_nr(page) >= nr_pages &&
+ !is_migrate_cma_page(page)) {
+ return false;
Is the CMA check working as expected?
I didn't test it as I don't have a good simulator for CMA contraints which
is still a mobile phone concern for devices like cameras.
The function sounds quite generic
and I agree that it would make sense if it was generic but it is used
only for GB pages in fact and unless I am missing something it would
allow to migrate CMA pages and potentially allocate over that region
without any possibility to migrate GB page out so the CMA region would
be essentially unusable for CMA users.
It's used primarily for 1G pages but does have other users (debugging
mostly, low priority). As it's advertised as a general API, I decided to
treat it as such and that meant being nice to CMA if possible. If CMA pages
migrate but can still use the target location then it should be fine. If a
CMA can migrate to an usable location that breaks a device then that's a bug.
GB pages already have their CMA
allocator path before we get to alloc_contig_pages. Or do I miss
something?
I don't think you missed anything. The CMA check is, at best, an effort
to have a potentially useful semantic but it's very doubtful anyone will
notice or care. I'm perfectly happy just to drop the CMA check because it's a
straight-forward fix and more suitable as a -stable backport. I'm also happy
to just go with a PageHuge check and ignore any possibility that a 2M page
could be migrated to satisfy a 1G allocation. 1G allocation requests after
significant uptime is a crapshoot at best and relying on them succeeding is
unwise. There is a non-zero possibility that the latency incurred migrating
2M pages and still failing a 1G allocation could itself be classed as a
bug with users preferring fast-failure of 1G allocation attempts.
Yes, the simpler the better. If we encounter a real usecase where couple
of 2MB hugetlb pages stand in the way to GB pages then we can add the
check so I would just go with reintroducing the PageHuge check alone.
alloc_contig_pages() -> __alloc_contig_pages() ->
alloc_contig_range(MIGRATE_MOVABLE)
Should always fail when stumbling over MIGRATE_CMA pageblocks IIRC.
So we could bail out in that function early if we stumble over any CMA
region.
--
Thanks,
David / dhildenb
