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

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

 



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?

> ---
>   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.

> +
> +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>

> +}
> +
> +static int
> +sumary_inode_page(ulong page)

Is "sumary" a typo of "summary"?  or a common word?

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.

>   "     -p inode   given a hexadecimal inode address, dump all of its pages",
>   "                that are in the page cache.",
>   "     -c         for each open file descriptor, prints a pointer to its",
> @@ -7974,6 +7976,14 @@ char *help_files[] = {
>   "    ca1ddde0  2eeef000  f59b91ac        3  2 82c referenced,uptodate,lru,private",
>   "    ca36b300  3b598000  f59b91ac        4  2 82c referenced,uptodate,lru,private",
>   "    ca202680  30134000  f59b91ac        5  2 82c referenced,uptodate,lru,private",
> +"    ",
> +"    %s> files -n ffff000118c49140",
> +"      INODE        NRPAGES",
> +" ffff000118c49140      985",
> +"    ",
> +" The physical memory in the page cache:",
> +"        Node[0]:                0 pages",
> +"        Node[1]:              985 pages",
>   " ",
>   NULL
>   };
> diff --git a/memory.c b/memory.c
> index 86ccec5..ed1a4fb 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -300,7 +300,6 @@ static int dump_vm_event_state(void);
>   static int dump_page_states(void);
>   static int generic_read_dumpfile(ulonglong, void *, long, char *, ulong);
>   static int generic_write_dumpfile(ulonglong, void *, long, char *, ulong);
> -static int page_to_nid(ulong);
>   static int get_kmem_cache_list(ulong **);
>   static int get_kmem_cache_root_list(ulong **);
>   static int get_kmem_cache_child_list(ulong **, ulong);
> @@ -19846,7 +19845,7 @@ is_kmem_cache_addr_common(ulong vaddr, char *kbuf)
>   /*
>    *  Kernel-config-neutral page-to-node evaluator.
>    */
> -static int
> +int
>   page_to_nid(ulong page)
>   {
>           int i;

Thanks,
Kazu
--
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