On 4/24/2023 5:54 PM, Michal Hocko wrote:
On Sun 23-04-23 18:59:11, Baolin Wang wrote:
Now the __pageblock_pfn_to_page() is used by set_zone_contiguous(), which
checks whether the given zone contains holes, and uses pfn_to_online_page()
to validate if the start pfn is online and valid, as well as using pfn_valid()
to validate the end pfn.
However, the __pageblock_pfn_to_page() function may return non-NULL even
if the end pfn of a pageblock is in a memory hole in some situations. For
example, if the pageblock order is MAX_ORDER, which will fall into 2
sub-sections, and the end pfn of the pageblock may be hole even though
the start pfn is online and valid.
This did not break anything until now, but the zone continuous is fragile
in this possible scenario. So as previous discussion[1], it is better to
add some comments to explain this possible issue in case there are some
future pfn walkers that rely on this.
[1] https://lore.kernel.org/all/87r0sdsmr6.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
Do I remember correctly you've had a specific configuration that would
trigger this case?
Yes, I provided an example in previous thread [2] so show the
__pageblock_pfn_to_page() is fragile in some cases.
[2]
https://lore.kernel.org/all/52dfdd2e-9c99-eac4-233e-59919a24323e@xxxxxxxxxxxxxxxxx/
Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
---
Changes from v1:
- Update the comments per Ying and Mike, thanks.
---
mm/page_alloc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6457b64fe562..9756d66f471c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1502,6 +1502,13 @@ void __free_pages_core(struct page *page, unsigned int order)
* interleaving within a single pageblock. It is therefore sufficient to check
* the first and last page of a pageblock and avoid checking each individual
* page in a pageblock.
+ *
+ * Note: the function may return non-NULL even if the end pfn of a pageblock
+ * is in a memory hole in some situations. For example, if the pageblock
+ * order is MAX_ORDER, which will fall into 2 sub-sections, and the end pfn
+ * of the pageblock may be hole even though the start pfn is online and valid.
+ * This did not break anything until now, but be careful about this possible
+ * issue when checking whether all pfns of a pageblock are valid.
It is not really clear what you should be doing (other than to be
careful which is not helpful much TBH) when you encounter this
situation. If the reality changes and this would break in the future
what would breakage look like? What should be done about that?
That depends on what the future pfn walkers do, which may access some
hole memory with zero-init page frame. For example, if checking the
__PageMovable() for a zero-init page frame, that will crash the system.
But I can not list all the possible cases.
So how about below words?
* Note: the function may return non-NULL even if the end pfn of a
pageblock
* is in a memory hole in some situations. For example, if the pageblock
* order is MAX_ORDER, which will fall into 2 sub-sections, and the end pfn
* of the pageblock may be hole even though the start pfn is online and
valid.
* This did not break anything until now, but be careful about this
possible
* issue when checking whether all pfns of a pageblock are valid, that may
* lead to accessing empty page frame, and the worst case can crash the
system.
* So you should use pfn_to_onlie_page() instead of pfn_valid() to
valid the
* pfns in a pageblock if such case happens.
