Re: [PATCH] add "files -n" command for an inode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




在 2023/10/23 15:58, HAGIO KAZUHITO(萩尾 一仁) 写道:
On 2023/10/11 10:40, Huang Shijie wrote:
In the NUMA machine, it is useful to know the memory distribution of
an inode page cache:
	How many pages in the node 0?
	How many pages in the node 1?

Add "files -n" command to get the memory distribution information:
	1.) Add new argument for dump_inode_page_cache_info()
	2.) make page_to_nid() a global function.
	3.) Add sumary_inode_page() to check each page's node
	    information.
	4.) Use print_inode_sumary_info() to print the
              memory distribution information of an inode.

Signed-off-by: Huang Shijie <shijie@xxxxxxxxxxxxxxxxxxxxxx>
Thanks for the patch.

I tried it, but most of file inodes emit an error like this:

crash> files 2249 | grep messages
   81 ffff8915f73f7300 ffff8915d6bb03c0 ffff8915edb12788 REG
/var/log/messages
crash> files -n ffff8915edb12788
       INODE        NRPAGES
ffff8915edb12788      200

files: do_xarray: callback operation failed: entry: 0  item:
fffff3e1f84f8540

Am I missing anything?

Strange..

I will check the code again..


---
   defs.h    |  1 +
   filesys.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----
   help.c    | 10 ++++++++++
   memory.c  |  3 +--
   4 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/defs.h b/defs.h
index 96a7a2a..dfcc33d 100644
--- a/defs.h
+++ b/defs.h
@@ -5748,6 +5748,7 @@ int dump_inode_page(ulong);
   ulong valid_section_nr(ulong);
   void display_memory_from_file_offset(ulonglong, long, void *);
   void swap_info_init(void);
+int page_to_nid(ulong);
/*
    *  filesys.c
diff --git a/filesys.c b/filesys.c
index 1d0ee7f..3c6a6bd 100644
--- a/filesys.c
+++ b/filesys.c
@@ -49,7 +49,7 @@ static int match_file_string(char *, char *, char *);
   static ulong get_root_vfsmount(char *);
   static void check_live_arch_mismatch(void);
   static long get_inode_nrpages(ulong);
-static void dump_inode_page_cache_info(ulong);
+static void dump_inode_page_cache_info(ulong, void *callback);
#define DENTRY_CACHE (20)
   #define INODE_CACHE  (20)
@@ -2192,8 +2192,30 @@ get_inode_nrpages(ulong i_mapping)
   	return nrpages;
   }
+/* Used to collect the numa information for an inode */
+static unsigned long *numa_node;
nitpicking, but usually we use ulong for unsigned long.
no problem.
+
+static void
+print_inode_sumary_info(void)
+{
+	int i;
+
+	fprintf(fp, "The physical memory in the page cache:\n");
+	for (i = 0; i < vt->numnodes; i++)
+		fprintf(fp, "\tNode[%d]: %16ld pages\n", i, numa_node[i]);
I would prefer more crash-like output, e.g.

crash> files -n ffff8915edb12788
       INODE        NRPAGES
ffff8915edb12788      200
        NODE          PAGES
           0            150
           1             50
crash>
o

OK. I will change to this format.


+}
+
+static int
+sumary_inode_page(ulong page)
Is "sumary" a typo of "summary"?  or a common word?
it should be summary..

And a warning is observed:

filesys.c: In function 'sumary_inode_page':
filesys.c:2215:1: warning: no return statement in function returning
non-void [-Wreturn-type]
   }
   ^

(Probably no return value causes the callback failure?)

+{
+	int node = page_to_nid(page);
+
+	if (node >= 0)
+		numa_node[node]++;
+}
+
   static void
-dump_inode_page_cache_info(ulong inode)
+dump_inode_page_cache_info(ulong inode, void *callback)
   {
   	char *inode_buf;
   	ulong i_mapping, nrpages, root_rnode, xarray, count;
@@ -2236,7 +2258,7 @@ dump_inode_page_cache_info(ulong inode)
   		root_rnode = i_mapping + OFFSET(address_space_page_tree);
lp.index = 0;
-	lp.value = (void *)&dump_inode_page;
+	lp.value = callback;
if (root_rnode)
   		count = do_radix_tree(root_rnode, RADIX_TREE_DUMP_CB, &lp);
@@ -2276,7 +2298,7 @@ cmd_files(void)
           ref = NULL;
           refarg = NULL;
- while ((c = getopt(argcnt, args, "d:R:p:c")) != EOF) {
+        while ((c = getopt(argcnt, args, "d:n:R:p:c")) != EOF) {
                   switch(c)
   		{
   		case 'R':
@@ -2295,11 +2317,31 @@ cmd_files(void)
   			display_dentry_info(value);
   			return;
+ case 'n':
+			if (VALID_MEMBER(address_space_page_tree) &&
+			    VALID_MEMBER(inode_i_mapping)) {
+				value = htol(optarg, FAULT_ON_ERROR, NULL);
+
+				/* Allocate the array for this inode */
+				numa_node = malloc(sizeof(unsigned long) * vt->numnodes);
+				BZERO(numa_node, sizeof(unsigned long) * vt->numnodes);
+
+				dump_inode_page_cache_info(value, (void *)&sumary_inode_page);
+
+				/* Print out the NUMA node information for this inode */
+				print_inode_sumary_info();
+
+				free(numa_node);
+				numa_node = NULL;
+			} else
+				option_not_supported('n');
+			return;
+
   		case 'p':
   			if (VALID_MEMBER(address_space_page_tree) &&
   			    VALID_MEMBER(inode_i_mapping)) {
   				value = htol(optarg, FAULT_ON_ERROR, NULL);
-				dump_inode_page_cache_info(value);
+				dump_inode_page_cache_info(value, (void *)&dump_inode_page);
   			} else
   				option_not_supported('p');
   			return;
diff --git a/help.c b/help.c
index cc7ab20..9b70940 100644
--- a/help.c
+++ b/help.c
@@ -7863,6 +7863,8 @@ char *help_files[] = {
   "  specific, and only shows the data requested.\n",
   "     -d dentry  given a hexadecimal dentry address, display its inode,",
   "                super block, file type, and full pathname.",
+"     -n inode   given a hexadecimal inode address, check all the pages",
+"                in the page cache, and give a NUMA node distribution.",
I would prefer "display" instead of "give" in crash help.

okay..


Thanks

Huang Shijie


--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux