Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drgn.helpers.linux.mm: Incorporate the use of pfn_mapped[] in for_each_page() #228

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aarontomlin
Copy link

Modify the for_each_page() helper to consider both pfn_mapped[] and nr_pfn_mapped respectively. Therefore only valid kernel mapped pages are returned.

Signed-off-by: Aaron Tomlin [email protected]

…h_page()

Modify the for_each_page() helper to consider both pfn_mapped[]
and nr_pfn_mapped respectively. Therefore only valid kernel
mapped pages are returned.

Signed-off-by: Aaron Tomlin <[email protected]>
osandov added a commit that referenced this pull request Dec 15, 2022
for_each_page() can return offline pages that don't actually exist.
Fixing this is difficult (see #228), but working around it by catching
FaultError is easy. Document the recommended usage.

Signed-off-by: Omar Sandoval <[email protected]>
@osandov
Copy link
Owner

osandov commented Dec 15, 2022

for_each_page() returning bogus pages has definitely been a pain point, thanks for looking at this! However, it looks like pfn_mapped is x86-specific. Given that drgn also fully supports ARM64 (and partially supports other architectures), I don't think we can use pfn_mapped in a generic helper like for_each_page().

I've looked into doing something based on pfn_to_online_page() in the kernel, but I couldn't find a good way to determine the sparsemem constants (PFN_SECTION_SHIFT, NR_MEM_SECTIONS, etc.) on every supported kernel version without hard coding lots of cases.

The easiest way around this right now is to catch errors from accessing the page. For example:

for page in for_each_page(prog):
    try:
        if PageLRU(page):
            print(hex(page))
    except drgn.FaultError:
        continue

I just added this to the documentation for for_each_page().

If you have ideas for how to implement a generic for_each_online_page(), I'd love to hear it.

@aarontomlin
Copy link
Author

Hi Omar,

for_each_page() returning bogus pages has definitely been a pain point,
thanks for looking at this! However, it looks like pfn_mapped is
x86-specific.

It is indeed. My understand was that drgn only supports x86.
Sorry about that.

Given that drgn also fully supports ARM64 (and partially supports other
architectures), I don't think we can use pfn_mapped in a generic helper
like for_each_page().

Yes, we cannot as per the above.

I've looked into doing something based on pfn_to_online_page() in the
kernel, but I couldn't find a good way to determine the sparsemem
constants (PFN_SECTION_SHIFT, NR_MEM_SECTIONS, etc.) on every
supported kernel version without hard coding lots of cases.

If I understand correctly, I do not think it is possible to implement a
generic for_each_page() helper. Why not incorporate an architecture
specific approach for each supported architecture?
The following is not ideal but is an example:

diff --git a/drgn/helpers/linux/mm.py b/drgn/helpers/linux/mm.py
index 5896ff9..4371a5b 100644
--- a/drgn/helpers/linux/mm.py
+++ b/drgn/helpers/linux/mm.py
@@ -10,6 +10,7 @@ Linux memory management (MM) subsystem. Only AArch64 and x86-64 are currently
 supported.
 """
 
+import platform
 import operator
 from typing import Iterator, List, Optional, Union, overload
 
@@ -695,8 +696,16 @@ def for_each_page(prog: Program) -> Iterator[Object]:
     :return: Iterator of ``struct page *`` objects.
     """
     vmemmap = prog["vmemmap"]
-    for i in range(prog["min_low_pfn"], prog["max_pfn"]):
-        yield vmemmap + i
+    if 'x86_64' or 'i386' in platform.machine():
+        pfn_mapped = prog["pfn_mapped"]
+        nr_pfn_mapped = prog["nr_pfn_mapped"]
+        for i in range(nr_pfn_mapped.value_()):
+            page = vmemmap + pfn_mapped[i].start.value_()
+            for j in range(pfn_mapped[i].start.value_(), pfn_mapped[i].end.value_() - pfn_mapped[i].start.value_()):
+                yield page + j
+    else:
+        for i in range(prog["min_low_pfn"], prog["max_pfn"]):
+            yield vmemmap + i
 
 
 @overload

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants