* [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
@ 2020-10-12 7:09 Julien Thierry
2020-10-13 9:27 ` Bhupesh Sharma
0 siblings, 1 reply; 13+ messages in thread
From: Julien Thierry @ 2020-10-12 7:09 UTC (permalink / raw)
To: kexec; +Cc: Julien Thierry, Kazuhito Hagio
A user might want to know how much space a vmcore file will take on
the system and how much space on their disk should be available to
save it during a crash.
The option --vmcore-size does not create the vmcore file but provides
an estimation of the size of the final vmcore file created with the
same make dumpfile options.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
Cc: Kazuhito Hagio <k-hagio-ab@nec.com>
---
makedumpfile.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
makedumpfile.h | 12 +++++++
print_info.c | 4 +++
3 files changed, 111 insertions(+), 3 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index 4c4251e..0a2bfba 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -26,6 +26,7 @@
#include <limits.h>
#include <assert.h>
#include <zlib.h>
+#include <libgen.h>
struct symbol_table symbol_table;
struct size_table size_table;
@@ -1366,7 +1367,25 @@ open_dump_file(void)
if (!info->flag_force)
open_flags |= O_EXCL;
- if (info->flag_flatten) {
+ if (info->flag_vmcore_size) {
+ char *namecpy;
+ struct stat statbuf;
+ int res;
+
+ namecpy = strdup(info->name_dumpfile ?
+ info->name_dumpfile : ".");
+
+ res = stat(dirname(namecpy), &statbuf);
+ free(namecpy);
+ if (res != 0)
+ return FALSE;
+
+ fd = -1;
+ info->dumpsize_info.blksize = statbuf.st_blksize;
+ info->dumpsize_info.block_buff_size = BASE_NUM_BLOCKS;
+ info->dumpsize_info.block_info = calloc(BASE_NUM_BLOCKS, 1);
+ info->dumpsize_info.non_hole_blocks = 0;
+ } else if (info->flag_flatten) {
fd = STDOUT_FILENO;
info->name_dumpfile = filename_stdout;
} else if ((fd = open(info->name_dumpfile, open_flags,
@@ -1384,6 +1403,9 @@ check_dump_file(const char *path)
{
char *err_str;
+ if (info->flag_vmcore_size)
+ return TRUE;
+
if (access(path, F_OK) != 0)
return TRUE; /* File does not exist */
if (info->flag_force) {
@@ -4622,6 +4644,47 @@ write_and_check_space(int fd, void *buf, size_t buf_size, char *file_name)
return TRUE;
}
+static int
+write_buffer_update_size_info(off_t offset, void *buf, size_t buf_size)
+{
+ struct dumpsize_info *dumpsize_info = &info->dumpsize_info;
+ int blk_end_idx = (offset + buf_size - 1) / dumpsize_info->blksize;
+ int i;
+
+ /* Need to grow the dumpsize block buffer? */
+ if (blk_end_idx >= dumpsize_info->block_buff_size) {
+ int alloc_size = MAX(blk_end_idx - dumpsize_info->block_buff_size, BASE_NUM_BLOCKS);
+
+ dumpsize_info->block_info = realloc(dumpsize_info->block_info,
+ dumpsize_info->block_buff_size + alloc_size);
+ if (!dumpsize_info->block_info) {
+ ERRMSG("Not enough memory\n");
+ return FALSE;
+ }
+
+ memset(dumpsize_info->block_info + dumpsize_info->block_buff_size,
+ 0, alloc_size);
+ dumpsize_info->block_buff_size += alloc_size;
+ }
+
+ for (i = 0; i < buf_size; ++i) {
+ int blk_idx = (offset + i) / dumpsize_info->blksize;
+
+ if (dumpsize_info->block_info[blk_idx]) {
+ i += dumpsize_info->blksize;
+ i = i - (i % dumpsize_info->blksize) - 1;
+ continue;
+ }
+
+ if (((char *) buf)[i] != 0) {
+ dumpsize_info->non_hole_blocks++;
+ dumpsize_info->block_info[blk_idx] = 1;
+ }
+ }
+
+ return TRUE;
+}
+
int
write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
{
@@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
}
if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
return FALSE;
+ } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
+ return write_buffer_update_size_info(offset, buf, buf_size);
} else {
if (lseek(fd, offset, SEEK_SET) == failed) {
ERRMSG("Can't seek the dump file(%s). %s\n",
@@ -9018,6 +9083,12 @@ close_dump_file(void)
if (info->flag_flatten)
return;
+ if (info->flag_vmcore_size && info->fd_dumpfile == -1) {
+ free(info->dumpsize_info.block_info);
+ info->dumpsize_info.block_info = NULL;
+ return;
+ }
+
if (close(info->fd_dumpfile) < 0)
ERRMSG("Can't close the dump file(%s). %s\n",
info->name_dumpfile, strerror(errno));
@@ -10963,6 +11034,12 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
if (info->flag_flatten && info->flag_split)
return FALSE;
+ if (info->flag_flatten && info->flag_vmcore_size)
+ return FALSE;
+
+ if (info->flag_mem_usage && info->flag_vmcore_size)
+ return FALSE;
+
if (info->name_filterconfig && !info->name_vmlinux)
return FALSE;
@@ -11043,7 +11120,8 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
*/
info->name_memory = argv[optind];
- } else if ((argc == optind + 1) && info->flag_mem_usage) {
+ } else if ((argc == optind + 1) && (info->flag_mem_usage ||
+ info->flag_vmcore_size)) {
/*
* Parameter for showing the page number of memory
* in different use from.
@@ -11423,6 +11501,7 @@ static struct option longopts[] = {
{"work-dir", required_argument, NULL, OPT_WORKING_DIR},
{"num-threads", required_argument, NULL, OPT_NUM_THREADS},
{"check-params", no_argument, NULL, OPT_CHECK_PARAMS},
+ {"vmcore-size", no_argument, NULL, OPT_VMCORE_SIZE},
{0, 0, 0, 0}
};
@@ -11589,6 +11668,9 @@ main(int argc, char *argv[])
info->flag_check_params = TRUE;
message_level = DEFAULT_MSG_LEVEL;
break;
+ case OPT_VMCORE_SIZE:
+ info->flag_vmcore_size = TRUE;
+ break;
case '?':
MSG("Commandline parameter is invalid.\n");
MSG("Try `makedumpfile --help' for more information.\n");
@@ -11598,6 +11680,10 @@ main(int argc, char *argv[])
if (flag_debug)
message_level |= ML_PRINT_DEBUG_MSG;
+ if (info->flag_vmcore_size)
+ /* Suppress progress indicator as dumpfile won't get written */
+ message_level &= ~ML_PRINT_PROGRESS;
+
if (info->flag_check_params)
/* suppress debugging messages */
message_level = DEFAULT_MSG_LEVEL;
@@ -11751,7 +11837,11 @@ main(int argc, char *argv[])
goto out;
MSG("\n");
- if (info->flag_split) {
+
+ if (info->flag_vmcore_size) {
+ MSG("Estimated size to save vmcore is: %lld Bytes\n",
+ (long long)info->dumpsize_info.non_hole_blocks * info->dumpsize_info.blksize);
+ } else if (info->flag_split) {
MSG("The dumpfiles are saved to ");
for (i = 0; i < info->num_dumpfile; i++) {
if (i != (info->num_dumpfile - 1))
@@ -11808,6 +11898,8 @@ out:
free(info->page_buf);
if (info->parallel_info != NULL)
free(info->parallel_info);
+ if (info->dumpsize_info.block_info != NULL)
+ free(info->dumpsize_info.block_info);
free(info);
if (splitblock) {
diff --git a/makedumpfile.h b/makedumpfile.h
index 03fb4ce..fd78d5f 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1277,6 +1277,15 @@ struct parallel_info {
#endif
};
+#define BASE_NUM_BLOCKS 50
+
+struct dumpsize_info {
+ int blksize;
+ int block_buff_size;
+ unsigned char *block_info;
+ int non_hole_blocks;
+};
+
struct ppc64_vmemmap {
unsigned long phys;
unsigned long virt;
@@ -1321,6 +1330,7 @@ struct DumpInfo {
int flag_vmemmap; /* kernel supports vmemmap address space */
int flag_excludevm; /* -e - excluding unused vmemmap pages */
int flag_use_count; /* _refcount is named _count in struct page */
+ int flag_vmcore_size; /* estimate the size of the vmcore file instead of creating it */
unsigned long vaddr_for_vtop; /* virtual address for debugging */
long page_size; /* size of page */
long page_shift;
@@ -1425,6 +1435,7 @@ struct DumpInfo {
int num_dumpfile;
struct splitting_info *splitting_info;
struct parallel_info *parallel_info;
+ struct dumpsize_info dumpsize_info;
/*
* bitmap info:
@@ -2364,6 +2375,7 @@ struct elf_prstatus {
#define OPT_NUM_THREADS OPT_START+16
#define OPT_PARTIAL_DMESG OPT_START+17
#define OPT_CHECK_PARAMS OPT_START+18
+#define OPT_VMCORE_SIZE OPT_START+19
/*
* Function Prototype.
diff --git a/print_info.c b/print_info.c
index e0c38b4..6f5a165 100644
--- a/print_info.c
+++ b/print_info.c
@@ -308,6 +308,10 @@ print_usage(void)
MSG(" the crashkernel range, then calculates the page number of different kind per\n");
MSG(" vmcoreinfo. So currently /proc/kcore need be specified explicitly.\n");
MSG("\n");
+ MSG(" [--vmcore-size]:\n");
+ MSG(" This option provides an estimation of the size needed to save VMCORE on disk.\n");
+ MSG(" This option option cannot be used in combination with -F.\n");
+ MSG("\n");
MSG(" [-D]:\n");
MSG(" Print debugging message.\n");
MSG("\n");
--
2.25.4
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-12 7:09 [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files Julien Thierry
@ 2020-10-13 9:27 ` Bhupesh Sharma
2020-10-13 9:53 ` Julien Thierry
2020-10-14 4:31 ` lijiang
0 siblings, 2 replies; 13+ messages in thread
From: Bhupesh Sharma @ 2020-10-13 9:27 UTC (permalink / raw)
To: Julien Thierry; +Cc: kexec mailing list, Kazuhito Hagio
Hello Julien,
Thanks for the patch. Some nitpicks inline:
On Mon, Oct 12, 2020 at 12:39 PM Julien Thierry <jthierry@redhat.com> wrote:
>
> A user might want to know how much space a vmcore file will take on
> the system and how much space on their disk should be available to
> save it during a crash.
>
> The option --vmcore-size does not create the vmcore file but provides
> an estimation of the size of the final vmcore file created with the
> same make dumpfile options.
>
> Signed-off-by: Julien Thierry <jthierry@redhat.com>
> Cc: Kazuhito Hagio <k-hagio-ab@nec.com>
> ---
> makedumpfile.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
> makedumpfile.h | 12 +++++++
> print_info.c | 4 +++
> 3 files changed, 111 insertions(+), 3 deletions(-)
Please update 'makedumpfile.8' as well in v2, so that the man page can
document the newly added option and how to use it to determine the
vmcore-size.
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 4c4251e..0a2bfba 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -26,6 +26,7 @@
> #include <limits.h>
> #include <assert.h>
> #include <zlib.h>
> +#include <libgen.h>
I know we don't follow alphabetical order for include files in
makedumpfile code, but it would be good to place the new - ones
accordingly. So <libgen.h> can go with <limits.h> here.
> struct symbol_table symbol_table;
> struct size_table size_table;
> @@ -1366,7 +1367,25 @@ open_dump_file(void)
> if (!info->flag_force)
> open_flags |= O_EXCL;
>
> - if (info->flag_flatten) {
> + if (info->flag_vmcore_size) {
> + char *namecpy;
> + struct stat statbuf;
> + int res;
> +
> + namecpy = strdup(info->name_dumpfile ?
> + info->name_dumpfile : ".");
> +
> + res = stat(dirname(namecpy), &statbuf);
> + free(namecpy);
> + if (res != 0)
> + return FALSE;
> +
> + fd = -1;
> + info->dumpsize_info.blksize = statbuf.st_blksize;
> + info->dumpsize_info.block_buff_size = BASE_NUM_BLOCKS;
> + info->dumpsize_info.block_info = calloc(BASE_NUM_BLOCKS, 1);
> + info->dumpsize_info.non_hole_blocks = 0;
> + } else if (info->flag_flatten) {
> fd = STDOUT_FILENO;
> info->name_dumpfile = filename_stdout;
> } else if ((fd = open(info->name_dumpfile, open_flags,
> @@ -1384,6 +1403,9 @@ check_dump_file(const char *path)
> {
> char *err_str;
>
> + if (info->flag_vmcore_size)
> + return TRUE;
> +
> if (access(path, F_OK) != 0)
> return TRUE; /* File does not exist */
> if (info->flag_force) {
> @@ -4622,6 +4644,47 @@ write_and_check_space(int fd, void *buf, size_t buf_size, char *file_name)
> return TRUE;
> }
>
> +static int
> +write_buffer_update_size_info(off_t offset, void *buf, size_t buf_size)
> +{
> + struct dumpsize_info *dumpsize_info = &info->dumpsize_info;
> + int blk_end_idx = (offset + buf_size - 1) / dumpsize_info->blksize;
> + int i;
> +
> + /* Need to grow the dumpsize block buffer? */
> + if (blk_end_idx >= dumpsize_info->block_buff_size) {
> + int alloc_size = MAX(blk_end_idx - dumpsize_info->block_buff_size, BASE_NUM_BLOCKS);
> +
> + dumpsize_info->block_info = realloc(dumpsize_info->block_info,
> + dumpsize_info->block_buff_size + alloc_size);
> + if (!dumpsize_info->block_info) {
> + ERRMSG("Not enough memory\n");
> + return FALSE;
> + }
> +
> + memset(dumpsize_info->block_info + dumpsize_info->block_buff_size,
> + 0, alloc_size);
> + dumpsize_info->block_buff_size += alloc_size;
> + }
> +
> + for (i = 0; i < buf_size; ++i) {
> + int blk_idx = (offset + i) / dumpsize_info->blksize;
> +
> + if (dumpsize_info->block_info[blk_idx]) {
> + i += dumpsize_info->blksize;
> + i = i - (i % dumpsize_info->blksize) - 1;
> + continue;
> + }
> +
> + if (((char *) buf)[i] != 0) {
> + dumpsize_info->non_hole_blocks++;
> + dumpsize_info->block_info[blk_idx] = 1;
> + }
> + }
> +
> + return TRUE;
> +}
> +
> int
> write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
> {
> @@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
> }
> if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
> return FALSE;
> + } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
> + return write_buffer_update_size_info(offset, buf, buf_size);
> } else {
> if (lseek(fd, offset, SEEK_SET) == failed) {
> ERRMSG("Can't seek the dump file(%s). %s\n",
> @@ -9018,6 +9083,12 @@ close_dump_file(void)
> if (info->flag_flatten)
> return;
>
> + if (info->flag_vmcore_size && info->fd_dumpfile == -1) {
> + free(info->dumpsize_info.block_info);
> + info->dumpsize_info.block_info = NULL;
> + return;
> + }
> +
> if (close(info->fd_dumpfile) < 0)
> ERRMSG("Can't close the dump file(%s). %s\n",
> info->name_dumpfile, strerror(errno));
> @@ -10963,6 +11034,12 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
> if (info->flag_flatten && info->flag_split)
> return FALSE;
>
> + if (info->flag_flatten && info->flag_vmcore_size)
> + return FALSE;
> +
> + if (info->flag_mem_usage && info->flag_vmcore_size)
> + return FALSE;
> +
> if (info->name_filterconfig && !info->name_vmlinux)
> return FALSE;
>
> @@ -11043,7 +11120,8 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
> */
> info->name_memory = argv[optind];
>
> - } else if ((argc == optind + 1) && info->flag_mem_usage) {
> + } else if ((argc == optind + 1) && (info->flag_mem_usage ||
> + info->flag_vmcore_size)) {
> /*
> * Parameter for showing the page number of memory
> * in different use from.
> @@ -11423,6 +11501,7 @@ static struct option longopts[] = {
> {"work-dir", required_argument, NULL, OPT_WORKING_DIR},
> {"num-threads", required_argument, NULL, OPT_NUM_THREADS},
> {"check-params", no_argument, NULL, OPT_CHECK_PARAMS},
> + {"vmcore-size", no_argument, NULL, OPT_VMCORE_SIZE},
> {0, 0, 0, 0}
> };
>
> @@ -11589,6 +11668,9 @@ main(int argc, char *argv[])
> info->flag_check_params = TRUE;
> message_level = DEFAULT_MSG_LEVEL;
> break;
> + case OPT_VMCORE_SIZE:
> + info->flag_vmcore_size = TRUE;
> + break;
> case '?':
> MSG("Commandline parameter is invalid.\n");
> MSG("Try `makedumpfile --help' for more information.\n");
> @@ -11598,6 +11680,10 @@ main(int argc, char *argv[])
> if (flag_debug)
> message_level |= ML_PRINT_DEBUG_MSG;
>
> + if (info->flag_vmcore_size)
> + /* Suppress progress indicator as dumpfile won't get written */
> + message_level &= ~ML_PRINT_PROGRESS;
> +
> if (info->flag_check_params)
> /* suppress debugging messages */
> message_level = DEFAULT_MSG_LEVEL;
> @@ -11751,7 +11837,11 @@ main(int argc, char *argv[])
> goto out;
>
> MSG("\n");
> - if (info->flag_split) {
> +
> + if (info->flag_vmcore_size) {
> + MSG("Estimated size to save vmcore is: %lld Bytes\n",
> + (long long)info->dumpsize_info.non_hole_blocks * info->dumpsize_info.blksize);
> + } else if (info->flag_split) {
> MSG("The dumpfiles are saved to ");
> for (i = 0; i < info->num_dumpfile; i++) {
> if (i != (info->num_dumpfile - 1))
> @@ -11808,6 +11898,8 @@ out:
> free(info->page_buf);
> if (info->parallel_info != NULL)
> free(info->parallel_info);
> + if (info->dumpsize_info.block_info != NULL)
> + free(info->dumpsize_info.block_info);
> free(info);
>
> if (splitblock) {
> diff --git a/makedumpfile.h b/makedumpfile.h
> index 03fb4ce..fd78d5f 100644
> --- a/makedumpfile.h
> +++ b/makedumpfile.h
> @@ -1277,6 +1277,15 @@ struct parallel_info {
> #endif
> };
>
> +#define BASE_NUM_BLOCKS 50
> +
> +struct dumpsize_info {
> + int blksize;
> + int block_buff_size;
> + unsigned char *block_info;
> + int non_hole_blocks;
> +};
> +
> struct ppc64_vmemmap {
> unsigned long phys;
> unsigned long virt;
> @@ -1321,6 +1330,7 @@ struct DumpInfo {
> int flag_vmemmap; /* kernel supports vmemmap address space */
> int flag_excludevm; /* -e - excluding unused vmemmap pages */
> int flag_use_count; /* _refcount is named _count in struct page */
> + int flag_vmcore_size; /* estimate the size of the vmcore file instead of creating it */
> unsigned long vaddr_for_vtop; /* virtual address for debugging */
> long page_size; /* size of page */
> long page_shift;
> @@ -1425,6 +1435,7 @@ struct DumpInfo {
> int num_dumpfile;
> struct splitting_info *splitting_info;
> struct parallel_info *parallel_info;
> + struct dumpsize_info dumpsize_info;
>
> /*
> * bitmap info:
> @@ -2364,6 +2375,7 @@ struct elf_prstatus {
> #define OPT_NUM_THREADS OPT_START+16
> #define OPT_PARTIAL_DMESG OPT_START+17
> #define OPT_CHECK_PARAMS OPT_START+18
> +#define OPT_VMCORE_SIZE OPT_START+19
>
> /*
> * Function Prototype.
> diff --git a/print_info.c b/print_info.c
> index e0c38b4..6f5a165 100644
> --- a/print_info.c
> +++ b/print_info.c
> @@ -308,6 +308,10 @@ print_usage(void)
> MSG(" the crashkernel range, then calculates the page number of different kind per\n");
> MSG(" vmcoreinfo. So currently /proc/kcore need be specified explicitly.\n");
> MSG("\n");
> + MSG(" [--vmcore-size]:\n");
> + MSG(" This option provides an estimation of the size needed to save VMCORE on disk.\n");
> + MSG(" This option option cannot be used in combination with -F.\n");
Also not in combination with --mem-usage (as per the code changes above)?
And may be the options '--mem-usage / -F' also need an update to
mention they can't be used with --vmcore-size option.
> + MSG("\n");
> MSG(" [-D]:\n");
> MSG(" Print debugging message.\n");
> MSG("\n");
> --
I like the idea, but sometimes we use makedumpfile to generate a
dumpfile in the primary kernel as well. For example:
$ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile
In such use-cases it is useful to use --vmcore-size and still generate
the dumpfile (right now the default behaviour is not to generate a
dumpfile when --vmcore-size is specified). Maybe we need to think more
on supporting this use-case as well.
Regards,
Bhupesh
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-13 9:27 ` Bhupesh Sharma
@ 2020-10-13 9:53 ` Julien Thierry
2020-10-13 19:45 ` Bhupesh Sharma
2020-10-14 4:31 ` lijiang
1 sibling, 1 reply; 13+ messages in thread
From: Julien Thierry @ 2020-10-13 9:53 UTC (permalink / raw)
To: Bhupesh Sharma; +Cc: kexec mailing list, Kazuhito Hagio
Hi Bhupesh,
On 10/13/20 10:27 AM, Bhupesh Sharma wrote:
> Hello Julien,
>
> Thanks for the patch. Some nitpicks inline:
>
> On Mon, Oct 12, 2020 at 12:39 PM Julien Thierry <jthierry@redhat.com> wrote:
>>
>> A user might want to know how much space a vmcore file will take on
>> the system and how much space on their disk should be available to
>> save it during a crash.
>>
>> The option --vmcore-size does not create the vmcore file but provides
>> an estimation of the size of the final vmcore file created with the
>> same make dumpfile options.
>>
>> Signed-off-by: Julien Thierry <jthierry@redhat.com>
>> Cc: Kazuhito Hagio <k-hagio-ab@nec.com>
>> ---
>> makedumpfile.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
>> makedumpfile.h | 12 +++++++
>> print_info.c | 4 +++
>> 3 files changed, 111 insertions(+), 3 deletions(-)
>
> Please update 'makedumpfile.8' as well in v2, so that the man page can
> document the newly added option and how to use it to determine the
> vmcore-size.
>
Ah yes, I'll do that.
>> diff --git a/makedumpfile.c b/makedumpfile.c
>> index 4c4251e..0a2bfba 100644
>> --- a/makedumpfile.c
>> +++ b/makedumpfile.c
>> @@ -26,6 +26,7 @@
>> #include <limits.h>
>> #include <assert.h>
>> #include <zlib.h>
>> +#include <libgen.h>
>
> I know we don't follow alphabetical order for include files in
> makedumpfile code, but it would be good to place the new - ones
> accordingly. So <libgen.h> can go with <limits.h> here.
>
Noted.
>> struct symbol_table symbol_table;
>> struct size_table size_table;
>> @@ -1366,7 +1367,25 @@ open_dump_file(void)
>> if (!info->flag_force)
>> open_flags |= O_EXCL;
>>
>> - if (info->flag_flatten) {
>> + if (info->flag_vmcore_size) {
>> + char *namecpy;
>> + struct stat statbuf;
>> + int res;
>> +
>> + namecpy = strdup(info->name_dumpfile ?
>> + info->name_dumpfile : ".");
>> +
>> + res = stat(dirname(namecpy), &statbuf);
>> + free(namecpy);
>> + if (res != 0)
>> + return FALSE;
>> +
>> + fd = -1;
>> + info->dumpsize_info.blksize = statbuf.st_blksize;
>> + info->dumpsize_info.block_buff_size = BASE_NUM_BLOCKS;
>> + info->dumpsize_info.block_info = calloc(BASE_NUM_BLOCKS, 1);
>> + info->dumpsize_info.non_hole_blocks = 0;
>> + } else if (info->flag_flatten) {
>> fd = STDOUT_FILENO;
>> info->name_dumpfile = filename_stdout;
>> } else if ((fd = open(info->name_dumpfile, open_flags,
>> @@ -1384,6 +1403,9 @@ check_dump_file(const char *path)
>> {
>> char *err_str;
>>
>> + if (info->flag_vmcore_size)
>> + return TRUE;
>> +
>> if (access(path, F_OK) != 0)
>> return TRUE; /* File does not exist */
>> if (info->flag_force) {
>> @@ -4622,6 +4644,47 @@ write_and_check_space(int fd, void *buf, size_t buf_size, char *file_name)
>> return TRUE;
>> }
>>
>> +static int
>> +write_buffer_update_size_info(off_t offset, void *buf, size_t buf_size)
>> +{
>> + struct dumpsize_info *dumpsize_info = &info->dumpsize_info;
>> + int blk_end_idx = (offset + buf_size - 1) / dumpsize_info->blksize;
>> + int i;
>> +
>> + /* Need to grow the dumpsize block buffer? */
>> + if (blk_end_idx >= dumpsize_info->block_buff_size) {
>> + int alloc_size = MAX(blk_end_idx - dumpsize_info->block_buff_size, BASE_NUM_BLOCKS);
>> +
>> + dumpsize_info->block_info = realloc(dumpsize_info->block_info,
>> + dumpsize_info->block_buff_size + alloc_size);
>> + if (!dumpsize_info->block_info) {
>> + ERRMSG("Not enough memory\n");
>> + return FALSE;
>> + }
>> +
>> + memset(dumpsize_info->block_info + dumpsize_info->block_buff_size,
>> + 0, alloc_size);
>> + dumpsize_info->block_buff_size += alloc_size;
>> + }
>> +
>> + for (i = 0; i < buf_size; ++i) {
>> + int blk_idx = (offset + i) / dumpsize_info->blksize;
>> +
>> + if (dumpsize_info->block_info[blk_idx]) {
>> + i += dumpsize_info->blksize;
>> + i = i - (i % dumpsize_info->blksize) - 1;
>> + continue;
>> + }
>> +
>> + if (((char *) buf)[i] != 0) {
>> + dumpsize_info->non_hole_blocks++;
>> + dumpsize_info->block_info[blk_idx] = 1;
>> + }
>> + }
>> +
>> + return TRUE;
>> +}
>> +
>> int
>> write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
>> {
>> @@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
>> }
>> if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
>> return FALSE;
>> + } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
>> + return write_buffer_update_size_info(offset, buf, buf_size);
>> } else {
>> if (lseek(fd, offset, SEEK_SET) == failed) {
>> ERRMSG("Can't seek the dump file(%s). %s\n",
>> @@ -9018,6 +9083,12 @@ close_dump_file(void)
>> if (info->flag_flatten)
>> return;
>>
>> + if (info->flag_vmcore_size && info->fd_dumpfile == -1) {
>> + free(info->dumpsize_info.block_info);
>> + info->dumpsize_info.block_info = NULL;
>> + return;
>> + }
>> +
>> if (close(info->fd_dumpfile) < 0)
>> ERRMSG("Can't close the dump file(%s). %s\n",
>> info->name_dumpfile, strerror(errno));
>> @@ -10963,6 +11034,12 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
>> if (info->flag_flatten && info->flag_split)
>> return FALSE;
>>
>> + if (info->flag_flatten && info->flag_vmcore_size)
>> + return FALSE;
>> +
>> + if (info->flag_mem_usage && info->flag_vmcore_size)
>> + return FALSE;
>> +
>> if (info->name_filterconfig && !info->name_vmlinux)
>> return FALSE;
>>
>> @@ -11043,7 +11120,8 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
>> */
>> info->name_memory = argv[optind];
>>
>> - } else if ((argc == optind + 1) && info->flag_mem_usage) {
>> + } else if ((argc == optind + 1) && (info->flag_mem_usage ||
>> + info->flag_vmcore_size)) {
>> /*
>> * Parameter for showing the page number of memory
>> * in different use from.
>> @@ -11423,6 +11501,7 @@ static struct option longopts[] = {
>> {"work-dir", required_argument, NULL, OPT_WORKING_DIR},
>> {"num-threads", required_argument, NULL, OPT_NUM_THREADS},
>> {"check-params", no_argument, NULL, OPT_CHECK_PARAMS},
>> + {"vmcore-size", no_argument, NULL, OPT_VMCORE_SIZE},
>> {0, 0, 0, 0}
>> };
>>
>> @@ -11589,6 +11668,9 @@ main(int argc, char *argv[])
>> info->flag_check_params = TRUE;
>> message_level = DEFAULT_MSG_LEVEL;
>> break;
>> + case OPT_VMCORE_SIZE:
>> + info->flag_vmcore_size = TRUE;
>> + break;
>> case '?':
>> MSG("Commandline parameter is invalid.\n");
>> MSG("Try `makedumpfile --help' for more information.\n");
>> @@ -11598,6 +11680,10 @@ main(int argc, char *argv[])
>> if (flag_debug)
>> message_level |= ML_PRINT_DEBUG_MSG;
>>
>> + if (info->flag_vmcore_size)
>> + /* Suppress progress indicator as dumpfile won't get written */
>> + message_level &= ~ML_PRINT_PROGRESS;
>> +
>> if (info->flag_check_params)
>> /* suppress debugging messages */
>> message_level = DEFAULT_MSG_LEVEL;
>> @@ -11751,7 +11837,11 @@ main(int argc, char *argv[])
>> goto out;
>>
>> MSG("\n");
>> - if (info->flag_split) {
>> +
>> + if (info->flag_vmcore_size) {
>> + MSG("Estimated size to save vmcore is: %lld Bytes\n",
>> + (long long)info->dumpsize_info.non_hole_blocks * info->dumpsize_info.blksize);
>> + } else if (info->flag_split) {
>> MSG("The dumpfiles are saved to ");
>> for (i = 0; i < info->num_dumpfile; i++) {
>> if (i != (info->num_dumpfile - 1))
>> @@ -11808,6 +11898,8 @@ out:
>> free(info->page_buf);
>> if (info->parallel_info != NULL)
>> free(info->parallel_info);
>> + if (info->dumpsize_info.block_info != NULL)
>> + free(info->dumpsize_info.block_info);
>> free(info);
>>
>> if (splitblock) {
>> diff --git a/makedumpfile.h b/makedumpfile.h
>> index 03fb4ce..fd78d5f 100644
>> --- a/makedumpfile.h
>> +++ b/makedumpfile.h
>> @@ -1277,6 +1277,15 @@ struct parallel_info {
>> #endif
>> };
>>
>> +#define BASE_NUM_BLOCKS 50
>> +
>> +struct dumpsize_info {
>> + int blksize;
>> + int block_buff_size;
>> + unsigned char *block_info;
>> + int non_hole_blocks;
>> +};
>> +
>> struct ppc64_vmemmap {
>> unsigned long phys;
>> unsigned long virt;
>> @@ -1321,6 +1330,7 @@ struct DumpInfo {
>> int flag_vmemmap; /* kernel supports vmemmap address space */
>> int flag_excludevm; /* -e - excluding unused vmemmap pages */
>> int flag_use_count; /* _refcount is named _count in struct page */
>> + int flag_vmcore_size; /* estimate the size of the vmcore file instead of creating it */
>> unsigned long vaddr_for_vtop; /* virtual address for debugging */
>> long page_size; /* size of page */
>> long page_shift;
>> @@ -1425,6 +1435,7 @@ struct DumpInfo {
>> int num_dumpfile;
>> struct splitting_info *splitting_info;
>> struct parallel_info *parallel_info;
>> + struct dumpsize_info dumpsize_info;
>>
>> /*
>> * bitmap info:
>> @@ -2364,6 +2375,7 @@ struct elf_prstatus {
>> #define OPT_NUM_THREADS OPT_START+16
>> #define OPT_PARTIAL_DMESG OPT_START+17
>> #define OPT_CHECK_PARAMS OPT_START+18
>> +#define OPT_VMCORE_SIZE OPT_START+19
>>
>> /*
>> * Function Prototype.
>> diff --git a/print_info.c b/print_info.c
>> index e0c38b4..6f5a165 100644
>> --- a/print_info.c
>> +++ b/print_info.c
>> @@ -308,6 +308,10 @@ print_usage(void)
>> MSG(" the crashkernel range, then calculates the page number of different kind per\n");
>> MSG(" vmcoreinfo. So currently /proc/kcore need be specified explicitly.\n");
>> MSG("\n");
>> + MSG(" [--vmcore-size]:\n");
>> + MSG(" This option provides an estimation of the size needed to save VMCORE on disk.\n");
>> + MSG(" This option option cannot be used in combination with -F.\n");
>
> Also not in combination with --mem-usage (as per the code changes above)?
> And may be the options '--mem-usage / -F' also need an update to
> mention they can't be used with --vmcore-size option.
>
Good point, I'll update those.
>> + MSG("\n");
>> MSG(" [-D]:\n");
>> MSG(" Print debugging message.\n");
>> MSG("\n");
>> --
>
> I like the idea, but sometimes we use makedumpfile to generate a
> dumpfile in the primary kernel as well. For example:
>
> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile
>
> In such use-cases it is useful to use --vmcore-size and still generate
> the dumpfile (right now the default behaviour is not to generate a
> dumpfile when --vmcore-size is specified). Maybe we need to think more
> on supporting this use-case as well.
>
The thing is, if you are generating the dumpfile, you can just check the
size of the file created with "du -b" or some other command.
Overall I don't mind supporting your case as well. Maybe that can depend
on whether a vmcore/dumpfile filename is provided:
$ makedumpfile -d 31 -x vmlinux /proc/kcore # only estimates the size
$ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile # writes the
dumpfile and gives the final size
Any thought, opinions, suggestions?
Thanks,
--
Julien Thierry
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-13 9:53 ` Julien Thierry
@ 2020-10-13 19:45 ` Bhupesh Sharma
2020-10-16 6:45 ` HAGIO KAZUHITO(萩尾 一仁)
0 siblings, 1 reply; 13+ messages in thread
From: Bhupesh Sharma @ 2020-10-13 19:45 UTC (permalink / raw)
To: Julien Thierry, Kazuhito Hagio; +Cc: kexec mailing list, Kazuhito Hagio
Hello Julien,
On Tue, Oct 13, 2020 at 3:23 PM Julien Thierry <jthierry@redhat.com> wrote:
>
> Hi Bhupesh,
>
> On 10/13/20 10:27 AM, Bhupesh Sharma wrote:
> > Hello Julien,
> >
> > Thanks for the patch. Some nitpicks inline:
> >
> > On Mon, Oct 12, 2020 at 12:39 PM Julien Thierry <jthierry@redhat.com> wrote:
> >>
> >> A user might want to know how much space a vmcore file will take on
> >> the system and how much space on their disk should be available to
> >> save it during a crash.
> >>
> >> The option --vmcore-size does not create the vmcore file but provides
> >> an estimation of the size of the final vmcore file created with the
> >> same make dumpfile options.
> >>
> >> Signed-off-by: Julien Thierry <jthierry@redhat.com>
> >> Cc: Kazuhito Hagio <k-hagio-ab@nec.com>
> >> ---
> >> makedumpfile.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
> >> makedumpfile.h | 12 +++++++
> >> print_info.c | 4 +++
> >> 3 files changed, 111 insertions(+), 3 deletions(-)
> >
> > Please update 'makedumpfile.8' as well in v2, so that the man page can
> > document the newly added option and how to use it to determine the
> > vmcore-size.
> >
>
> Ah yes, I'll do that.
>
> >> diff --git a/makedumpfile.c b/makedumpfile.c
> >> index 4c4251e..0a2bfba 100644
> >> --- a/makedumpfile.c
> >> +++ b/makedumpfile.c
> >> @@ -26,6 +26,7 @@
> >> #include <limits.h>
> >> #include <assert.h>
> >> #include <zlib.h>
> >> +#include <libgen.h>
> >
> > I know we don't follow alphabetical order for include files in
> > makedumpfile code, but it would be good to place the new - ones
> > accordingly. So <libgen.h> can go with <limits.h> here.
> >
>
> Noted.
>
> >> struct symbol_table symbol_table;
> >> struct size_table size_table;
> >> @@ -1366,7 +1367,25 @@ open_dump_file(void)
> >> if (!info->flag_force)
> >> open_flags |= O_EXCL;
> >>
> >> - if (info->flag_flatten) {
> >> + if (info->flag_vmcore_size) {
> >> + char *namecpy;
> >> + struct stat statbuf;
> >> + int res;
> >> +
> >> + namecpy = strdup(info->name_dumpfile ?
> >> + info->name_dumpfile : ".");
> >> +
> >> + res = stat(dirname(namecpy), &statbuf);
> >> + free(namecpy);
> >> + if (res != 0)
> >> + return FALSE;
> >> +
> >> + fd = -1;
> >> + info->dumpsize_info.blksize = statbuf.st_blksize;
> >> + info->dumpsize_info.block_buff_size = BASE_NUM_BLOCKS;
> >> + info->dumpsize_info.block_info = calloc(BASE_NUM_BLOCKS, 1);
> >> + info->dumpsize_info.non_hole_blocks = 0;
> >> + } else if (info->flag_flatten) {
> >> fd = STDOUT_FILENO;
> >> info->name_dumpfile = filename_stdout;
> >> } else if ((fd = open(info->name_dumpfile, open_flags,
> >> @@ -1384,6 +1403,9 @@ check_dump_file(const char *path)
> >> {
> >> char *err_str;
> >>
> >> + if (info->flag_vmcore_size)
> >> + return TRUE;
> >> +
> >> if (access(path, F_OK) != 0)
> >> return TRUE; /* File does not exist */
> >> if (info->flag_force) {
> >> @@ -4622,6 +4644,47 @@ write_and_check_space(int fd, void *buf, size_t buf_size, char *file_name)
> >> return TRUE;
> >> }
> >>
> >> +static int
> >> +write_buffer_update_size_info(off_t offset, void *buf, size_t buf_size)
> >> +{
> >> + struct dumpsize_info *dumpsize_info = &info->dumpsize_info;
> >> + int blk_end_idx = (offset + buf_size - 1) / dumpsize_info->blksize;
> >> + int i;
> >> +
> >> + /* Need to grow the dumpsize block buffer? */
> >> + if (blk_end_idx >= dumpsize_info->block_buff_size) {
> >> + int alloc_size = MAX(blk_end_idx - dumpsize_info->block_buff_size, BASE_NUM_BLOCKS);
> >> +
> >> + dumpsize_info->block_info = realloc(dumpsize_info->block_info,
> >> + dumpsize_info->block_buff_size + alloc_size);
> >> + if (!dumpsize_info->block_info) {
> >> + ERRMSG("Not enough memory\n");
> >> + return FALSE;
> >> + }
> >> +
> >> + memset(dumpsize_info->block_info + dumpsize_info->block_buff_size,
> >> + 0, alloc_size);
> >> + dumpsize_info->block_buff_size += alloc_size;
> >> + }
> >> +
> >> + for (i = 0; i < buf_size; ++i) {
> >> + int blk_idx = (offset + i) / dumpsize_info->blksize;
> >> +
> >> + if (dumpsize_info->block_info[blk_idx]) {
> >> + i += dumpsize_info->blksize;
> >> + i = i - (i % dumpsize_info->blksize) - 1;
> >> + continue;
> >> + }
> >> +
> >> + if (((char *) buf)[i] != 0) {
> >> + dumpsize_info->non_hole_blocks++;
> >> + dumpsize_info->block_info[blk_idx] = 1;
> >> + }
> >> + }
> >> +
> >> + return TRUE;
> >> +}
> >> +
> >> int
> >> write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
> >> {
> >> @@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
> >> }
> >> if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
> >> return FALSE;
> >> + } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
> >> + return write_buffer_update_size_info(offset, buf, buf_size);
> >> } else {
> >> if (lseek(fd, offset, SEEK_SET) == failed) {
> >> ERRMSG("Can't seek the dump file(%s). %s\n",
> >> @@ -9018,6 +9083,12 @@ close_dump_file(void)
> >> if (info->flag_flatten)
> >> return;
> >>
> >> + if (info->flag_vmcore_size && info->fd_dumpfile == -1) {
> >> + free(info->dumpsize_info.block_info);
> >> + info->dumpsize_info.block_info = NULL;
> >> + return;
> >> + }
> >> +
> >> if (close(info->fd_dumpfile) < 0)
> >> ERRMSG("Can't close the dump file(%s). %s\n",
> >> info->name_dumpfile, strerror(errno));
> >> @@ -10963,6 +11034,12 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
> >> if (info->flag_flatten && info->flag_split)
> >> return FALSE;
> >>
> >> + if (info->flag_flatten && info->flag_vmcore_size)
> >> + return FALSE;
> >> +
> >> + if (info->flag_mem_usage && info->flag_vmcore_size)
> >> + return FALSE;
> >> +
> >> if (info->name_filterconfig && !info->name_vmlinux)
> >> return FALSE;
> >>
> >> @@ -11043,7 +11120,8 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
> >> */
> >> info->name_memory = argv[optind];
> >>
> >> - } else if ((argc == optind + 1) && info->flag_mem_usage) {
> >> + } else if ((argc == optind + 1) && (info->flag_mem_usage ||
> >> + info->flag_vmcore_size)) {
> >> /*
> >> * Parameter for showing the page number of memory
> >> * in different use from.
> >> @@ -11423,6 +11501,7 @@ static struct option longopts[] = {
> >> {"work-dir", required_argument, NULL, OPT_WORKING_DIR},
> >> {"num-threads", required_argument, NULL, OPT_NUM_THREADS},
> >> {"check-params", no_argument, NULL, OPT_CHECK_PARAMS},
> >> + {"vmcore-size", no_argument, NULL, OPT_VMCORE_SIZE},
> >> {0, 0, 0, 0}
> >> };
> >>
> >> @@ -11589,6 +11668,9 @@ main(int argc, char *argv[])
> >> info->flag_check_params = TRUE;
> >> message_level = DEFAULT_MSG_LEVEL;
> >> break;
> >> + case OPT_VMCORE_SIZE:
> >> + info->flag_vmcore_size = TRUE;
> >> + break;
> >> case '?':
> >> MSG("Commandline parameter is invalid.\n");
> >> MSG("Try `makedumpfile --help' for more information.\n");
> >> @@ -11598,6 +11680,10 @@ main(int argc, char *argv[])
> >> if (flag_debug)
> >> message_level |= ML_PRINT_DEBUG_MSG;
> >>
> >> + if (info->flag_vmcore_size)
> >> + /* Suppress progress indicator as dumpfile won't get written */
> >> + message_level &= ~ML_PRINT_PROGRESS;
> >> +
> >> if (info->flag_check_params)
> >> /* suppress debugging messages */
> >> message_level = DEFAULT_MSG_LEVEL;
> >> @@ -11751,7 +11837,11 @@ main(int argc, char *argv[])
> >> goto out;
> >>
> >> MSG("\n");
> >> - if (info->flag_split) {
> >> +
> >> + if (info->flag_vmcore_size) {
> >> + MSG("Estimated size to save vmcore is: %lld Bytes\n",
> >> + (long long)info->dumpsize_info.non_hole_blocks * info->dumpsize_info.blksize);
> >> + } else if (info->flag_split) {
> >> MSG("The dumpfiles are saved to ");
> >> for (i = 0; i < info->num_dumpfile; i++) {
> >> if (i != (info->num_dumpfile - 1))
> >> @@ -11808,6 +11898,8 @@ out:
> >> free(info->page_buf);
> >> if (info->parallel_info != NULL)
> >> free(info->parallel_info);
> >> + if (info->dumpsize_info.block_info != NULL)
> >> + free(info->dumpsize_info.block_info);
> >> free(info);
> >>
> >> if (splitblock) {
> >> diff --git a/makedumpfile.h b/makedumpfile.h
> >> index 03fb4ce..fd78d5f 100644
> >> --- a/makedumpfile.h
> >> +++ b/makedumpfile.h
> >> @@ -1277,6 +1277,15 @@ struct parallel_info {
> >> #endif
> >> };
> >>
> >> +#define BASE_NUM_BLOCKS 50
> >> +
> >> +struct dumpsize_info {
> >> + int blksize;
> >> + int block_buff_size;
> >> + unsigned char *block_info;
> >> + int non_hole_blocks;
> >> +};
> >> +
> >> struct ppc64_vmemmap {
> >> unsigned long phys;
> >> unsigned long virt;
> >> @@ -1321,6 +1330,7 @@ struct DumpInfo {
> >> int flag_vmemmap; /* kernel supports vmemmap address space */
> >> int flag_excludevm; /* -e - excluding unused vmemmap pages */
> >> int flag_use_count; /* _refcount is named _count in struct page */
> >> + int flag_vmcore_size; /* estimate the size of the vmcore file instead of creating it */
> >> unsigned long vaddr_for_vtop; /* virtual address for debugging */
> >> long page_size; /* size of page */
> >> long page_shift;
> >> @@ -1425,6 +1435,7 @@ struct DumpInfo {
> >> int num_dumpfile;
> >> struct splitting_info *splitting_info;
> >> struct parallel_info *parallel_info;
> >> + struct dumpsize_info dumpsize_info;
> >>
> >> /*
> >> * bitmap info:
> >> @@ -2364,6 +2375,7 @@ struct elf_prstatus {
> >> #define OPT_NUM_THREADS OPT_START+16
> >> #define OPT_PARTIAL_DMESG OPT_START+17
> >> #define OPT_CHECK_PARAMS OPT_START+18
> >> +#define OPT_VMCORE_SIZE OPT_START+19
> >>
> >> /*
> >> * Function Prototype.
> >> diff --git a/print_info.c b/print_info.c
> >> index e0c38b4..6f5a165 100644
> >> --- a/print_info.c
> >> +++ b/print_info.c
> >> @@ -308,6 +308,10 @@ print_usage(void)
> >> MSG(" the crashkernel range, then calculates the page number of different kind per\n");
> >> MSG(" vmcoreinfo. So currently /proc/kcore need be specified explicitly.\n");
> >> MSG("\n");
> >> + MSG(" [--vmcore-size]:\n");
> >> + MSG(" This option provides an estimation of the size needed to save VMCORE on disk.\n");
> >> + MSG(" This option option cannot be used in combination with -F.\n");
> >
> > Also not in combination with --mem-usage (as per the code changes above)?
> > And may be the options '--mem-usage / -F' also need an update to
> > mention they can't be used with --vmcore-size option.
> >
>
> Good point, I'll update those.
>
> >> + MSG("\n");
> >> MSG(" [-D]:\n");
> >> MSG(" Print debugging message.\n");
> >> MSG("\n");
> >> --
> >
> > I like the idea, but sometimes we use makedumpfile to generate a
> > dumpfile in the primary kernel as well. For example:
> >
> > $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile
> >
> > In such use-cases it is useful to use --vmcore-size and still generate
> > the dumpfile (right now the default behaviour is not to generate a
> > dumpfile when --vmcore-size is specified). Maybe we need to think more
> > on supporting this use-case as well.
> >
>
> The thing is, if you are generating the dumpfile, you can just check the
> size of the file created with "du -b" or some other command.
I agree, but I just was looking to replace the two 'makedumpfile +
du' steps with a single 'makedumpfile --vmcore-size' step.
> Overall I don't mind supporting your case as well. Maybe that can depend
> on whether a vmcore/dumpfile filename is provided:
>
> $ makedumpfile -d 31 -x vmlinux /proc/kcore # only estimates the size
>
> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile # writes the
> dumpfile and gives the final size
>
> Any thought, opinions, suggestions?
Let's wait for Kazu's opinion on the same, but I am ok with using a
two-step 'makedumpfile + du' approach for now (and later expand
--vmcore-size as we encounter more use-cases).
@Kazuhito Hagio : What's your opinion on the above?
Regards,
Bhupesh
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-13 9:27 ` Bhupesh Sharma
2020-10-13 9:53 ` Julien Thierry
@ 2020-10-14 4:31 ` lijiang
1 sibling, 0 replies; 13+ messages in thread
From: lijiang @ 2020-10-14 4:31 UTC (permalink / raw)
To: Bhupesh Sharma, Julien Thierry; +Cc: kexec mailing list, Kazuhito Hagio
在 2020年10月13日 17:27, Bhupesh Sharma 写道:
> Hello Julien,
>
> Thanks for the patch. Some nitpicks inline:
>
> On Mon, Oct 12, 2020 at 12:39 PM Julien Thierry <jthierry@redhat.com> wrote:
>>
>> A user might want to know how much space a vmcore file will take on
>> the system and how much space on their disk should be available to
>> save it during a crash.
>>
>> The option --vmcore-size does not create the vmcore file but provides
>> an estimation of the size of the final vmcore file created with the
>> same make dumpfile options.
>>
>> Signed-off-by: Julien Thierry <jthierry@redhat.com>
>> Cc: Kazuhito Hagio <k-hagio-ab@nec.com>
>> ---
>> makedumpfile.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
>> makedumpfile.h | 12 +++++++
>> print_info.c | 4 +++
>> 3 files changed, 111 insertions(+), 3 deletions(-)
>
> Please update 'makedumpfile.8' as well in v2, so that the man page can
> document the newly added option and how to use it to determine the
> vmcore-size.
>
>> diff --git a/makedumpfile.c b/makedumpfile.c
>> index 4c4251e..0a2bfba 100644
>> --- a/makedumpfile.c
>> +++ b/makedumpfile.c
>> @@ -26,6 +26,7 @@
>> #include <limits.h>
>> #include <assert.h>
>> #include <zlib.h>
>> +#include <libgen.h>
>
> I know we don't follow alphabetical order for include files in
> makedumpfile code, but it would be good to place the new - ones
> accordingly. So <libgen.h> can go with <limits.h> here.
>
>> struct symbol_table symbol_table;
>> struct size_table size_table;
>> @@ -1366,7 +1367,25 @@ open_dump_file(void)
>> if (!info->flag_force)
>> open_flags |= O_EXCL;
>>
>> - if (info->flag_flatten) {
>> + if (info->flag_vmcore_size) {
>> + char *namecpy;
>> + struct stat statbuf;
>> + int res;
>> +
>> + namecpy = strdup(info->name_dumpfile ?
>> + info->name_dumpfile : ".");
>> +
>> + res = stat(dirname(namecpy), &statbuf);
>> + free(namecpy);
>> + if (res != 0)
>> + return FALSE;
>> +
>> + fd = -1;
>> + info->dumpsize_info.blksize = statbuf.st_blksize;
>> + info->dumpsize_info.block_buff_size = BASE_NUM_BLOCKS;
>> + info->dumpsize_info.block_info = calloc(BASE_NUM_BLOCKS, 1);
>> + info->dumpsize_info.non_hole_blocks = 0;
>> + } else if (info->flag_flatten) {
>> fd = STDOUT_FILENO;
>> info->name_dumpfile = filename_stdout;
>> } else if ((fd = open(info->name_dumpfile, open_flags,
>> @@ -1384,6 +1403,9 @@ check_dump_file(const char *path)
>> {
>> char *err_str;
>>
>> + if (info->flag_vmcore_size)
>> + return TRUE;
>> +
>> if (access(path, F_OK) != 0)
>> return TRUE; /* File does not exist */
>> if (info->flag_force) {
>> @@ -4622,6 +4644,47 @@ write_and_check_space(int fd, void *buf, size_t buf_size, char *file_name)
>> return TRUE;
>> }
>>
>> +static int
>> +write_buffer_update_size_info(off_t offset, void *buf, size_t buf_size)
>> +{
>> + struct dumpsize_info *dumpsize_info = &info->dumpsize_info;
>> + int blk_end_idx = (offset + buf_size - 1) / dumpsize_info->blksize;
>> + int i;
>> +
>> + /* Need to grow the dumpsize block buffer? */
>> + if (blk_end_idx >= dumpsize_info->block_buff_size) {
>> + int alloc_size = MAX(blk_end_idx - dumpsize_info->block_buff_size, BASE_NUM_BLOCKS);
>> +
>> + dumpsize_info->block_info = realloc(dumpsize_info->block_info,
>> + dumpsize_info->block_buff_size + alloc_size);
>> + if (!dumpsize_info->block_info) {
>> + ERRMSG("Not enough memory\n");
>> + return FALSE;
>> + }
>> +
>> + memset(dumpsize_info->block_info + dumpsize_info->block_buff_size,
>> + 0, alloc_size);
>> + dumpsize_info->block_buff_size += alloc_size;
>> + }
>> +
>> + for (i = 0; i < buf_size; ++i) {
>> + int blk_idx = (offset + i) / dumpsize_info->blksize;
>> +
>> + if (dumpsize_info->block_info[blk_idx]) {
>> + i += dumpsize_info->blksize;
>> + i = i - (i % dumpsize_info->blksize) - 1;
>> + continue;
>> + }
>> +
>> + if (((char *) buf)[i] != 0) {
>> + dumpsize_info->non_hole_blocks++;
>> + dumpsize_info->block_info[blk_idx] = 1;
>> + }
>> + }
>> +
>> + return TRUE;
>> +}
>> +
>> int
>> write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
>> {
>> @@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
>> }
>> if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
>> return FALSE;
>> + } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
>> + return write_buffer_update_size_info(offset, buf, buf_size);
>> } else {
>> if (lseek(fd, offset, SEEK_SET) == failed) {
>> ERRMSG("Can't seek the dump file(%s). %s\n",
>> @@ -9018,6 +9083,12 @@ close_dump_file(void)
>> if (info->flag_flatten)
>> return;
>>
>> + if (info->flag_vmcore_size && info->fd_dumpfile == -1) {
>> + free(info->dumpsize_info.block_info);
>> + info->dumpsize_info.block_info = NULL;
>> + return;
>> + }
>> +
>> if (close(info->fd_dumpfile) < 0)
>> ERRMSG("Can't close the dump file(%s). %s\n",
>> info->name_dumpfile, strerror(errno));
>> @@ -10963,6 +11034,12 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
>> if (info->flag_flatten && info->flag_split)
>> return FALSE;
>>
>> + if (info->flag_flatten && info->flag_vmcore_size)
>> + return FALSE;
>> +
>> + if (info->flag_mem_usage && info->flag_vmcore_size)
>> + return FALSE;
>> +
>> if (info->name_filterconfig && !info->name_vmlinux)
>> return FALSE;
>>
>> @@ -11043,7 +11120,8 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
>> */
>> info->name_memory = argv[optind];
>>
>> - } else if ((argc == optind + 1) && info->flag_mem_usage) {
>> + } else if ((argc == optind + 1) && (info->flag_mem_usage ||
>> + info->flag_vmcore_size)) {
>> /*
>> * Parameter for showing the page number of memory
>> * in different use from.
>> @@ -11423,6 +11501,7 @@ static struct option longopts[] = {
>> {"work-dir", required_argument, NULL, OPT_WORKING_DIR},
>> {"num-threads", required_argument, NULL, OPT_NUM_THREADS},
>> {"check-params", no_argument, NULL, OPT_CHECK_PARAMS},
>> + {"vmcore-size", no_argument, NULL, OPT_VMCORE_SIZE},
>> {0, 0, 0, 0}
>> };
>>
>> @@ -11589,6 +11668,9 @@ main(int argc, char *argv[])
>> info->flag_check_params = TRUE;
>> message_level = DEFAULT_MSG_LEVEL;
>> break;
>> + case OPT_VMCORE_SIZE:
>> + info->flag_vmcore_size = TRUE;
>> + break;
>> case '?':
>> MSG("Commandline parameter is invalid.\n");
>> MSG("Try `makedumpfile --help' for more information.\n");
>> @@ -11598,6 +11680,10 @@ main(int argc, char *argv[])
>> if (flag_debug)
>> message_level |= ML_PRINT_DEBUG_MSG;
>>
>> + if (info->flag_vmcore_size)
>> + /* Suppress progress indicator as dumpfile won't get written */
>> + message_level &= ~ML_PRINT_PROGRESS;
>> +
>> if (info->flag_check_params)
>> /* suppress debugging messages */
>> message_level = DEFAULT_MSG_LEVEL;
>> @@ -11751,7 +11837,11 @@ main(int argc, char *argv[])
>> goto out;
>>
>> MSG("\n");
>> - if (info->flag_split) {
>> +
>> + if (info->flag_vmcore_size) {
>> + MSG("Estimated size to save vmcore is: %lld Bytes\n",
>> + (long long)info->dumpsize_info.non_hole_blocks * info->dumpsize_info.blksize);
>> + } else if (info->flag_split) {
>> MSG("The dumpfiles are saved to ");
>> for (i = 0; i < info->num_dumpfile; i++) {
>> if (i != (info->num_dumpfile - 1))
>> @@ -11808,6 +11898,8 @@ out:
>> free(info->page_buf);
>> if (info->parallel_info != NULL)
>> free(info->parallel_info);
>> + if (info->dumpsize_info.block_info != NULL)
>> + free(info->dumpsize_info.block_info);
>> free(info);
>>
>> if (splitblock) {
>> diff --git a/makedumpfile.h b/makedumpfile.h
>> index 03fb4ce..fd78d5f 100644
>> --- a/makedumpfile.h
>> +++ b/makedumpfile.h
>> @@ -1277,6 +1277,15 @@ struct parallel_info {
>> #endif
>> };
>>
>> +#define BASE_NUM_BLOCKS 50
>> +
>> +struct dumpsize_info {
>> + int blksize;
>> + int block_buff_size;
>> + unsigned char *block_info;
>> + int non_hole_blocks;
>> +};
>> +
>> struct ppc64_vmemmap {
>> unsigned long phys;
>> unsigned long virt;
>> @@ -1321,6 +1330,7 @@ struct DumpInfo {
>> int flag_vmemmap; /* kernel supports vmemmap address space */
>> int flag_excludevm; /* -e - excluding unused vmemmap pages */
>> int flag_use_count; /* _refcount is named _count in struct page */
>> + int flag_vmcore_size; /* estimate the size of the vmcore file instead of creating it */
>> unsigned long vaddr_for_vtop; /* virtual address for debugging */
>> long page_size; /* size of page */
>> long page_shift;
>> @@ -1425,6 +1435,7 @@ struct DumpInfo {
>> int num_dumpfile;
>> struct splitting_info *splitting_info;
>> struct parallel_info *parallel_info;
>> + struct dumpsize_info dumpsize_info;
>>
>> /*
>> * bitmap info:
>> @@ -2364,6 +2375,7 @@ struct elf_prstatus {
>> #define OPT_NUM_THREADS OPT_START+16
>> #define OPT_PARTIAL_DMESG OPT_START+17
>> #define OPT_CHECK_PARAMS OPT_START+18
>> +#define OPT_VMCORE_SIZE OPT_START+19
>>
>> /*
>> * Function Prototype.
>> diff --git a/print_info.c b/print_info.c
>> index e0c38b4..6f5a165 100644
>> --- a/print_info.c
>> +++ b/print_info.c
>> @@ -308,6 +308,10 @@ print_usage(void)
>> MSG(" the crashkernel range, then calculates the page number of different kind per\n");
>> MSG(" vmcoreinfo. So currently /proc/kcore need be specified explicitly.\n");
>> MSG("\n");
>> + MSG(" [--vmcore-size]:\n");
>> + MSG(" This option provides an estimation of the size needed to save VMCORE on disk.\n");
>> + MSG(" This option option cannot be used in combination with -F.\n");
>
> Also not in combination with --mem-usage (as per the code changes above)?
> And may be the options '--mem-usage / -F' also need an update to
> mention they can't be used with --vmcore-size option.
>
>> + MSG("\n");
>> MSG(" [-D]:\n");
>> MSG(" Print debugging message.\n");
>> MSG("\n");
>> --
>
> I like the idea, but sometimes we use makedumpfile to generate a
> dumpfile in the primary kernel as well. For example:
>
> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile
>
> In such use-cases it is useful to use --vmcore-size and still generate
> the dumpfile (right now the default behaviour is not to generate a
The option '--vmcore-size' will get all dumpable pages like the process of
saving vmcore, compute and return the final size of vmcore, but it doesn't
save the vmcore data.
The reason is that it doesn't affect the performance of mkdumprd, because
mkdumprd wants to use the size of vmcore to determine if there is enough
free disk space to hold the vmcore at a certain time point.
Saving vmcore data will waste time and cause the performance degradation
in this case.
Unless, provide additional option and work with the '--vmcore-size' together
in order to choose generating a dumpfile or not.
Thanks.
Lianbo
> dumpfile when --vmcore-size is specified). Maybe we need to think more
> on supporting this use-case as well.
>
> Regards,
> Bhupesh
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-13 19:45 ` Bhupesh Sharma
@ 2020-10-16 6:45 ` HAGIO KAZUHITO(萩尾 一仁)
2020-10-20 11:36 ` Julien Thierry
0 siblings, 1 reply; 13+ messages in thread
From: HAGIO KAZUHITO(萩尾 一仁) @ 2020-10-16 6:45 UTC (permalink / raw)
To: Bhupesh Sharma, Julien Thierry, Kazuhito Hagio; +Cc: kexec mailing list
Hi Julien,
-----Original Message-----
> Hello Julien,
>
> On Tue, Oct 13, 2020 at 3:23 PM Julien Thierry <jthierry@redhat.com> wrote:
> >
> > Hi Bhupesh,
> >
> > On 10/13/20 10:27 AM, Bhupesh Sharma wrote:
> > > Hello Julien,
> > >
> > > Thanks for the patch. Some nitpicks inline:
> > >
> > > On Mon, Oct 12, 2020 at 12:39 PM Julien Thierry <jthierry@redhat.com> wrote:
> > >>
> > >> A user might want to know how much space a vmcore file will take on
> > >> the system and how much space on their disk should be available to
> > >> save it during a crash.
> > >>
> > >> The option --vmcore-size does not create the vmcore file but provides
> > >> an estimation of the size of the final vmcore file created with the
> > >> same make dumpfile options.
Interesting. Do you have any actual use case? e.g. used by kdumpctl?
or use it in kdump initramfs?
> > >>
> > >> Signed-off-by: Julien Thierry <jthierry@redhat.com>
> > >> Cc: Kazuhito Hagio <k-hagio-ab@nec.com>
> > >> ---
> > >> makedumpfile.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
> > >> makedumpfile.h | 12 +++++++
> > >> print_info.c | 4 +++
> > >> 3 files changed, 111 insertions(+), 3 deletions(-)
> > >
> > > Please update 'makedumpfile.8' as well in v2, so that the man page can
> > > document the newly added option and how to use it to determine the
> > > vmcore-size.
> > >
> >
> > Ah yes, I'll do that.
> >
> > >> diff --git a/makedumpfile.c b/makedumpfile.c
> > >> index 4c4251e..0a2bfba 100644
> > >> --- a/makedumpfile.c
> > >> +++ b/makedumpfile.c
> > >> @@ -26,6 +26,7 @@
> > >> #include <limits.h>
> > >> #include <assert.h>
> > >> #include <zlib.h>
> > >> +#include <libgen.h>
> > >
> > > I know we don't follow alphabetical order for include files in
> > > makedumpfile code, but it would be good to place the new - ones
> > > accordingly. So <libgen.h> can go with <limits.h> here.
> > >
> >
> > Noted.
> >
> > >> struct symbol_table symbol_table;
> > >> struct size_table size_table;
> > >> @@ -1366,7 +1367,25 @@ open_dump_file(void)
> > >> if (!info->flag_force)
> > >> open_flags |= O_EXCL;
> > >>
> > >> - if (info->flag_flatten) {
> > >> + if (info->flag_vmcore_size) {
> > >> + char *namecpy;
> > >> + struct stat statbuf;
> > >> + int res;
> > >> +
> > >> + namecpy = strdup(info->name_dumpfile ?
> > >> + info->name_dumpfile : ".");
> > >> +
> > >> + res = stat(dirname(namecpy), &statbuf);
> > >> + free(namecpy);
> > >> + if (res != 0)
> > >> + return FALSE;
> > >> +
> > >> + fd = -1;
> > >> + info->dumpsize_info.blksize = statbuf.st_blksize;
> > >> + info->dumpsize_info.block_buff_size = BASE_NUM_BLOCKS;
> > >> + info->dumpsize_info.block_info = calloc(BASE_NUM_BLOCKS, 1);
> > >> + info->dumpsize_info.non_hole_blocks = 0;
> > >> + } else if (info->flag_flatten) {
> > >> fd = STDOUT_FILENO;
> > >> info->name_dumpfile = filename_stdout;
> > >> } else if ((fd = open(info->name_dumpfile, open_flags,
> > >> @@ -1384,6 +1403,9 @@ check_dump_file(const char *path)
> > >> {
> > >> char *err_str;
> > >>
> > >> + if (info->flag_vmcore_size)
> > >> + return TRUE;
> > >> +
> > >> if (access(path, F_OK) != 0)
> > >> return TRUE; /* File does not exist */
> > >> if (info->flag_force) {
> > >> @@ -4622,6 +4644,47 @@ write_and_check_space(int fd, void *buf, size_t buf_size, char *file_name)
> > >> return TRUE;
> > >> }
> > >>
> > >> +static int
> > >> +write_buffer_update_size_info(off_t offset, void *buf, size_t buf_size)
> > >> +{
> > >> + struct dumpsize_info *dumpsize_info = &info->dumpsize_info;
> > >> + int blk_end_idx = (offset + buf_size - 1) / dumpsize_info->blksize;
> > >> + int i;
> > >> +
> > >> + /* Need to grow the dumpsize block buffer? */
> > >> + if (blk_end_idx >= dumpsize_info->block_buff_size) {
> > >> + int alloc_size = MAX(blk_end_idx - dumpsize_info->block_buff_size, BASE_NUM_BLOCKS);
> > >> +
> > >> + dumpsize_info->block_info = realloc(dumpsize_info->block_info,
> > >> + dumpsize_info->block_buff_size + alloc_size);
> > >> + if (!dumpsize_info->block_info) {
> > >> + ERRMSG("Not enough memory\n");
> > >> + return FALSE;
> > >> + }
> > >> +
> > >> + memset(dumpsize_info->block_info + dumpsize_info->block_buff_size,
> > >> + 0, alloc_size);
> > >> + dumpsize_info->block_buff_size += alloc_size;
> > >> + }
> > >> +
> > >> + for (i = 0; i < buf_size; ++i) {
> > >> + int blk_idx = (offset + i) / dumpsize_info->blksize;
> > >> +
> > >> + if (dumpsize_info->block_info[blk_idx]) {
> > >> + i += dumpsize_info->blksize;
> > >> + i = i - (i % dumpsize_info->blksize) - 1;
> > >> + continue;
> > >> + }
> > >> +
> > >> + if (((char *) buf)[i] != 0) {
> > >> + dumpsize_info->non_hole_blocks++;
> > >> + dumpsize_info->block_info[blk_idx] = 1;
> > >> + }
> > >> + }
> > >> +
> > >> + return TRUE;
> > >> +}
> > >> +
> > >> int
> > >> write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
> > >> {
> > >> @@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
> > >> }
> > >> if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
> > >> return FALSE;
> > >> + } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
> > >> + return write_buffer_update_size_info(offset, buf, buf_size);
Why do we need this function? makedumpfile actually writes zero-filled
pages to the dumpfile with -d 0, and doesn't write them with -d 1.
So isn't "write_bytes += buf_size" enough? For example, with -d 30,
# makedumpfile --vmcore-size -d30 vmcore
Estimated size to save vmcore is: 147595264 Bytes
write_bytes: 172782736 Bytes // calculated by "write_bytes += buf_size"
makedumpfile Completed.
# makedumpfile -d30 vmcore dump.d30
Copying data : [100.0 %] / eta: 0s
The dumpfile is saved to dump.d30.
makedumpfile Completed.
# ls -ls dump.d30
168740 -rw------- 1 root root 172787864 Oct 16 15:14 dump.d30
> > >> } else {
> > >> if (lseek(fd, offset, SEEK_SET) == failed) {
> > >> ERRMSG("Can't seek the dump file(%s). %s\n",
> > >> @@ -9018,6 +9083,12 @@ close_dump_file(void)
> > >> if (info->flag_flatten)
> > >> return;
> > >>
> > >> + if (info->flag_vmcore_size && info->fd_dumpfile == -1) {
> > >> + free(info->dumpsize_info.block_info);
> > >> + info->dumpsize_info.block_info = NULL;
> > >> + return;
> > >> + }
> > >> +
> > >> if (close(info->fd_dumpfile) < 0)
> > >> ERRMSG("Can't close the dump file(%s). %s\n",
> > >> info->name_dumpfile, strerror(errno));
> > >> @@ -10963,6 +11034,12 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
> > >> if (info->flag_flatten && info->flag_split)
> > >> return FALSE;
> > >>
> > >> + if (info->flag_flatten && info->flag_vmcore_size)
> > >> + return FALSE;
> > >> +
> > >> + if (info->flag_mem_usage && info->flag_vmcore_size)
> > >> + return FALSE;
> > >> +
> > >> if (info->name_filterconfig && !info->name_vmlinux)
> > >> return FALSE;
> > >>
> > >> @@ -11043,7 +11120,8 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
> > >> */
> > >> info->name_memory = argv[optind];
> > >>
> > >> - } else if ((argc == optind + 1) && info->flag_mem_usage) {
> > >> + } else if ((argc == optind + 1) && (info->flag_mem_usage ||
> > >> + info->flag_vmcore_size)) {
> > >> /*
> > >> * Parameter for showing the page number of memory
> > >> * in different use from.
> > >> @@ -11423,6 +11501,7 @@ static struct option longopts[] = {
> > >> {"work-dir", required_argument, NULL, OPT_WORKING_DIR},
> > >> {"num-threads", required_argument, NULL, OPT_NUM_THREADS},
> > >> {"check-params", no_argument, NULL, OPT_CHECK_PARAMS},
> > >> + {"vmcore-size", no_argument, NULL, OPT_VMCORE_SIZE},
> > >> {0, 0, 0, 0}
> > >> };
> > >>
> > >> @@ -11589,6 +11668,9 @@ main(int argc, char *argv[])
> > >> info->flag_check_params = TRUE;
> > >> message_level = DEFAULT_MSG_LEVEL;
> > >> break;
> > >> + case OPT_VMCORE_SIZE:
> > >> + info->flag_vmcore_size = TRUE;
> > >> + break;
> > >> case '?':
> > >> MSG("Commandline parameter is invalid.\n");
> > >> MSG("Try `makedumpfile --help' for more information.\n");
> > >> @@ -11598,6 +11680,10 @@ main(int argc, char *argv[])
> > >> if (flag_debug)
> > >> message_level |= ML_PRINT_DEBUG_MSG;
> > >>
> > >> + if (info->flag_vmcore_size)
> > >> + /* Suppress progress indicator as dumpfile won't get written */
> > >> + message_level &= ~ML_PRINT_PROGRESS;
> > >> +
> > >> if (info->flag_check_params)
> > >> /* suppress debugging messages */
> > >> message_level = DEFAULT_MSG_LEVEL;
> > >> @@ -11751,7 +11837,11 @@ main(int argc, char *argv[])
> > >> goto out;
> > >>
> > >> MSG("\n");
> > >> - if (info->flag_split) {
> > >> +
> > >> + if (info->flag_vmcore_size) {
> > >> + MSG("Estimated size to save vmcore is: %lld Bytes\n",
> > >> + (long long)info->dumpsize_info.non_hole_blocks *
> info->dumpsize_info.blksize);
> > >> + } else if (info->flag_split) {
> > >> MSG("The dumpfiles are saved to ");
> > >> for (i = 0; i < info->num_dumpfile; i++) {
> > >> if (i != (info->num_dumpfile - 1))
> > >> @@ -11808,6 +11898,8 @@ out:
> > >> free(info->page_buf);
> > >> if (info->parallel_info != NULL)
> > >> free(info->parallel_info);
> > >> + if (info->dumpsize_info.block_info != NULL)
> > >> + free(info->dumpsize_info.block_info);
> > >> free(info);
> > >>
> > >> if (splitblock) {
> > >> diff --git a/makedumpfile.h b/makedumpfile.h
> > >> index 03fb4ce..fd78d5f 100644
> > >> --- a/makedumpfile.h
> > >> +++ b/makedumpfile.h
> > >> @@ -1277,6 +1277,15 @@ struct parallel_info {
> > >> #endif
> > >> };
> > >>
> > >> +#define BASE_NUM_BLOCKS 50
> > >> +
> > >> +struct dumpsize_info {
> > >> + int blksize;
> > >> + int block_buff_size;
> > >> + unsigned char *block_info;
> > >> + int non_hole_blocks;
> > >> +};
> > >> +
> > >> struct ppc64_vmemmap {
> > >> unsigned long phys;
> > >> unsigned long virt;
> > >> @@ -1321,6 +1330,7 @@ struct DumpInfo {
> > >> int flag_vmemmap; /* kernel supports vmemmap address space */
> > >> int flag_excludevm; /* -e - excluding unused vmemmap pages */
> > >> int flag_use_count; /* _refcount is named _count in struct page */
> > >> + int flag_vmcore_size; /* estimate the size of the vmcore file instead of
> creating it */
> > >> unsigned long vaddr_for_vtop; /* virtual address for debugging */
> > >> long page_size; /* size of page */
> > >> long page_shift;
> > >> @@ -1425,6 +1435,7 @@ struct DumpInfo {
> > >> int num_dumpfile;
> > >> struct splitting_info *splitting_info;
> > >> struct parallel_info *parallel_info;
> > >> + struct dumpsize_info dumpsize_info;
> > >>
> > >> /*
> > >> * bitmap info:
> > >> @@ -2364,6 +2375,7 @@ struct elf_prstatus {
> > >> #define OPT_NUM_THREADS OPT_START+16
> > >> #define OPT_PARTIAL_DMESG OPT_START+17
> > >> #define OPT_CHECK_PARAMS OPT_START+18
> > >> +#define OPT_VMCORE_SIZE OPT_START+19
> > >>
> > >> /*
> > >> * Function Prototype.
> > >> diff --git a/print_info.c b/print_info.c
> > >> index e0c38b4..6f5a165 100644
> > >> --- a/print_info.c
> > >> +++ b/print_info.c
> > >> @@ -308,6 +308,10 @@ print_usage(void)
> > >> MSG(" the crashkernel range, then calculates the page number of different kind per\n");
> > >> MSG(" vmcoreinfo. So currently /proc/kcore need be specified explicitly.\n");
> > >> MSG("\n");
> > >> + MSG(" [--vmcore-size]:\n");
> > >> + MSG(" This option provides an estimation of the size needed to save VMCORE on disk.\n");
> > >> + MSG(" This option option cannot be used in combination with -F.\n");
> > >
> > > Also not in combination with --mem-usage (as per the code changes above)?
> > > And may be the options '--mem-usage / -F' also need an update to
> > > mention they can't be used with --vmcore-size option.
> > >
> >
> > Good point, I'll update those.
> >
> > >> + MSG("\n");
> > >> MSG(" [-D]:\n");
> > >> MSG(" Print debugging message.\n");
> > >> MSG("\n");
> > >> --
> > >
> > > I like the idea, but sometimes we use makedumpfile to generate a
> > > dumpfile in the primary kernel as well. For example:
> > >
> > > $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile
> > >
> > > In such use-cases it is useful to use --vmcore-size and still generate
> > > the dumpfile (right now the default behaviour is not to generate a
> > > dumpfile when --vmcore-size is specified). Maybe we need to think more
> > > on supporting this use-case as well.
> > >
> >
> > The thing is, if you are generating the dumpfile, you can just check the
> > size of the file created with "du -b" or some other command.
>
> I agree, but I just was looking to replace the two 'makedumpfile +
> du' steps with a single 'makedumpfile --vmcore-size' step.
>
> > Overall I don't mind supporting your case as well. Maybe that can depend
> > on whether a vmcore/dumpfile filename is provided:
> >
> > $ makedumpfile -d 31 -x vmlinux /proc/kcore # only estimates the size
> >
> > $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile # writes the
> > dumpfile and gives the final size
> >
> > Any thought, opinions, suggestions?
>
> Let's wait for Kazu's opinion on the same, but I am ok with using a
> two-step 'makedumpfile + du' approach for now (and later expand
> --vmcore-size as we encounter more use-cases).
>
> @Kazuhito Hagio : What's your opinion on the above?
I would prefer only estimating with the option.
And if the write_bytes method above is usable, it can be shown also
in report messages when wrote the dumpfile.
Thanks,
Kazu
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-16 6:45 ` HAGIO KAZUHITO(萩尾 一仁)
@ 2020-10-20 11:36 ` Julien Thierry
2020-10-28 8:32 ` HAGIO KAZUHITO(萩尾 一仁)
0 siblings, 1 reply; 13+ messages in thread
From: Julien Thierry @ 2020-10-20 11:36 UTC (permalink / raw)
To: HAGIO KAZUHITO(萩尾 一仁),
Bhupesh Sharma, Kazuhito Hagio
Cc: kexec mailing list
Hi Kazuhito,
On 10/16/20 7:45 AM, HAGIO KAZUHITO(萩尾 一仁) wrote:
> Hi Julien,
>
> -----Original Message-----
>> Hello Julien,
>>
>> On Tue, Oct 13, 2020 at 3:23 PM Julien Thierry <jthierry@redhat.com> wrote:
>>>
>>> Hi Bhupesh,
>>>
>>> On 10/13/20 10:27 AM, Bhupesh Sharma wrote:
>>>> Hello Julien,
>>>>
>>>> Thanks for the patch. Some nitpicks inline:
>>>>
>>>> On Mon, Oct 12, 2020 at 12:39 PM Julien Thierry <jthierry@redhat.com> wrote:
>>>>>
>>>>> A user might want to know how much space a vmcore file will take on
>>>>> the system and how much space on their disk should be available to
>>>>> save it during a crash.
>>>>>
>>>>> The option --vmcore-size does not create the vmcore file but provides
>>>>> an estimation of the size of the final vmcore file created with the
>>>>> same make dumpfile options.
>
> Interesting. Do you have any actual use case? e.g. used by kdumpctl?
> or use it in kdump initramfs?
>
Yes, the idea would be to use this in mkdumprd to have a more accurate
estimate of the dump size (currently it cannot take compression into
account and warns about potential lack of space, considering the system
memory size as a whole).
>>>>>
>>>>> Signed-off-by: Julien Thierry <jthierry@redhat.com>
>>>>> Cc: Kazuhito Hagio <k-hagio-ab@nec.com>
>>>>> ---
>>>>> makedumpfile.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++--
>>>>> makedumpfile.h | 12 +++++++
>>>>> print_info.c | 4 +++
>>>>> 3 files changed, 111 insertions(+), 3 deletions(-)
>>>>
>>>> Please update 'makedumpfile.8' as well in v2, so that the man page can
>>>> document the newly added option and how to use it to determine the
>>>> vmcore-size.
>>>>
>>>
>>> Ah yes, I'll do that.
>>>
>>>>> diff --git a/makedumpfile.c b/makedumpfile.c
>>>>> index 4c4251e..0a2bfba 100644
>>>>> --- a/makedumpfile.c
>>>>> +++ b/makedumpfile.c
>>>>> @@ -26,6 +26,7 @@
>>>>> #include <limits.h>
>>>>> #include <assert.h>
>>>>> #include <zlib.h>
>>>>> +#include <libgen.h>
>>>>
>>>> I know we don't follow alphabetical order for include files in
>>>> makedumpfile code, but it would be good to place the new - ones
>>>> accordingly. So <libgen.h> can go with <limits.h> here.
>>>>
>>>
>>> Noted.
>>>
>>>>> struct symbol_table symbol_table;
>>>>> struct size_table size_table;
>>>>> @@ -1366,7 +1367,25 @@ open_dump_file(void)
>>>>> if (!info->flag_force)
>>>>> open_flags |= O_EXCL;
>>>>>
>>>>> - if (info->flag_flatten) {
>>>>> + if (info->flag_vmcore_size) {
>>>>> + char *namecpy;
>>>>> + struct stat statbuf;
>>>>> + int res;
>>>>> +
>>>>> + namecpy = strdup(info->name_dumpfile ?
>>>>> + info->name_dumpfile : ".");
>>>>> +
>>>>> + res = stat(dirname(namecpy), &statbuf);
>>>>> + free(namecpy);
>>>>> + if (res != 0)
>>>>> + return FALSE;
>>>>> +
>>>>> + fd = -1;
>>>>> + info->dumpsize_info.blksize = statbuf.st_blksize;
>>>>> + info->dumpsize_info.block_buff_size = BASE_NUM_BLOCKS;
>>>>> + info->dumpsize_info.block_info = calloc(BASE_NUM_BLOCKS, 1);
>>>>> + info->dumpsize_info.non_hole_blocks = 0;
>>>>> + } else if (info->flag_flatten) {
>>>>> fd = STDOUT_FILENO;
>>>>> info->name_dumpfile = filename_stdout;
>>>>> } else if ((fd = open(info->name_dumpfile, open_flags,
>>>>> @@ -1384,6 +1403,9 @@ check_dump_file(const char *path)
>>>>> {
>>>>> char *err_str;
>>>>>
>>>>> + if (info->flag_vmcore_size)
>>>>> + return TRUE;
>>>>> +
>>>>> if (access(path, F_OK) != 0)
>>>>> return TRUE; /* File does not exist */
>>>>> if (info->flag_force) {
>>>>> @@ -4622,6 +4644,47 @@ write_and_check_space(int fd, void *buf, size_t buf_size, char *file_name)
>>>>> return TRUE;
>>>>> }
>>>>>
>>>>> +static int
>>>>> +write_buffer_update_size_info(off_t offset, void *buf, size_t buf_size)
>>>>> +{
>>>>> + struct dumpsize_info *dumpsize_info = &info->dumpsize_info;
>>>>> + int blk_end_idx = (offset + buf_size - 1) / dumpsize_info->blksize;
>>>>> + int i;
>>>>> +
>>>>> + /* Need to grow the dumpsize block buffer? */
>>>>> + if (blk_end_idx >= dumpsize_info->block_buff_size) {
>>>>> + int alloc_size = MAX(blk_end_idx - dumpsize_info->block_buff_size, BASE_NUM_BLOCKS);
>>>>> +
>>>>> + dumpsize_info->block_info = realloc(dumpsize_info->block_info,
>>>>> + dumpsize_info->block_buff_size + alloc_size);
>>>>> + if (!dumpsize_info->block_info) {
>>>>> + ERRMSG("Not enough memory\n");
>>>>> + return FALSE;
>>>>> + }
>>>>> +
>>>>> + memset(dumpsize_info->block_info + dumpsize_info->block_buff_size,
>>>>> + 0, alloc_size);
>>>>> + dumpsize_info->block_buff_size += alloc_size;
>>>>> + }
>>>>> +
>>>>> + for (i = 0; i < buf_size; ++i) {
>>>>> + int blk_idx = (offset + i) / dumpsize_info->blksize;
>>>>> +
>>>>> + if (dumpsize_info->block_info[blk_idx]) {
>>>>> + i += dumpsize_info->blksize;
>>>>> + i = i - (i % dumpsize_info->blksize) - 1;
>>>>> + continue;
>>>>> + }
>>>>> +
>>>>> + if (((char *) buf)[i] != 0) {
>>>>> + dumpsize_info->non_hole_blocks++;
>>>>> + dumpsize_info->block_info[blk_idx] = 1;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + return TRUE;
>>>>> +}
>>>>> +
>>>>> int
>>>>> write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
>>>>> {
>>>>> @@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
>>>>> }
>>>>> if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
>>>>> return FALSE;
>>>>> + } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
>>>>> + return write_buffer_update_size_info(offset, buf, buf_size);
>
> Why do we need this function? makedumpfile actually writes zero-filled
> pages to the dumpfile with -d 0, and doesn't write them with -d 1.
> So isn't "write_bytes += buf_size" enough? For example, with -d 30,
>
The reason I went with this method was to make an estimate of the number
of blocks actually allocated on the disk (since depending on how the
data written is scattered in the file, there might be a significant
difference between bytes written vs actual size allocated on disk). But
I realize that there is some misunderstanding from my end since written
0 do make block allocation as opposed to not writing at some offset
(skipping the with lseek() ), I would need to fix that.
To highlight the behaviour I'm talking about:
$ dd if=/dev/zero of=./testfile bs=4096 count=1 seek=1
1+0 records in
1+0 records out
4096 bytes (4.1 kB, 4.0 KiB) copied, 0.000302719 s, 13.5 MB/s
$ du -h testfile
4.0K testfile
$ dd if=/dev/zero of=./testfile bs=4096 count=2
2+0 records in
2+0 records out
8192 bytes (8.2 kB, 8.0 KiB) copied, 0.000373002 s, 22.0 MB/s
$ du -h testfile
8.0K testfile
So, do you think it's not worth bothering estimating the number of
blocks allocated an that I should only consider the number of bytes written?
> # makedumpfile --vmcore-size -d30 vmcore
>
> Estimated size to save vmcore is: 147595264 Bytes
> write_bytes: 172782736 Bytes // calculated by "write_bytes += buf_size"
>
> makedumpfile Completed.
> # makedumpfile -d30 vmcore dump.d30
> Copying data : [100.0 %] / eta: 0s
>
> The dumpfile is saved to dump.d30.
>
> makedumpfile Completed.
> # ls -ls dump.d30
> 168740 -rw------- 1 root root 172787864 Oct 16 15:14 dump.d30
>
>
>>>>> } else {
>>>>> if (lseek(fd, offset, SEEK_SET) == failed) {
>>>>> ERRMSG("Can't seek the dump file(%s). %s\n",
>>>>> @@ -9018,6 +9083,12 @@ close_dump_file(void)
>>>>> if (info->flag_flatten)
>>>>> return;
>>>>>
>>>>> + if (info->flag_vmcore_size && info->fd_dumpfile == -1) {
>>>>> + free(info->dumpsize_info.block_info);
>>>>> + info->dumpsize_info.block_info = NULL;
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> if (close(info->fd_dumpfile) < 0)
>>>>> ERRMSG("Can't close the dump file(%s). %s\n",
>>>>> info->name_dumpfile, strerror(errno));
>>>>> @@ -10963,6 +11034,12 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
>>>>> if (info->flag_flatten && info->flag_split)
>>>>> return FALSE;
>>>>>
>>>>> + if (info->flag_flatten && info->flag_vmcore_size)
>>>>> + return FALSE;
>>>>> +
>>>>> + if (info->flag_mem_usage && info->flag_vmcore_size)
>>>>> + return FALSE;
>>>>> +
>>>>> if (info->name_filterconfig && !info->name_vmlinux)
>>>>> return FALSE;
>>>>>
>>>>> @@ -11043,7 +11120,8 @@ check_param_for_creating_dumpfile(int argc, char *argv[])
>>>>> */
>>>>> info->name_memory = argv[optind];
>>>>>
>>>>> - } else if ((argc == optind + 1) && info->flag_mem_usage) {
>>>>> + } else if ((argc == optind + 1) && (info->flag_mem_usage ||
>>>>> + info->flag_vmcore_size)) {
>>>>> /*
>>>>> * Parameter for showing the page number of memory
>>>>> * in different use from.
>>>>> @@ -11423,6 +11501,7 @@ static struct option longopts[] = {
>>>>> {"work-dir", required_argument, NULL, OPT_WORKING_DIR},
>>>>> {"num-threads", required_argument, NULL, OPT_NUM_THREADS},
>>>>> {"check-params", no_argument, NULL, OPT_CHECK_PARAMS},
>>>>> + {"vmcore-size", no_argument, NULL, OPT_VMCORE_SIZE},
>>>>> {0, 0, 0, 0}
>>>>> };
>>>>>
>>>>> @@ -11589,6 +11668,9 @@ main(int argc, char *argv[])
>>>>> info->flag_check_params = TRUE;
>>>>> message_level = DEFAULT_MSG_LEVEL;
>>>>> break;
>>>>> + case OPT_VMCORE_SIZE:
>>>>> + info->flag_vmcore_size = TRUE;
>>>>> + break;
>>>>> case '?':
>>>>> MSG("Commandline parameter is invalid.\n");
>>>>> MSG("Try `makedumpfile --help' for more information.\n");
>>>>> @@ -11598,6 +11680,10 @@ main(int argc, char *argv[])
>>>>> if (flag_debug)
>>>>> message_level |= ML_PRINT_DEBUG_MSG;
>>>>>
>>>>> + if (info->flag_vmcore_size)
>>>>> + /* Suppress progress indicator as dumpfile won't get written */
>>>>> + message_level &= ~ML_PRINT_PROGRESS;
>>>>> +
>>>>> if (info->flag_check_params)
>>>>> /* suppress debugging messages */
>>>>> message_level = DEFAULT_MSG_LEVEL;
>>>>> @@ -11751,7 +11837,11 @@ main(int argc, char *argv[])
>>>>> goto out;
>>>>>
>>>>> MSG("\n");
>>>>> - if (info->flag_split) {
>>>>> +
>>>>> + if (info->flag_vmcore_size) {
>>>>> + MSG("Estimated size to save vmcore is: %lld Bytes\n",
>>>>> + (long long)info->dumpsize_info.non_hole_blocks *
>> info->dumpsize_info.blksize);
>>>>> + } else if (info->flag_split) {
>>>>> MSG("The dumpfiles are saved to ");
>>>>> for (i = 0; i < info->num_dumpfile; i++) {
>>>>> if (i != (info->num_dumpfile - 1))
>>>>> @@ -11808,6 +11898,8 @@ out:
>>>>> free(info->page_buf);
>>>>> if (info->parallel_info != NULL)
>>>>> free(info->parallel_info);
>>>>> + if (info->dumpsize_info.block_info != NULL)
>>>>> + free(info->dumpsize_info.block_info);
>>>>> free(info);
>>>>>
>>>>> if (splitblock) {
>>>>> diff --git a/makedumpfile.h b/makedumpfile.h
>>>>> index 03fb4ce..fd78d5f 100644
>>>>> --- a/makedumpfile.h
>>>>> +++ b/makedumpfile.h
>>>>> @@ -1277,6 +1277,15 @@ struct parallel_info {
>>>>> #endif
>>>>> };
>>>>>
>>>>> +#define BASE_NUM_BLOCKS 50
>>>>> +
>>>>> +struct dumpsize_info {
>>>>> + int blksize;
>>>>> + int block_buff_size;
>>>>> + unsigned char *block_info;
>>>>> + int non_hole_blocks;
>>>>> +};
>>>>> +
>>>>> struct ppc64_vmemmap {
>>>>> unsigned long phys;
>>>>> unsigned long virt;
>>>>> @@ -1321,6 +1330,7 @@ struct DumpInfo {
>>>>> int flag_vmemmap; /* kernel supports vmemmap address space */
>>>>> int flag_excludevm; /* -e - excluding unused vmemmap pages */
>>>>> int flag_use_count; /* _refcount is named _count in struct page */
>>>>> + int flag_vmcore_size; /* estimate the size of the vmcore file instead of
>> creating it */
>>>>> unsigned long vaddr_for_vtop; /* virtual address for debugging */
>>>>> long page_size; /* size of page */
>>>>> long page_shift;
>>>>> @@ -1425,6 +1435,7 @@ struct DumpInfo {
>>>>> int num_dumpfile;
>>>>> struct splitting_info *splitting_info;
>>>>> struct parallel_info *parallel_info;
>>>>> + struct dumpsize_info dumpsize_info;
>>>>>
>>>>> /*
>>>>> * bitmap info:
>>>>> @@ -2364,6 +2375,7 @@ struct elf_prstatus {
>>>>> #define OPT_NUM_THREADS OPT_START+16
>>>>> #define OPT_PARTIAL_DMESG OPT_START+17
>>>>> #define OPT_CHECK_PARAMS OPT_START+18
>>>>> +#define OPT_VMCORE_SIZE OPT_START+19
>>>>>
>>>>> /*
>>>>> * Function Prototype.
>>>>> diff --git a/print_info.c b/print_info.c
>>>>> index e0c38b4..6f5a165 100644
>>>>> --- a/print_info.c
>>>>> +++ b/print_info.c
>>>>> @@ -308,6 +308,10 @@ print_usage(void)
>>>>> MSG(" the crashkernel range, then calculates the page number of different kind per\n");
>>>>> MSG(" vmcoreinfo. So currently /proc/kcore need be specified explicitly.\n");
>>>>> MSG("\n");
>>>>> + MSG(" [--vmcore-size]:\n");
>>>>> + MSG(" This option provides an estimation of the size needed to save VMCORE on disk.\n");
>>>>> + MSG(" This option option cannot be used in combination with -F.\n");
>>>>
>>>> Also not in combination with --mem-usage (as per the code changes above)?
>>>> And may be the options '--mem-usage / -F' also need an update to
>>>> mention they can't be used with --vmcore-size option.
>>>>
>>>
>>> Good point, I'll update those.
>>>
>>>>> + MSG("\n");
>>>>> MSG(" [-D]:\n");
>>>>> MSG(" Print debugging message.\n");
>>>>> MSG("\n");
>>>>> --
>>>>
>>>> I like the idea, but sometimes we use makedumpfile to generate a
>>>> dumpfile in the primary kernel as well. For example:
>>>>
>>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile
>>>>
>>>> In such use-cases it is useful to use --vmcore-size and still generate
>>>> the dumpfile (right now the default behaviour is not to generate a
>>>> dumpfile when --vmcore-size is specified). Maybe we need to think more
>>>> on supporting this use-case as well.
>>>>
>>>
>>> The thing is, if you are generating the dumpfile, you can just check the
>>> size of the file created with "du -b" or some other command.
>>
>> I agree, but I just was looking to replace the two 'makedumpfile +
>> du' steps with a single 'makedumpfile --vmcore-size' step.
>>
>>> Overall I don't mind supporting your case as well. Maybe that can depend
>>> on whether a vmcore/dumpfile filename is provided:
>>>
>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore # only estimates the size
>>>
>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile # writes the
>>> dumpfile and gives the final size
>>>
>>> Any thought, opinions, suggestions?
>>
>> Let's wait for Kazu's opinion on the same, but I am ok with using a
>> two-step 'makedumpfile + du' approach for now (and later expand
>> --vmcore-size as we encounter more use-cases).
>>
>> @Kazuhito Hagio : What's your opinion on the above?
>
> I would prefer only estimating with the option.
>
> And if the write_bytes method above is usable, it can be shown also
> in report messages when wrote the dumpfile.
>
Let me know your preferred approach considering my comment above and
I'll send out a v2.
Thanks,
--
Julien Thierry
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-20 11:36 ` Julien Thierry
@ 2020-10-28 8:32 ` HAGIO KAZUHITO(萩尾 一仁)
2020-10-29 12:43 ` lijiang
2020-11-18 3:57 ` lijiang
0 siblings, 2 replies; 13+ messages in thread
From: HAGIO KAZUHITO(萩尾 一仁) @ 2020-10-28 8:32 UTC (permalink / raw)
To: Julien Thierry, Bhupesh Sharma; +Cc: kexec mailing list
Hi Julien,
sorry for my delayed reply.
-----Original Message-----
> >>>>> A user might want to know how much space a vmcore file will take on
> >>>>> the system and how much space on their disk should be available to
> >>>>> save it during a crash.
> >>>>>
> >>>>> The option --vmcore-size does not create the vmcore file but provides
> >>>>> an estimation of the size of the final vmcore file created with the
> >>>>> same make dumpfile options.
> >
> > Interesting. Do you have any actual use case? e.g. used by kdumpctl?
> > or use it in kdump initramfs?
> >
>
> Yes, the idea would be to use this in mkdumprd to have a more accurate
> estimate of the dump size (currently it cannot take compression into
> account and warns about potential lack of space, considering the system
> memory size as a whole).
Hmm, I'm not sure how you are going to implement in mkdumprd, but I do not
recommend that you use it to determine how much disk space should be
allocated for crash dump. Because, I think that
- It cannot estimate the dump size when a real crash occurs, e.g. if slab
explodes with non-zero data, almost all memory will be captured by makedumpfile
even with -d 31, and compression ratio varies with data in memory.
Also, in most cases, mkdumprd runs at boot time or construction phase
with less memory usage, not at usual application running time. So it
can underestimate the needed size easily.
- The system might need a full vmcore and need to change makedumpfile's
dump level for an issue in the future. But many systems cannot change
their disk space allocation easily. So we should prevent users from
having minimum disk space for crash dump.
So, the following is from mkdumprd on Fedora 32, personally I think this
is good for now.
if [ $avail -lt $memtotal ]; then
echo "Warning: There might not be enough space to save a vmcore."
echo " The size of $2 should be greater than $memtotal kilo bytes."
fi
The patch's functionality itself might be useful and I don't reject, though.
> >>>>> @@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
> >>>>> }
> >>>>> if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
> >>>>> return FALSE;
> >>>>> + } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
> >>>>> + return write_buffer_update_size_info(offset, buf, buf_size);
> >
> > Why do we need this function? makedumpfile actually writes zero-filled
> > pages to the dumpfile with -d 0, and doesn't write them with -d 1.
> > So isn't "write_bytes += buf_size" enough? For example, with -d 30,
> >
>
> The reason I went with this method was to make an estimate of the number
> of blocks actually allocated on the disk (since depending on how the
> data written is scattered in the file, there might be a significant
> difference between bytes written vs actual size allocated on disk). But
> I realize that there is some misunderstanding from my end since written
> 0 do make block allocation as opposed to not writing at some offset
> (skipping the with lseek() ), I would need to fix that.
>
> To highlight the behaviour I'm talking about:
> $ dd if=/dev/zero of=./testfile bs=4096 count=1 seek=1
> 1+0 records in
> 1+0 records out
> 4096 bytes (4.1 kB, 4.0 KiB) copied, 0.000302719 s, 13.5 MB/s
> $ du -h testfile
> 4.0K testfile
>
> $ dd if=/dev/zero of=./testfile bs=4096 count=2
> 2+0 records in
> 2+0 records out
> 8192 bytes (8.2 kB, 8.0 KiB) copied, 0.000373002 s, 22.0 MB/s
> $ du -h testfile
> 8.0K testfile
>
>
> So, do you think it's not worth bothering estimating the number of
> blocks allocated an that I should only consider the number of bytes written?
Yes, makedumpfile almost doesn't make empty (sparse) blocks,
so the error would be small enough.
> >>>>
> >>>> I like the idea, but sometimes we use makedumpfile to generate a
> >>>> dumpfile in the primary kernel as well. For example:
> >>>>
> >>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile
> >>>>
> >>>> In such use-cases it is useful to use --vmcore-size and still generate
> >>>> the dumpfile (right now the default behaviour is not to generate a
> >>>> dumpfile when --vmcore-size is specified). Maybe we need to think more
> >>>> on supporting this use-case as well.
> >>>>
> >>>
> >>> The thing is, if you are generating the dumpfile, you can just check the
> >>> size of the file created with "du -b" or some other command.
> >>
> >> I agree, but I just was looking to replace the two 'makedumpfile +
> >> du' steps with a single 'makedumpfile --vmcore-size' step.
> >>
> >>> Overall I don't mind supporting your case as well. Maybe that can depend
> >>> on whether a vmcore/dumpfile filename is provided:
> >>>
> >>> $ makedumpfile -d 31 -x vmlinux /proc/kcore # only estimates the size
> >>>
> >>> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile # writes the
> >>> dumpfile and gives the final size
> >>>
> >>> Any thought, opinions, suggestions?
> >>
> >> Let's wait for Kazu's opinion on the same, but I am ok with using a
> >> two-step 'makedumpfile + du' approach for now (and later expand
> >> --vmcore-size as we encounter more use-cases).
> >>
> >> @Kazuhito Hagio : What's your opinion on the above?
> >
> > I would prefer only estimating with the option.
> >
> > And if the write_bytes method above is usable, it can be shown also
> > in report messages when wrote the dumpfile.
> >
>
> Let me know your preferred approach considering my comment above and
> I'll send out a v2.
I'm rethinking about what command options makedumpfile should have.
If once we add an option to makedumpfile, we cannot change it easily,
so I'd like to think carefully.
The calculated size might be useful if it's printed so that it can be
easily post-processed by scripts, e.g. for automated tests. If so,
makedumpfile already prints its statistics with "--message-level 16",
and it might be useful to also print them by an option like "--show-stats".
# makedumpfile --show-stats -l -d 31 vmcore dump.ld31
total_pages xxx
excluded_pages yyy
...
write_bytes zzz
Also, if we also have "--dry-run" option to not write actually, it's
explicit and meets Bhupesh's use case. What do you think?
Thanks,
Kazu
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-28 8:32 ` HAGIO KAZUHITO(萩尾 一仁)
@ 2020-10-29 12:43 ` lijiang
2020-10-30 6:29 ` HAGIO KAZUHITO(萩尾 一仁)
2020-11-18 3:57 ` lijiang
1 sibling, 1 reply; 13+ messages in thread
From: lijiang @ 2020-10-29 12:43 UTC (permalink / raw)
To: HAGIO KAZUHITO(萩尾 一仁),
Julien Thierry, Bhupesh Sharma
Cc: kexec mailing list
在 2020年10月28日 16:32, HAGIO KAZUHITO(萩尾 一仁) 写道:
> Hi Julien,
>
> sorry for my delayed reply.
>
> -----Original Message-----
>>>>>>> A user might want to know how much space a vmcore file will take on
>>>>>>> the system and how much space on their disk should be available to
>>>>>>> save it during a crash.
>>>>>>>
>>>>>>> The option --vmcore-size does not create the vmcore file but provides
>>>>>>> an estimation of the size of the final vmcore file created with the
>>>>>>> same make dumpfile options.
>>>
>>> Interesting. Do you have any actual use case? e.g. used by kdumpctl?
>>> or use it in kdump initramfs?
>>>
>>
>> Yes, the idea would be to use this in mkdumprd to have a more accurate
>> estimate of the dump size (currently it cannot take compression into
>> account and warns about potential lack of space, considering the system
>> memory size as a whole).
>
> Hmm, I'm not sure how you are going to implement in mkdumprd, but I do not
> recommend that you use it to determine how much disk space should be
> allocated for crash dump. Because, I think that
>
> - It cannot estimate the dump size when a real crash occurs, e.g. if slab
> explodes with non-zero data, almost all memory will be captured by makedumpfile
I agree with you, but this could be rare? If yes, I'm not sure if it is worth
thinking more about the rare situations.
> even with -d 31, and compression ratio varies with data in memory.
Indeed.
> Also, in most cases, mkdumprd runs at boot time or construction phase
> with less memory usage, not at usual application running time. So it
> can underestimate the needed size easily.
>
If administrator can monitor the estimated size periodically, maybe it
won't be a problem?
> - The system might need a full vmcore and need to change makedumpfile's
> dump level for an issue in the future. But many systems cannot change
> their disk space allocation easily. So we should prevent users from
> having minimum disk space for crash dump.
>
> So, the following is from mkdumprd on Fedora 32, personally I think this
> is good for now.
>
> if [ $avail -lt $memtotal ]; then
> echo "Warning: There might not be enough space to save a vmcore."
> echo " The size of $2 should be greater than $memtotal kilo bytes."
> fi
>
Currently, some users are complaining that mkdumprd overestimates the needed size,
and most vmcores are significantly smaller than the size of system memory.
Furthermore, in most cases, the system memory will not be completely exhausted, but
that still depends on how the memory is used in the system, for example:
[1] make the stressful test for memory
[2] always occupies amount of memory and not release it.
For the above two cases, there may be rare. Therefore, can we find out a compromise
between the size of vmcore and system memory so that makedumpfile can estimate the
size of vmcore more accurately?
And finally, mkdumprd can use the estimated size of vmcore instead of system memory(memtotal)
to determine if the target disk has enough space to store vmcore.
Thanks.
Lianbo
> The patch's functionality itself might be useful and I don't reject, though.
>
>>>>>>> @@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
>>>>>>> }
>>>>>>> if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
>>>>>>> return FALSE;
>>>>>>> + } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
>>>>>>> + return write_buffer_update_size_info(offset, buf, buf_size);
>>>
>>> Why do we need this function? makedumpfile actually writes zero-filled
>>> pages to the dumpfile with -d 0, and doesn't write them with -d 1.
>>> So isn't "write_bytes += buf_size" enough? For example, with -d 30,
>>>
>>
>> The reason I went with this method was to make an estimate of the number
>> of blocks actually allocated on the disk (since depending on how the
>> data written is scattered in the file, there might be a significant
>> difference between bytes written vs actual size allocated on disk). But
>> I realize that there is some misunderstanding from my end since written
>> 0 do make block allocation as opposed to not writing at some offset
>> (skipping the with lseek() ), I would need to fix that.
>>
>> To highlight the behaviour I'm talking about:
>> $ dd if=/dev/zero of=./testfile bs=4096 count=1 seek=1
>> 1+0 records in
>> 1+0 records out
>> 4096 bytes (4.1 kB, 4.0 KiB) copied, 0.000302719 s, 13.5 MB/s
>> $ du -h testfile
>> 4.0K testfile
>>
>> $ dd if=/dev/zero of=./testfile bs=4096 count=2
>> 2+0 records in
>> 2+0 records out
>> 8192 bytes (8.2 kB, 8.0 KiB) copied, 0.000373002 s, 22.0 MB/s
>> $ du -h testfile
>> 8.0K testfile
>>
>>
>> So, do you think it's not worth bothering estimating the number of
>> blocks allocated an that I should only consider the number of bytes written?
>
> Yes, makedumpfile almost doesn't make empty (sparse) blocks,
> so the error would be small enough.
>
>>>>>>
>>>>>> I like the idea, but sometimes we use makedumpfile to generate a
>>>>>> dumpfile in the primary kernel as well. For example:
>>>>>>
>>>>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile
>>>>>>
>>>>>> In such use-cases it is useful to use --vmcore-size and still generate
>>>>>> the dumpfile (right now the default behaviour is not to generate a
>>>>>> dumpfile when --vmcore-size is specified). Maybe we need to think more
>>>>>> on supporting this use-case as well.
>>>>>>
>>>>>
>>>>> The thing is, if you are generating the dumpfile, you can just check the
>>>>> size of the file created with "du -b" or some other command.
>>>>
>>>> I agree, but I just was looking to replace the two 'makedumpfile +
>>>> du' steps with a single 'makedumpfile --vmcore-size' step.
>>>>
>>>>> Overall I don't mind supporting your case as well. Maybe that can depend
>>>>> on whether a vmcore/dumpfile filename is provided:
>>>>>
>>>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore # only estimates the size
>>>>>
>>>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile # writes the
>>>>> dumpfile and gives the final size
>>>>>
>>>>> Any thought, opinions, suggestions?
>>>>
>>>> Let's wait for Kazu's opinion on the same, but I am ok with using a
>>>> two-step 'makedumpfile + du' approach for now (and later expand
>>>> --vmcore-size as we encounter more use-cases).
>>>>
>>>> @Kazuhito Hagio : What's your opinion on the above?
>>>
>>> I would prefer only estimating with the option.
>>>
>>> And if the write_bytes method above is usable, it can be shown also
>>> in report messages when wrote the dumpfile.
>>>
>>
>> Let me know your preferred approach considering my comment above and
>> I'll send out a v2.
>
> I'm rethinking about what command options makedumpfile should have.
> If once we add an option to makedumpfile, we cannot change it easily,
> so I'd like to think carefully.
>
> The calculated size might be useful if it's printed so that it can be
> easily post-processed by scripts, e.g. for automated tests. If so,
> makedumpfile already prints its statistics with "--message-level 16",
> and it might be useful to also print them by an option like "--show-stats".
>
> # makedumpfile --show-stats -l -d 31 vmcore dump.ld31
> total_pages xxx
> excluded_pages yyy
> ...
> write_bytes zzz
>
> Also, if we also have "--dry-run" option to not write actually, it's
> explicit and meets Bhupesh's use case. What do you think?
>
> Thanks,
> Kazu
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-29 12:43 ` lijiang
@ 2020-10-30 6:29 ` HAGIO KAZUHITO(萩尾 一仁)
2020-11-02 8:07 ` lijiang
0 siblings, 1 reply; 13+ messages in thread
From: HAGIO KAZUHITO(萩尾 一仁) @ 2020-10-30 6:29 UTC (permalink / raw)
To: lijiang, Julien Thierry, Bhupesh Sharma; +Cc: kexec mailing list
-----Original Message-----
> 在 2020年10月28日 16:32, HAGIO KAZUHITO(萩尾 一仁) 写道:
> > Hi Julien,
> >
> > sorry for my delayed reply.
> >
> > -----Original Message-----
> >>>>>>> A user might want to know how much space a vmcore file will take on
> >>>>>>> the system and how much space on their disk should be available to
> >>>>>>> save it during a crash.
> >>>>>>>
> >>>>>>> The option --vmcore-size does not create the vmcore file but provides
> >>>>>>> an estimation of the size of the final vmcore file created with the
> >>>>>>> same make dumpfile options.
> >>>
> >>> Interesting. Do you have any actual use case? e.g. used by kdumpctl?
> >>> or use it in kdump initramfs?
> >>>
> >>
> >> Yes, the idea would be to use this in mkdumprd to have a more accurate
> >> estimate of the dump size (currently it cannot take compression into
> >> account and warns about potential lack of space, considering the system
> >> memory size as a whole).
> >
> > Hmm, I'm not sure how you are going to implement in mkdumprd, but I do not
> > recommend that you use it to determine how much disk space should be
> > allocated for crash dump. Because, I think that
> >
> > - It cannot estimate the dump size when a real crash occurs, e.g. if slab
> > explodes with non-zero data, almost all memory will be captured by makedumpfile
>
> I agree with you, but this could be rare? If yes, I'm not sure if it is worth
> thinking more about the rare situations.
Cases that a dumpfile is inflated with -d 31 might be rare, but if users
need user data, e.g. for gcore, underestimation will occur easily.
>
> > even with -d 31, and compression ratio varies with data in memory.
>
> Indeed.
>
> > Also, in most cases, mkdumprd runs at boot time or construction phase
> > with less memory usage, not at usual application running time. So it
> > can underestimate the needed size easily.
> >
> If administrator can monitor the estimated size periodically, maybe it
> won't be a problem?
I think most of them cannot or do not do that, and even if they could do,
when a panic occurs by an unknown problem, can you depend on that estimation?
>
> > - The system might need a full vmcore and need to change makedumpfile's
> > dump level for an issue in the future. But many systems cannot change
> > their disk space allocation easily. So we should prevent users from
> > having minimum disk space for crash dump.
> >
> > So, the following is from mkdumprd on Fedora 32, personally I think this
> > is good for now.
> >
> > if [ $avail -lt $memtotal ]; then
> > echo "Warning: There might not be enough space to save a vmcore."
> > echo " The size of $2 should be greater than $memtotal kilo bytes."
> > fi
> >
> Currently, some users are complaining that mkdumprd overestimates the needed size,
> and most vmcores are significantly smaller than the size of system memory.
>
> Furthermore, in most cases, the system memory will not be completely exhausted, but
> that still depends on how the memory is used in the system, for example:
> [1] make the stressful test for memory
> [2] always occupies amount of memory and not release it.
>
> For the above two cases, there may be rare.
I've seen and worked on thousands of support cases, memory is exhausted
easily and unexpectedly.. Especially nowadays I often see panics by
vm.panic_on_oom.
> Therefore, can we find out a compromise
> between the size of vmcore and system memory so that makedumpfile can estimate the
> size of vmcore more accurately?
>
> And finally, mkdumprd can use the estimated size of vmcore instead of system memory(memtotal)
> to determine if the target disk has enough space to store vmcore.
The current mkdumprd just warns the possibility of lack of space,
it doesn't fail. I think this is a good balance.
Users can choose the estimated size over the whole memory size with
their discretion. Providing the useful estimation tool for them
might be good.
But, if we do so, we should let users know the tradeoff between the
disk space and the risk of failure. So I believe that we should
continue to warn the possibility of failure of capturing vmcore
with less space than the whole memory.
Thanks,
Kazu
>
>
> Thanks.
> Lianbo
>
> > The patch's functionality itself might be useful and I don't reject, though.
> >
> >>>>>>> @@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
> >>>>>>> }
> >>>>>>> if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
> >>>>>>> return FALSE;
> >>>>>>> + } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
> >>>>>>> + return write_buffer_update_size_info(offset, buf, buf_size);
> >>>
> >>> Why do we need this function? makedumpfile actually writes zero-filled
> >>> pages to the dumpfile with -d 0, and doesn't write them with -d 1.
> >>> So isn't "write_bytes += buf_size" enough? For example, with -d 30,
> >>>
> >>
> >> The reason I went with this method was to make an estimate of the number
> >> of blocks actually allocated on the disk (since depending on how the
> >> data written is scattered in the file, there might be a significant
> >> difference between bytes written vs actual size allocated on disk). But
> >> I realize that there is some misunderstanding from my end since written
> >> 0 do make block allocation as opposed to not writing at some offset
> >> (skipping the with lseek() ), I would need to fix that.
> >>
> >> To highlight the behaviour I'm talking about:
> >> $ dd if=/dev/zero of=./testfile bs=4096 count=1 seek=1
> >> 1+0 records in
> >> 1+0 records out
> >> 4096 bytes (4.1 kB, 4.0 KiB) copied, 0.000302719 s, 13.5 MB/s
> >> $ du -h testfile
> >> 4.0K testfile
> >>
> >> $ dd if=/dev/zero of=./testfile bs=4096 count=2
> >> 2+0 records in
> >> 2+0 records out
> >> 8192 bytes (8.2 kB, 8.0 KiB) copied, 0.000373002 s, 22.0 MB/s
> >> $ du -h testfile
> >> 8.0K testfile
> >>
> >>
> >> So, do you think it's not worth bothering estimating the number of
> >> blocks allocated an that I should only consider the number of bytes written?
> >
> > Yes, makedumpfile almost doesn't make empty (sparse) blocks,
> > so the error would be small enough.
> >
> >>>>>>
> >>>>>> I like the idea, but sometimes we use makedumpfile to generate a
> >>>>>> dumpfile in the primary kernel as well. For example:
> >>>>>>
> >>>>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile
> >>>>>>
> >>>>>> In such use-cases it is useful to use --vmcore-size and still generate
> >>>>>> the dumpfile (right now the default behaviour is not to generate a
> >>>>>> dumpfile when --vmcore-size is specified). Maybe we need to think more
> >>>>>> on supporting this use-case as well.
> >>>>>>
> >>>>>
> >>>>> The thing is, if you are generating the dumpfile, you can just check the
> >>>>> size of the file created with "du -b" or some other command.
> >>>>
> >>>> I agree, but I just was looking to replace the two 'makedumpfile +
> >>>> du' steps with a single 'makedumpfile --vmcore-size' step.
> >>>>
> >>>>> Overall I don't mind supporting your case as well. Maybe that can depend
> >>>>> on whether a vmcore/dumpfile filename is provided:
> >>>>>
> >>>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore # only estimates the size
> >>>>>
> >>>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile # writes the
> >>>>> dumpfile and gives the final size
> >>>>>
> >>>>> Any thought, opinions, suggestions?
> >>>>
> >>>> Let's wait for Kazu's opinion on the same, but I am ok with using a
> >>>> two-step 'makedumpfile + du' approach for now (and later expand
> >>>> --vmcore-size as we encounter more use-cases).
> >>>>
> >>>> @Kazuhito Hagio : What's your opinion on the above?
> >>>
> >>> I would prefer only estimating with the option.
> >>>
> >>> And if the write_bytes method above is usable, it can be shown also
> >>> in report messages when wrote the dumpfile.
> >>>
> >>
> >> Let me know your preferred approach considering my comment above and
> >> I'll send out a v2.
> >
> > I'm rethinking about what command options makedumpfile should have.
> > If once we add an option to makedumpfile, we cannot change it easily,
> > so I'd like to think carefully.
> >
> > The calculated size might be useful if it's printed so that it can be
> > easily post-processed by scripts, e.g. for automated tests. If so,
> > makedumpfile already prints its statistics with "--message-level 16",
> > and it might be useful to also print them by an option like "--show-stats".
> >
> > # makedumpfile --show-stats -l -d 31 vmcore dump.ld31
> > total_pages xxx
> > excluded_pages yyy
> > ...
> > write_bytes zzz
> >
> > Also, if we also have "--dry-run" option to not write actually, it's
> > explicit and meets Bhupesh's use case. What do you think?
> >
> > Thanks,
> > Kazu
> >
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec
> >
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-30 6:29 ` HAGIO KAZUHITO(萩尾 一仁)
@ 2020-11-02 8:07 ` lijiang
0 siblings, 0 replies; 13+ messages in thread
From: lijiang @ 2020-11-02 8:07 UTC (permalink / raw)
To: HAGIO KAZUHITO(萩尾 一仁),
Julien Thierry, Bhupesh Sharma
Cc: kexec mailing list
在 2020年10月30日 14:29, HAGIO KAZUHITO(萩尾 一仁) 写道:
> -----Original Message-----
>> 在 2020年10月28日 16:32, HAGIO KAZUHITO(萩尾 一仁) 写道:
>>> Hi Julien,
>>>
>>> sorry for my delayed reply.
>>>
>>> -----Original Message-----
>>>>>>>>> A user might want to know how much space a vmcore file will take on
>>>>>>>>> the system and how much space on their disk should be available to
>>>>>>>>> save it during a crash.
>>>>>>>>>
>>>>>>>>> The option --vmcore-size does not create the vmcore file but provides
>>>>>>>>> an estimation of the size of the final vmcore file created with the
>>>>>>>>> same make dumpfile options.
>>>>>
>>>>> Interesting. Do you have any actual use case? e.g. used by kdumpctl?
>>>>> or use it in kdump initramfs?
>>>>>
>>>>
>>>> Yes, the idea would be to use this in mkdumprd to have a more accurate
>>>> estimate of the dump size (currently it cannot take compression into
>>>> account and warns about potential lack of space, considering the system
>>>> memory size as a whole).
>>>
>>> Hmm, I'm not sure how you are going to implement in mkdumprd, but I do not
>>> recommend that you use it to determine how much disk space should be
>>> allocated for crash dump. Because, I think that
>>>
>>> - It cannot estimate the dump size when a real crash occurs, e.g. if slab
>>> explodes with non-zero data, almost all memory will be captured by makedumpfile
>>
>> I agree with you, but this could be rare? If yes, I'm not sure if it is worth
>> thinking more about the rare situations.
>
> Cases that a dumpfile is inflated with -d 31 might be rare, but if users
> need user data, e.g. for gcore, underestimation will occur easily.
>
Yes, that's true.
>>
>>> even with -d 31, and compression ratio varies with data in memory.
>>
>> Indeed.
>>
>>> Also, in most cases, mkdumprd runs at boot time or construction phase
>>> with less memory usage, not at usual application running time. So it
>>> can underestimate the needed size easily.
>>>
>> If administrator can monitor the estimated size periodically, maybe it
>> won't be a problem?
>
> I think most of them cannot or do not do that, and even if they could do,
> when a panic occurs by an unknown problem, can you depend on that estimation?
>
This requires user to evaluate the risk. The tools only provide a reference
value at a certain time point, and remind users of such risks.
>>
>>> - The system might need a full vmcore and need to change makedumpfile's
>>> dump level for an issue in the future. But many systems cannot change
>>> their disk space allocation easily. So we should prevent users from
>>> having minimum disk space for crash dump.
>>>
>>> So, the following is from mkdumprd on Fedora 32, personally I think this
>>> is good for now.
>>>
>>> if [ $avail -lt $memtotal ]; then
>>> echo "Warning: There might not be enough space to save a vmcore."
>>> echo " The size of $2 should be greater than $memtotal kilo bytes."
>>> fi
>>>
>> Currently, some users are complaining that mkdumprd overestimates the needed size,
>> and most vmcores are significantly smaller than the size of system memory.
>>
>> Furthermore, in most cases, the system memory will not be completely exhausted, but
>> that still depends on how the memory is used in the system, for example:
>> [1] make the stressful test for memory
>> [2] always occupies amount of memory and not release it.
>>
>> For the above two cases, there may be rare.
>
> I've seen and worked on thousands of support cases, memory is exhausted
> easily and unexpectedly.. Especially nowadays I often see panics by
> vm.panic_on_oom.
>
>> Therefore, can we find out a compromise
>> between the size of vmcore and system memory so that makedumpfile can estimate the
>> size of vmcore more accurately?
>>
>> And finally, mkdumprd can use the estimated size of vmcore instead of system memory(memtotal)
>> to determine if the target disk has enough space to store vmcore.
>
> The current mkdumprd just warns the possibility of lack of space,
> it doesn't fail. I think this is a good balance.
>
> Users can choose the estimated size over the whole memory size with
> their discretion. Providing the useful estimation tool for them
> might be good.
>
> But, if we do so, we should let users know the tradeoff between the
> disk space and the risk of failure. So I believe that we should
> continue to warn the possibility of failure of capturing vmcore
> with less space than the whole memory.
>
Our understanding is consistent about this issue. Maybe we could have a document
to explain the details.
Thanks.
Lianbo
> Thanks,
> Kazu
>
>
>>
>>
>> Thanks.
>> Lianbo
>>
>>> The patch's functionality itself might be useful and I don't reject, though.
>>>
>>>>>>>>> @@ -4643,6 +4706,8 @@ write_buffer(int fd, off_t offset, void *buf, size_t buf_size, char *file_name)
>>>>>>>>> }
>>>>>>>>> if (!write_and_check_space(fd, &fdh, sizeof(fdh), file_name))
>>>>>>>>> return FALSE;
>>>>>>>>> + } else if (info->flag_vmcore_size && fd == info->fd_dumpfile) {
>>>>>>>>> + return write_buffer_update_size_info(offset, buf, buf_size);
>>>>>
>>>>> Why do we need this function? makedumpfile actually writes zero-filled
>>>>> pages to the dumpfile with -d 0, and doesn't write them with -d 1.
>>>>> So isn't "write_bytes += buf_size" enough? For example, with -d 30,
>>>>>
>>>>
>>>> The reason I went with this method was to make an estimate of the number
>>>> of blocks actually allocated on the disk (since depending on how the
>>>> data written is scattered in the file, there might be a significant
>>>> difference between bytes written vs actual size allocated on disk). But
>>>> I realize that there is some misunderstanding from my end since written
>>>> 0 do make block allocation as opposed to not writing at some offset
>>>> (skipping the with lseek() ), I would need to fix that.
>>>>
>>>> To highlight the behaviour I'm talking about:
>>>> $ dd if=/dev/zero of=./testfile bs=4096 count=1 seek=1
>>>> 1+0 records in
>>>> 1+0 records out
>>>> 4096 bytes (4.1 kB, 4.0 KiB) copied, 0.000302719 s, 13.5 MB/s
>>>> $ du -h testfile
>>>> 4.0K testfile
>>>>
>>>> $ dd if=/dev/zero of=./testfile bs=4096 count=2
>>>> 2+0 records in
>>>> 2+0 records out
>>>> 8192 bytes (8.2 kB, 8.0 KiB) copied, 0.000373002 s, 22.0 MB/s
>>>> $ du -h testfile
>>>> 8.0K testfile
>>>>
>>>>
>>>> So, do you think it's not worth bothering estimating the number of
>>>> blocks allocated an that I should only consider the number of bytes written?
>>>
>>> Yes, makedumpfile almost doesn't make empty (sparse) blocks,
>>> so the error would be small enough.
>>>
>>>>>>>>
>>>>>>>> I like the idea, but sometimes we use makedumpfile to generate a
>>>>>>>> dumpfile in the primary kernel as well. For example:
>>>>>>>>
>>>>>>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile
>>>>>>>>
>>>>>>>> In such use-cases it is useful to use --vmcore-size and still generate
>>>>>>>> the dumpfile (right now the default behaviour is not to generate a
>>>>>>>> dumpfile when --vmcore-size is specified). Maybe we need to think more
>>>>>>>> on supporting this use-case as well.
>>>>>>>>
>>>>>>>
>>>>>>> The thing is, if you are generating the dumpfile, you can just check the
>>>>>>> size of the file created with "du -b" or some other command.
>>>>>>
>>>>>> I agree, but I just was looking to replace the two 'makedumpfile +
>>>>>> du' steps with a single 'makedumpfile --vmcore-size' step.
>>>>>>
>>>>>>> Overall I don't mind supporting your case as well. Maybe that can depend
>>>>>>> on whether a vmcore/dumpfile filename is provided:
>>>>>>>
>>>>>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore # only estimates the size
>>>>>>>
>>>>>>> $ makedumpfile -d 31 -x vmlinux /proc/kcore dumpfile # writes the
>>>>>>> dumpfile and gives the final size
>>>>>>>
>>>>>>> Any thought, opinions, suggestions?
>>>>>>
>>>>>> Let's wait for Kazu's opinion on the same, but I am ok with using a
>>>>>> two-step 'makedumpfile + du' approach for now (and later expand
>>>>>> --vmcore-size as we encounter more use-cases).
>>>>>>
>>>>>> @Kazuhito Hagio : What's your opinion on the above?
>>>>>
>>>>> I would prefer only estimating with the option.
>>>>>
>>>>> And if the write_bytes method above is usable, it can be shown also
>>>>> in report messages when wrote the dumpfile.
>>>>>
>>>>
>>>> Let me know your preferred approach considering my comment above and
>>>> I'll send out a v2.
>>>
>>> I'm rethinking about what command options makedumpfile should have.
>>> If once we add an option to makedumpfile, we cannot change it easily,
>>> so I'd like to think carefully.
>>>
>>> The calculated size might be useful if it's printed so that it can be
>>> easily post-processed by scripts, e.g. for automated tests. If so,
>>> makedumpfile already prints its statistics with "--message-level 16",
>>> and it might be useful to also print them by an option like "--show-stats".
>>>
>>> # makedumpfile --show-stats -l -d 31 vmcore dump.ld31
>>> total_pages xxx
>>> excluded_pages yyy
>>> ...
>>> write_bytes zzz
>>>
>>> Also, if we also have "--dry-run" option to not write actually, it's
>>> explicit and meets Bhupesh's use case. What do you think?
>>>
>>> Thanks,
>>> Kazu
>>>
>>> _______________________________________________
>>> kexec mailing list
>>> kexec@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/kexec
>>>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-10-28 8:32 ` HAGIO KAZUHITO(萩尾 一仁)
2020-10-29 12:43 ` lijiang
@ 2020-11-18 3:57 ` lijiang
2020-11-18 7:33 ` Julien Thierry
1 sibling, 1 reply; 13+ messages in thread
From: lijiang @ 2020-11-18 3:57 UTC (permalink / raw)
To: HAGIO KAZUHITO(萩尾 一仁),
Julien Thierry, Bhupesh Sharma
Cc: kexec mailing list
Hi, Kazu, Julien and Bhupesh
在 2020年10月28日 16:32, HAGIO KAZUHITO(萩尾 一仁) 写道:
> I'm rethinking about what command options makedumpfile should have.
> If once we add an option to makedumpfile, we cannot change it easily,
> so I'd like to think carefully.
>
> The calculated size might be useful if it's printed so that it can be
> easily post-processed by scripts, e.g. for automated tests. If so,
> makedumpfile already prints its statistics with "--message-level 16",
> and it might be useful to also print them by an option like "--show-stats".
>
> # makedumpfile --show-stats -l -d 31 vmcore dump.ld31
> total_pages xxx
> excluded_pages yyy
> ...
> write_bytes zzz
>
> Also, if we also have "--dry-run" option to not write actually, it's
> explicit and meets Bhupesh's use case. What do you think?
>
It seems that adding a statistical option could be better than nothing.
Do you have any decisions on this issue? Or any thoughts?
Thanks
Lianbo
> Thanks,
> Kazu
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files
2020-11-18 3:57 ` lijiang
@ 2020-11-18 7:33 ` Julien Thierry
0 siblings, 0 replies; 13+ messages in thread
From: Julien Thierry @ 2020-11-18 7:33 UTC (permalink / raw)
To: lijiang, HAGIO KAZUHITO(萩尾 一仁),
Bhupesh Sharma
Cc: kexec mailing list
Hi,
On 11/18/20 3:57 AM, lijiang wrote:
> Hi, Kazu, Julien and Bhupesh
>
> 在 2020年10月28日 16:32, HAGIO KAZUHITO(萩尾 一仁) 写道:
>> I'm rethinking about what command options makedumpfile should have.
>> If once we add an option to makedumpfile, we cannot change it easily,
>> so I'd like to think carefully.
>>
>> The calculated size might be useful if it's printed so that it can be
>> easily post-processed by scripts, e.g. for automated tests. If so,
>> makedumpfile already prints its statistics with "--message-level 16",
>> and it might be useful to also print them by an option like "--show-stats".
>>
>> # makedumpfile --show-stats -l -d 31 vmcore dump.ld31
>> total_pages xxx
>> excluded_pages yyy
>> ...
>> write_bytes zzz
>>
>> Also, if we also have "--dry-run" option to not write actually, it's
>> explicit and meets Bhupesh's use case. What do you think?
>>
>
> It seems that adding a statistical option could be better than nothing.
>
> Do you have any decisions on this issue? Or any thoughts?
>
On my end this makes sense, I'll try to add the --dry-run and
--show-stats options as Kazu suggested and post a new version.
--
Julien Thierry
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-11-18 7:33 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-12 7:09 [MAKDUMPFILE PATCH] Add option to estimate the size of vmcore dump files Julien Thierry
2020-10-13 9:27 ` Bhupesh Sharma
2020-10-13 9:53 ` Julien Thierry
2020-10-13 19:45 ` Bhupesh Sharma
2020-10-16 6:45 ` HAGIO KAZUHITO(萩尾 一仁)
2020-10-20 11:36 ` Julien Thierry
2020-10-28 8:32 ` HAGIO KAZUHITO(萩尾 一仁)
2020-10-29 12:43 ` lijiang
2020-10-30 6:29 ` HAGIO KAZUHITO(萩尾 一仁)
2020-11-02 8:07 ` lijiang
2020-11-18 3:57 ` lijiang
2020-11-18 7:33 ` Julien Thierry
2020-10-14 4:31 ` lijiang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.