10:56 a.m.
Hi,
makedumpfile may produce an incomplete dump if interrupted early or in
case if an unrecoverable I/O error happens during the execution of
makedumpfile. As of now crash begins analysis of such incomplete dump
and fails in some misleading way.
The series helps crash to avoid going too far with incomplete dumps
while provides means to localize how much data is lost in the core.
Thanks,
Roman
Roman Bolshakov (2):
diskdump: Fail readmem() early if dump is incomplete
diskdump: Print total number of dumpable pages
defs.h | 1 +
diskdump.c | 20 ++++++++++++++++----
memory.c | 7 +++++++
3 files changed, 24 insertions(+), 4 deletions(-)
--
2.31.1
10:56 a.m.
New subject: [PATCH 1/2] diskdump: Fail readmem() early if dump is incomplete
kdump format description [1] says:
[...] zero page has its own offset not equal 0. So when reading page
from incomplete core, only the page lost by ENOSPACE errors has 0 in its
corresponding page descriptor's member offset.
crash has special treatment for page descriptors with zero offset only if
DUMP_DH_COMPRESSED_INCOMPLETE is set in dump header. However,
makedumpfile places the flag after ENOSPC is hit and only if dump header
modification went without errors.
In case if crashkernel environment was terminated early (e.g. by BMC) or
some other reason, DUMP_DH_COMPRESSED_INCOMPLETE won't be set on the
dump header. Then cache_page() would be performed on pages with
pd.offset == 0 and due to pd.size == 0 it'll skip read into
compressed_page and then non related pre-existing contents of
compressed_page will copied into page cache for the non-present page.
Ultimately, it'll lead to a cryptic failure, like:
crash: invalid kernel virtual address: 72288cacacf427f8 [...]
The failure would be a bit cleaner if crash explicitly fails on the page
that is an outcome of incomplete dump:
crash: page incomplete: kernel virtual address: c000003fff9d17e8 [...]
Debugging level 8 would also produce exact offset from data_offset to
print descriptor value with ease:
read_diskdump/cache_page: descriptor with zero offset found at paddr/pfn/pos:
3fff9d0000/3fff9d/743dd
That helps in inspecting broken descriptor with hexdump or similar tools:
hexdump -s (data_offset + pos * 0x18) -n 0x18
1. https://github.com/makedumpfile/makedumpfile/blob/master/IMPLEMENTATION
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
defs.h | 1 +
diskdump.c | 13 ++++++++++---
memory.c | 7 +++++++
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/defs.h b/defs.h
index 396d61a..8418da2 100644
--- a/defs.h
+++ b/defs.h
@@ -361,6 +361,7 @@ struct number_option {
#define READ_ERROR (-2)
#define WRITE_ERROR (-3)
#define PAGE_EXCLUDED (-4)
+#define PAGE_INCOMPLETE (-5)
#define RESTART() (longjmp(pc->main_loop_env, 1))
#define RESUME_FOREACH() (longjmp(pc->foreach_loop_env, 1))
diff --git a/diskdump.c b/diskdump.c
index 3effb52..c05f1ec 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -1146,7 +1146,7 @@ cache_page(physaddr_t paddr)
if (FLAT_FORMAT()) {
if (!read_flattened_format(dd->dfd, pd.offset, dd->compressed_page, pd.size))
return READ_ERROR;
- } else if (is_incomplete_dump() && (0 == pd.offset)) {
+ } else if (0 == pd.offset) {
/*
* If the incomplete flag has been set in the header,
* first check whether zero_excluded has been set.
@@ -1158,8 +1158,15 @@ cache_page(physaddr_t paddr)
"paddr/pfn: %llx/%lx\n",
(ulonglong)paddr, pfn);
memset(dd->compressed_page, 0, dd->block_size);
- } else
- return READ_ERROR;
+ } else {
+ if (CRASHDEBUG(8))
+ fprintf(fp,
+ "read_diskdump/cache_page: "
+ "descriptor with zero offset found at "
+ "paddr/pfn/pos: %llx/%lx/%lx\n",
+ (ulonglong)paddr, pfn, desc_pos);
+ return PAGE_INCOMPLETE;
+ }
} else {
if (lseek(dd->dfd, pd.offset, SEEK_SET) == failed)
return SEEK_ERROR;
diff --git a/memory.c b/memory.c
index 8c6bbe4..5d7eee6 100644
--- a/memory.c
+++ b/memory.c
@@ -2211,6 +2211,7 @@ accessible(ulong kva)
#define READ_ERRMSG "read error: %s address: %llx type:
\"%s\"\n"
#define WRITE_ERRMSG "write error: %s address: %llx type:
\"%s\"\n"
#define PAGE_EXCLUDED_ERRMSG "page excluded: %s address: %llx type:
\"%s\"\n"
+#define PAGE_INCOMPLETE_ERRMSG "page incomplete: %s address: %llx type:
\"%s\"\n"
#define RETURN_ON_PARTIAL_READ() \
if ((error_handle & RETURN_PARTIAL) && (size < orig_size)) { \
@@ -2376,6 +2377,12 @@ readmem(ulonglong addr, int memtype, void *buffer, long size,
error(INFO, PAGE_EXCLUDED_ERRMSG, memtype_string(memtype, 0),
addr, type);
goto readmem_error;
+ case PAGE_INCOMPLETE:
+ RETURN_ON_PARTIAL_READ();
+ if (PRINT_ERROR_MESSAGE)
+ error(INFO, PAGE_INCOMPLETE_ERRMSG, memtype_string(memtype, 0), addr, type);
+ goto readmem_error;
+
default:
break;
}
--
2.31.1
10:56 a.m.
New subject: [PATCH 2/2] diskdump: Print total number of dumpable pages
It's not clear how broken an incomplete dump from the existing debugging
prints. Aggregate number of valid pages helps to figure out approximate
size of the dump. Size of a complete dump is roughly:
EXPECTED_CORE_SIZE = a few pages (kdump headers + bitmaps + descriptors) +
(total_valid_pages * block_size) * compression rate
An incomplete core would be significantly smaller than:
total_valid_pages * block_size
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
diskdump.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/diskdump.c b/diskdump.c
index c05f1ec..1428141 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -74,6 +74,7 @@ struct diskdump_data {
ulong evictions; /* total evictions done */
ulong cached_reads;
ulong *valid_pages;
+ ulong total_valid_pages; /* expected number of dumpable pages */
ulong accesses;
ulong snapshot_task;
};
@@ -877,11 +878,14 @@ restart:
}
dd->valid_pages = calloc(sizeof(ulong), max_sect_len + 1);
+ dd->total_valid_pages = 0;
for (i = 1; i < max_sect_len + 1; i++) {
dd->valid_pages[i] = dd->valid_pages[i - 1];
for (j = 0; j < BITMAP_SECT_LEN; j++, pfn++)
- if (page_is_dumpable(pfn))
+ if (page_is_dumpable(pfn)) {
dd->valid_pages[i]++;
+ dd->total_valid_pages++;
+ }
}
return TRUE;
@@ -2090,6 +2094,7 @@ __diskdump_memory_dump(FILE *fp)
else
fprintf(fp, "\n");
fprintf(fp, " valid_pages: %lx\n", (ulong)dd->valid_pages);
+ fprintf(fp, " total_valid_pages: %lx\n", dd->total_valid_pages);
return 0;
}
--
2.31.1
1:28 a.m.
New subject: [PATCH 2/2] diskdump: Print total number of dumpable pages
-----Original Message-----
It's not clear how broken an incomplete dump from the existing
debugging
prints. Aggregate number of valid pages helps to figure out approximate
size of the dump. Size of a complete dump is roughly:
EXPECTED_CORE_SIZE = a few pages (kdump headers + bitmaps + descriptors) +
(total_valid_pages * block_size) * compression rate
An incomplete core would be significantly smaller than:
total_valid_pages * block_size
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
diskdump.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/diskdump.c b/diskdump.c
index c05f1ec..1428141 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -74,6 +74,7 @@ struct diskdump_data {
ulong evictions; /* total evictions done */
ulong cached_reads;
ulong *valid_pages;
+ ulong total_valid_pages; /* expected number of dumpable pages */
ulong accesses;
ulong snapshot_task;
};
@@ -877,11 +878,14 @@ restart:
}
dd->valid_pages = calloc(sizeof(ulong), max_sect_len + 1);
+ dd->total_valid_pages = 0;
for (i = 1; i < max_sect_len + 1; i++) {
dd->valid_pages[i] = dd->valid_pages[i - 1];
for (j = 0; j < BITMAP_SECT_LEN; j++, pfn++)
- if (page_is_dumpable(pfn))
+ if (page_is_dumpable(pfn)) {
dd->valid_pages[i]++;
+ dd->total_valid_pages++;
+ }
}
return TRUE;
@@ -2090,6 +2094,7 @@ __diskdump_memory_dump(FILE *fp)
else
fprintf(fp, "\n");
fprintf(fp, " valid_pages: %lx\n", (ulong)dd->valid_pages);
+ fprintf(fp, " total_valid_pages: %lx\n", dd->total_valid_pages);
I'd like to use %ld for this, will change.
Otherwise, the 2/2 patch looks good to me.
Acked-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
Thanks,
Kazu
11:12 a.m.
New subject: [PATCH 2/2] diskdump: Print total number of dumpable pages
On Tue, Jun 08, 2021 at 06:28:51AM +0000, HAGIO KAZUHITO(萩尾 一仁) wrote:
> @@ -2090,6 +2094,7 @@ __diskdump_memory_dump(FILE *fp)
> else
> fprintf(fp, "\n");
> fprintf(fp, " valid_pages: %lx\n", (ulong)dd->valid_pages);
> + fprintf(fp, " total_valid_pages: %lx\n", dd->total_valid_pages);
I'd like to use %ld for this, will change.
Yes, it makes sense. I'll fix it in v2.
Thanks,
Roman
Otherwise, the 2/2 patch looks good to me.
Acked-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
Thanks,
Kazu
1119
days inactive
1125
days old
devel@lists.crash-utility.osci.io
4 comments
2 participants
participants (2)
-
HAGIO KAZUHITO(萩尾 一仁) -
Roman Bolshakov