On 2022-01-19 20:06, Zi Yan wrote:
From: Zi Yan <z...@nvidia.com>

Enable set_migratetype_isolate() to check specified sub-range for
unmovable pages during isolation. Page isolation is done
at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all
pages within that granularity are intended to be isolated. For example,
alloc_contig_range(), which uses page isolation, allows ranges without
alignment. This commit makes unmovable page check only look for
interesting pages, so that page isolation can succeed for any
non-overlapping ranges.

Hi Zi Yan,

I had to re-read this several times as I found this a bit misleading.
I was mainly confused by the fact that memory_hotplug does isolation on PAGES_PER_SECTION granularity, and reading the above seems to indicate that either do it at MAX_ORDER_NR_PAGES or at pageblock_nr_pages granularity.

True is that start_isolate_page_range() expects the range to be pageblock aligned and works in pageblock_nr_pages chunks, but I do not think that is what you meant to say here.

Now, to the change itself, below:


@@ -47,8 +51,8 @@ static struct page *has_unmovable_pages(struct zone
*zone, struct page *page,
                return page;
        }

-       for (; iter < pageblock_nr_pages - offset; iter++) {
-               page = pfn_to_page(pfn + iter);
+       for (pfn = first_pfn; pfn < last_pfn; pfn++) {

You already did pfn = first_pfn before.

 /**
* start_isolate_page_range() - make page-allocation-type of range of pages to
  * be MIGRATE_ISOLATE.
- * @start_pfn:         The lower PFN of the range to be isolated.
- * @end_pfn:           The upper PFN of the range to be isolated.
+ * @start_pfn:         The lower PFN of the range to be checked for
+ *                     possibility of isolation.
+ * @end_pfn:           The upper PFN of the range to be checked for
+ *                     possibility of isolation.
+ * @isolate_start:             The lower PFN of the range to be isolated.
+ * @isolate_end:               The upper PFN of the range to be isolated.

So, what does "possibility" means here. I think this need to be clarified a bit better.

From what you pointed out in the commit message I think what you are doing is:

- alloc_contig_range() gets a range to be isolated.
- then you pass two ranges to start_isolate_page_range()
(start_pfn, end_pfn]: which is the unaligned range you got in alloc_contig_range() (isolate_start, isolate_end]: which got aligned to, let's say, to MAX_ORDER_NR_PAGES

Now, most likely, (start_pfn, end_pfn] only covers a sub-range of (isolate_start, isolate_end], and that
sub-range is what you really want to isolate (so (start_pfn, end_pfn])?

If so, should not the names be reversed?


--
Oscar Salvador
SUSE Labs
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to