3:28 p.m.
----- Original Message -----
search:
-f struct-page.flags-mask
When searching kernel memory, crash walks through all identity
mapping space which includes all physical memory. Nowadays we
have machines that equipped with more than 256G memory, it takes
a long time to go through all pages, especially most of them
are used by user space programs (e.g. LRU pages). This option
allows us to skip pages with certain struct page flags.
-g
Allow us to skip whole compound pages instead of only skipping
head pages when using -f, because some flags are only set to
head pages which tails pages share same properties with their
head pages.
kmem:
-k
Verify compound page order. Useful when debugging memory leak
caused by freeing compound pages with wrong order.
-r
Only print used pages (i.e. the page count is not zero). Makes
the output smaller (and faster to grep/vim/emacs).
Yu Zhao (5):
memory: better compound page support
memory: struct page.flags based filter
memory: skip compound pages when using search filter
memory: display and verify compound page order
memory: kmem option to skip free pages
Hello Yu,
OK, initially I have looked at these patches from an end-user perspective,
and will wait on reviewing the technical implementation of your patchset
until some basics below are taken care of.
Also, can you separate this into two separate patchset postings, one for
the kmem command, and the other for the search command? I understand
they both use the compound page flag changes, but the common stuff can
be put in one or the other, and it can be specified that one patch
essentially depends upon the other being in place.
First, please fix these warnings:
$ make warn
... [ cut ] ...
cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 memory.c -Wall -O2 -Wstrict-prototypes
-Wmissing-prototypes -fstack-protector -Wformat-security
memory.c: In function ‘PG_slab_flag_init’:
memory.c:4909:2: warning: pointer targets in passing argument 2 of ‘enumerator_value’
differ in signedness [-Wpointer-sign]
In file included from memory.c:19:0:
defs.h:4645:5: note: expected ‘long int *’ but argument is of type ‘ulong *’
memory.c:4910:6: warning: pointer targets in passing argument 2 of ‘enumerator_value’
differ in signedness [-Wpointer-sign]
In file included from memory.c:19:0:
defs.h:4645:5: note: expected ‘long int *’ but argument is of type ‘ulong *’
memory.c:4917:2: warning: pointer targets in passing argument 2 of ‘enumerator_value’
differ in signedness [-Wpointer-sign]
In file included from memory.c:19:0:
defs.h:4645:5: note: expected ‘long int *’ but argument is of type ‘ulong *’
memory.c:4918:13: warning: pointer targets in passing argument 2 of ‘enumerator_value’
differ in signedness [-Wpointer-sign]
In file included from memory.c:19:0:
defs.h:4645:5: note: expected ‘long int *’ but argument is of type ‘ulong *’
memory.c: In function ‘__is_page_head’:
memory.c:18332:9: warning: suggest parentheses around comparison in operand of ‘&’
[-Wparentheses]
memory.c: In function ‘is_page_head’:
memory.c:18345:9: warning: suggest parentheses around comparison in operand of ‘&’
[-Wparentheses]
memory.c: In function ‘__is_page_tail’:
memory.c:18354:9: warning: suggest parentheses around comparison in operand of ‘&’
[-Wparentheses]
memory.c: In function ‘is_page_tail’:
memory.c:18367:9: warning: suggest parentheses around comparison in operand of ‘&’
[-Wparentheses]
...
Since the kmem -k and -r flags are only relevant when used with -p,
then that should be specified clearly in the help page synopsis,
i.e., something like:
-"[-f|-F|-p|-c|-C|-i|-s|-S|-v|-V|-n|-z|-o|-h] [slab] [[-P] address]\n"
+"[-f|-F|-p [-k][-r]|-c|-C|-i|-s|-S|-v|-V|-n|-z|-o|-h] [slab] [[-P]
address]\n"
And in cmd_kmem(), any attempt to use -k or -r outside of kmem -p
should generate an error message and fail immediately. So in the case
of a specified address, put a check for the kflag and rflag right
after the for(...) line, and error(FATAL, ...) right there:
4607 for (i = 0; i < spec_addr; i++) {
4608
4609 if (Pflag)
4610 meminfo.memtype = PHYSADDR;
4611 else
4612 meminfo.memtype = IS_KVADDR(value[i]) ?
4613 KVADDR : PHYSADDR;
4614
4615 if (fflag) {
4616 meminfo.spec_addr = value[i];
4617 meminfo.flags = ADDRESS_SPECIFIED;
4618 if (meminfo.calls++)
4619 fprintf(fp, "\n");
4620 vt->dump_free_pages(&meminfo);
4621 fflag++;
4622 }
4623
4624 if (pflag) {
4625 meminfo.spec_addr = value[i];
4626 meminfo.flags = ADDRESS_SPECIFIED;
4627 dump_mem_map(&meminfo);
4628 pflag++;
4629 }
4630
And here, you should check for the invalid kflag and rflag if the pflag
is not set:
4735 if (pflag == 1) {
4736 if (kflag)
4737 meminfo.extra_flags |= VERIFY_COMPOUND_PAGES;
4738 if (rflag)
4739 meminfo.extra_flags |= SKIP_FREE_PAGES;
4740 dump_mem_map(&meminfo);
4741 }
i.e., something like:
if (pflag == 1) {
(as above)
} else if (kflag || rflag)
error(FATAL, ...)
Lastly, I'm not all that excited about the -k option. It sounds like you
have had a specific bug where the "freed with the wrong order" bug occurred,
and now you're addressing it from the crash utility after the fact. How
often does that even happen? I don't know -- others may disagree.
With respect to the search command, the new options need better documentation.
First, I don't even see -g in the synopsis? It should be contained inside
the [-f ...] block.
Then, the "-f struct-page.flags-mask" option needs to be simplified in its
name string, maybe "-f page-mask". And the "preceded by the letter n"
business
is something that's never been done by any other command, and it's just kind of
strange. Maybe there could be both an "-f page-mask" and an "-F
page-mask"?
Also, fix this:
SYNOPSIS
search [-s start] [ -[kKV] | -u | -p | -t ] [-e end | -l length] [-m mask]
[-f struct-page.flags-mask] [-x count] -[cwh] [value | (expression) |
symbol | string] ...
so that it does not exceed 80 columns.
The explanation of each option should be on the same line:
-g
Skip whole compound page instead of only skipping head pages when
using with -f. Only has effect with -f.
Is there a good reason for restricting -f to virtual addresses only?
And like cmd_kmem(), cmd_search() should have error checking, and fail if
the -g option is used without -f (or -F).
Thanks,
Dave