Discussion:
[Crash-utility] [PATCH] Fix for "files -[cp]" options on Linux 4.17
Kazuhito Hagio
2018-09-04 15:07:09 UTC
Permalink
Since the kernel commit b93b016313b3ba8003c3b8bb71f569af91f19fc7
("page cache: use xa_lock") renamed the address_space ->page_tree
to ->i_pages, without this patch, the "files -[cp]" options don't
work on Linux 4.17 and later kernels.
(In this case, is it OK not to add a new member to offset_table?)

Also, it looks like the address_space's member that the "files -c"
option really requires is ->nrpages, not ->page_tree.

Signed-off-by: Kazuhito Hagio <k-***@ab.jp.nec.com>
---
filesys.c | 2 +-
memory.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/filesys.c b/filesys.c
index 47f5a24..527b3f6 100644
--- a/filesys.c
+++ b/filesys.c
@@ -2275,7 +2275,7 @@ cmd_files(void)
return;

case 'c':
- if (VALID_MEMBER(address_space_page_tree) &&
+ if (VALID_MEMBER(address_space_nrpages) &&
VALID_MEMBER(inode_i_mapping))
open_flags |= PRINT_NRPAGES;
else
diff --git a/memory.c b/memory.c
index 24fce5e..ea25047 100644
--- a/memory.c
+++ b/memory.c
@@ -487,6 +487,8 @@ vm_init(void)
MEMBER_OFFSET_INIT(block_device_bd_disk, "block_device", "bd_disk");
MEMBER_OFFSET_INIT(inode_i_mapping, "inode", "i_mapping");
MEMBER_OFFSET_INIT(address_space_page_tree, "address_space", "page_tree");
+ if (INVALID_MEMBER(address_space_page_tree))
+ MEMBER_OFFSET_INIT(address_space_page_tree, "address_space", "i_pages");
MEMBER_OFFSET_INIT(address_space_nrpages, "address_space", "nrpages");
if (INVALID_MEMBER(address_space_nrpages))
MEMBER_OFFSET_INIT(address_space_nrpages, "address_space", "__nrpages");
--
1.8.3.1
Dave Anderson
2018-09-04 18:47:33 UTC
Permalink
----- Original Message -----
Post by Kazuhito Hagio
Since the kernel commit b93b016313b3ba8003c3b8bb71f569af91f19fc7
("page cache: use xa_lock") renamed the address_space ->page_tree
to ->i_pages, without this patch, the "files -[cp]" options don't
work on Linux 4.17 and later kernels.
(In this case, is it OK not to add a new member to offset_table?)
Sure, that's fine.
Post by Kazuhito Hagio
Also, it looks like the address_space's member that the "files -c"
option really requires is ->nrpages, not ->page_tree.
Looks right to me.

I also noticed this is an attempt is made to dump the page cache contents
when nrpages count is 0:

crash> files -p ffff8803c8d67038
INODE NRPAGES
ffff8803c8d67038 0

files: do_radix_tree: callback operation failed: entry: 0 item: 0
crash>

So I added this:

--- filesys.c 2018-08-09 10:51:41.657042511 -0400
+++ filesys.c 2018-09-04 11:46:02.186072870 -0400
@@ -2207,6 +2207,11 @@ dump_inode_page_cache_info(ulong inode)
RJUST|LONG_DEC,
MKSTR(nrpages)));

+ FREEBUF(inode_buf);
+
+ if (!nrpages)
+ return;
+
root_rnode = i_mapping + OFFSET(address_space_page_tree);
rtp.index = 0;
rtp.value = (void *)&dump_inode_page;
@@ -2217,7 +2222,6 @@ dump_inode_page_cache_info(ulong inode)
error(INFO, "page_tree count: %ld nrpages: %ld\n",
count, nrpages);

- FREEBUF(inode_buf);
return;
}

But then again, I note that "files -p" fails quite frequently due to
some kind of mis-use of the contents of the radix tree. The 2 options
were introduced together in crash-7.1.2, but I've never really used them.

Anyway, the patch is queued for crash-7.2.4:

https://github.com/crash-utility/crash/commit/2f57a96ce27d8b121c2822de2a66c71b83bdad21

Thanks,
Dave
Post by Kazuhito Hagio
---
filesys.c | 2 +-
memory.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/filesys.c b/filesys.c
index 47f5a24..527b3f6 100644
--- a/filesys.c
+++ b/filesys.c
@@ -2275,7 +2275,7 @@ cmd_files(void)
return;
- if (VALID_MEMBER(address_space_page_tree) &&
+ if (VALID_MEMBER(address_space_nrpages) &&
VALID_MEMBER(inode_i_mapping))
open_flags |= PRINT_NRPAGES;
else
diff --git a/memory.c b/memory.c
index 24fce5e..ea25047 100644
--- a/memory.c
+++ b/memory.c
@@ -487,6 +487,8 @@ vm_init(void)
MEMBER_OFFSET_INIT(block_device_bd_disk, "block_device", "bd_disk");
MEMBER_OFFSET_INIT(inode_i_mapping, "inode", "i_mapping");
MEMBER_OFFSET_INIT(address_space_page_tree, "address_space", "page_tree");
+ if (INVALID_MEMBER(address_space_page_tree))
+ MEMBER_OFFSET_INIT(address_space_page_tree, "address_space", "i_pages");
MEMBER_OFFSET_INIT(address_space_nrpages, "address_space", "nrpages");
if (INVALID_MEMBER(address_space_nrpages))
MEMBER_OFFSET_INIT(address_space_nrpages, "address_space", "__nrpages");
--
1.8.3.1
--
Crash-utility mailing list
https://www.redhat.com/mailman/listinfo/crash-utility
Dave Anderson
2018-09-06 18:08:03 UTC
Permalink
----- Original Message -----
... [ cut ] ...
But then again, I note that "files -p" fails quite frequently due to
some kind of mis-use of the contents of the radix tree. The 2 options
were introduced together in crash-7.1.2, but I've never really used them.
FYI: I fixed do_radix_tree_dump_cb() to check for RADIX_TREE_EXCEPTIONAL_ENTRY
slot entries, and now "files -p" seems to work as expected:

https://github.com/crash-utility/crash/commit/b9df4d156838b4e8aa2b2f51aff3460a1c61f6a8

Dave

Loading...