[PATCH] kexec-tools: Remove duplicate ultoa() definitions and redefine it
There exist duplicate ultoa() definitions in many archs, remove them, and also redefine ultoa() in kexec/kexec.h to make it more readable. Signed-off-by: Tiezhu Yang --- kexec/arch/i386/crashdump-x86.c| 19 --- kexec/arch/mips/crashdump-mips.c | 19 --- kexec/arch/mips/kexec-elf-mips.c | 19 --- kexec/arch/ppc64/crashdump-ppc64.c | 19 --- kexec/arch/sh/crashdump-sh.c | 19 --- kexec/kexec.h | 16 6 files changed, 16 insertions(+), 95 deletions(-) diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c index 9826f6d..df1f24c 100644 --- a/kexec/arch/i386/crashdump-x86.c +++ b/kexec/arch/i386/crashdump-x86.c @@ -601,25 +601,6 @@ static int delete_memmap(struct memory_range *memmap_p, int *nr_memmap, return 0; } -/* Converts unsigned long to ascii string. */ -static void ultoa(unsigned long i, char *str) -{ - int j = 0, k; - char tmp; - - do { - str[j++] = i % 10 + '0'; - } while ((i /=10) > 0); - str[j] = '\0'; - - /* Reverse the string. */ - for (j = 0, k = strlen(str) - 1; j < k; j++, k--) { - tmp = str[k]; - str[k] = str[j]; - str[j] = tmp; - } -} - static void cmdline_add_memmap_internal(char *cmdline, unsigned long startk, unsigned long endk, int type) { diff --git a/kexec/arch/mips/crashdump-mips.c b/kexec/arch/mips/crashdump-mips.c index d9f4515..00f4335 100644 --- a/kexec/arch/mips/crashdump-mips.c +++ b/kexec/arch/mips/crashdump-mips.c @@ -224,25 +224,6 @@ static int get_crash_memory_ranges(struct memory_range **range, int *ranges) return 0; } -/* Converts unsigned long to ascii string. */ -static void ultoa(unsigned long i, char *str) -{ - int j = 0, k; - char tmp; - - do { - str[j++] = i % 10 + '0'; - } while ((i /= 10) > 0); - str[j] = '\0'; - - /* Reverse the string. */ - for (j = 0, k = strlen(str) - 1; j < k; j++, k--) { - tmp = str[k]; - str[k] = str[j]; - str[j] = tmp; - } -} - /* Adds the appropriate mem= options to command line, indicating the * memory region the new kernel can use to boot into. */ static int cmdline_add_mem(char *cmdline, unsigned long addr, diff --git a/kexec/arch/mips/kexec-elf-mips.c b/kexec/arch/mips/kexec-elf-mips.c index 31f4c47..230d806 100644 --- a/kexec/arch/mips/kexec-elf-mips.c +++ b/kexec/arch/mips/kexec-elf-mips.c @@ -40,25 +40,6 @@ static const int probe_debug = 0; #define CMDLINE_PREFIX "kexec " static char cmdline_buf[COMMAND_LINE_SIZE] = CMDLINE_PREFIX; -/* Converts unsigned long to ascii string. */ -static void ultoa(unsigned long i, char *str) -{ - int j = 0, k; - char tmp; - - do { - str[j++] = i % 10 + '0'; - } while ((i /= 10) > 0); - str[j] = '\0'; - - /* Reverse the string. */ - for (j = 0, k = strlen(str) - 1; j < k; j++, k--) { - tmp = str[k]; - str[k] = str[j]; - str[j] = tmp; - } -} - /* Adds initrd parameters to command line. */ static int cmdline_add_initrd(char *cmdline, unsigned long addr, char *new_para) { diff --git a/kexec/arch/ppc64/crashdump-ppc64.c b/kexec/arch/ppc64/crashdump-ppc64.c index 91f9521..6d47898 100644 --- a/kexec/arch/ppc64/crashdump-ppc64.c +++ b/kexec/arch/ppc64/crashdump-ppc64.c @@ -436,25 +436,6 @@ err: return -1; } -/* Converts unsigned long to ascii string. */ -static void ultoa(uint64_t i, char *str) -{ - int j = 0, k; - char tmp; - - do { - str[j++] = i % 10 + '0'; - } while ((i /=10) > 0); - str[j] = '\0'; - - /* Reverse the string. */ - for (j = 0, k = strlen(str) - 1; j < k; j++, k--) { - tmp = str[k]; - str[k] = str[j]; - str[j] = tmp; - } -} - static int add_cmdline_param(char *cmdline, uint64_t addr, char *cmdstr, char *byte) { diff --git a/kexec/arch/sh/crashdump-sh.c b/kexec/arch/sh/crashdump-sh.c index aa25dea..36e9aaf 100644 --- a/kexec/arch/sh/crashdump-sh.c +++ b/kexec/arch/sh/crashdump-sh.c @@ -90,25 +90,6 @@ static struct crash_elf_info elf_info32 = page_offset: PAGE_OFFSET, }; -/* Converts unsigned long to ascii string. */ -static void ultoa(unsigned long i, char *str) -{ - int j = 0, k; - char tmp; - - do { - str[j++] = i % 10 + '0'; - } while ((i /=10) > 0); - str[j] = '\0'; - - /* Reverse the string. */ - for (j = 0, k = strlen(str) - 1; j < k; j++, k--) { - tmp = str[k]; - str[k] = str[j]; - str[j] = tmp; - } -} - static int add_cmdline_param(ch
[PATCH v2 5/5] kasan: no need to unset panic_on_warn in end_report()
panic_on_warn is unset inside panic(), so no need to unset it before calling panic() in end_report(). Signed-off-by: Tiezhu Yang --- mm/kasan/report.c | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/mm/kasan/report.c b/mm/kasan/report.c index 3ad9624..f141465 100644 --- a/mm/kasan/report.c +++ b/mm/kasan/report.c @@ -117,16 +117,8 @@ static void end_report(unsigned long *flags, unsigned long addr) pr_err("==\n"); add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE); spin_unlock_irqrestore(_lock, *flags); - if (panic_on_warn && !test_bit(KASAN_BIT_MULTI_SHOT, _flags)) { - /* -* This thread may hit another WARN() in the panic path. -* Resetting this prevents additional WARN() from panicking the -* system on this thread. Other threads are blocked by the -* panic_mutex in panic(). -*/ - panic_on_warn = 0; + if (panic_on_warn && !test_bit(KASAN_BIT_MULTI_SHOT, _flags)) panic("panic_on_warn set ...\n"); - } if (kasan_arg_fault == KASAN_ARG_FAULT_PANIC) panic("kasan.fault=panic set ...\n"); kasan_enable_current(); -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 2/5] docs: kdump: add scp example to write out the dump file
Except cp and makedumpfile, add scp example to write out the dump file. Signed-off-by: Tiezhu Yang Acked-by: Baoquan He --- Documentation/admin-guide/kdump/kdump.rst | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst index d187df2..a748e7e 100644 --- a/Documentation/admin-guide/kdump/kdump.rst +++ b/Documentation/admin-guide/kdump/kdump.rst @@ -533,6 +533,10 @@ the following command:: cp /proc/vmcore +or use scp to write out the dump file between hosts on a network, e.g:: + + scp /proc/vmcore remote_username@remote_ip: + You can also use makedumpfile utility to write out the dump file with specified options to filter out unwanted contents, e.g:: -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 1/5] docs: kdump: update description about sysfs file system support
After commit 6a108a14fa35 ("kconfig: rename CONFIG_EMBEDDED to CONFIG_EXPERT"), "Configure standard kernel features (for small systems)" is not exist, we should use "Configure standard kernel features (expert users)" now. Signed-off-by: Tiezhu Yang Acked-by: Baoquan He --- Documentation/admin-guide/kdump/kdump.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst index cb30ca3d..d187df2 100644 --- a/Documentation/admin-guide/kdump/kdump.rst +++ b/Documentation/admin-guide/kdump/kdump.rst @@ -146,9 +146,9 @@ System kernel config options CONFIG_SYSFS=y Note that "sysfs file system support" might not appear in the "Pseudo - filesystems" menu if "Configure standard kernel features (for small - systems)" is not enabled in "General Setup." In this case, check the - .config file itself to ensure that sysfs is turned on, as follows:: + filesystems" menu if "Configure standard kernel features (expert users)" + is not enabled in "General Setup." In this case, check the .config file + itself to ensure that sysfs is turned on, as follows:: grep 'CONFIG_SYSFS' .config -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 4/5] ubsan: no need to unset panic_on_warn in ubsan_epilogue()
panic_on_warn is unset inside panic(), so no need to unset it before calling panic() in ubsan_epilogue(). Signed-off-by: Tiezhu Yang --- lib/ubsan.c | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/ubsan.c b/lib/ubsan.c index bdc380f..36bd75e 100644 --- a/lib/ubsan.c +++ b/lib/ubsan.c @@ -154,16 +154,8 @@ static void ubsan_epilogue(void) current->in_ubsan--; - if (panic_on_warn) { - /* -* This thread may hit another WARN() in the panic path. -* Resetting this prevents additional WARN() from panicking the -* system on this thread. Other threads are blocked by the -* panic_mutex in panic(). -*/ - panic_on_warn = 0; + if (panic_on_warn) panic("panic_on_warn set ...\n"); - } } void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 3/5] panic: unset panic_on_warn inside panic()
In the current code, the following three places need to unset panic_on_warn before calling panic() to avoid recursive panics: kernel/kcsan/report.c: print_report() kernel/sched/core.c: __schedule_bug() mm/kfence/report.c: kfence_report_error() In order to avoid copy-pasting "panic_on_warn = 0" all over the places, it is better to move it inside panic() and then remove it from the other places. Signed-off-by: Tiezhu Yang --- kernel/panic.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/kernel/panic.c b/kernel/panic.c index 55b50e0..95ba825 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -185,6 +185,16 @@ void panic(const char *fmt, ...) int old_cpu, this_cpu; bool _crash_kexec_post_notifiers = crash_kexec_post_notifiers; + if (panic_on_warn) { + /* +* This thread may hit another WARN() in the panic path. +* Resetting this prevents additional WARN() from panicking the +* system on this thread. Other threads are blocked by the +* panic_mutex in panic(). +*/ + panic_on_warn = 0; + } + /* * Disable local interrupts. This will prevent panic_smp_self_stop * from deadlocking the first cpu that invokes the panic, since @@ -576,16 +586,8 @@ void __warn(const char *file, int line, void *caller, unsigned taint, if (regs) show_regs(regs); - if (panic_on_warn) { - /* -* This thread may hit another WARN() in the panic path. -* Resetting this prevents additional WARN() from panicking the -* system on this thread. Other threads are blocked by the -* panic_mutex in panic(). -*/ - panic_on_warn = 0; + if (panic_on_warn) panic("panic_on_warn set ...\n"); - } if (!regs) dump_stack(); -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 0/5] Update doc and fix some issues about kdump
Tiezhu Yang (5): docs: kdump: update description about sysfs file system support docs: kdump: add scp example to write out the dump file panic: unset panic_on_warn inside panic() ubsan: no need to unset panic_on_warn in ubsan_epilogue() kasan: no need to unset panic_on_warn in end_report() Documentation/admin-guide/kdump/kdump.rst | 10 +++--- kernel/panic.c| 20 +++- lib/ubsan.c | 10 +- mm/kasan/report.c | 10 +- 4 files changed, 20 insertions(+), 30 deletions(-) -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 4/5] sched: unset panic_on_warn before calling panic()
On 1/28/22 19:52, Marco Elver wrote: On Fri, 28 Jan 2022 at 12:42, Tiezhu Yang wrote: As done in the full WARN() handler, panic_on_warn needs to be cleared before calling panic() to avoid recursive panics. Signed-off-by: Tiezhu Yang --- kernel/sched/core.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 848eaa0..f5b0886 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -5524,8 +5524,17 @@ static noinline void __schedule_bug(struct task_struct *prev) pr_err("Preemption disabled at:"); print_ip_sym(KERN_ERR, preempt_disable_ip); } - if (panic_on_warn) + + if (panic_on_warn) { + /* +* This thread may hit another WARN() in the panic path. +* Resetting this prevents additional WARN() from panicking the +* system on this thread. Other threads are blocked by the +* panic_mutex in panic(). +*/ + panic_on_warn = 0; panic("scheduling while atomic\n"); I agree this is worth fixing. But: Why can't the "panic_on_warn = 0" just be moved inside panic(), instead of copy-pasting this all over the place? OK, it looks better. Let me wait for some days, if no more comments, I will send v2 to move "panic_on_warn = 0" inside panic() and remove it from the other places, like this: diff --git a/kernel/panic.c b/kernel/panic.c index 55b50e052ec3..95ba825522dd 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -185,6 +185,16 @@ void panic(const char *fmt, ...) int old_cpu, this_cpu; bool _crash_kexec_post_notifiers = crash_kexec_post_notifiers; + if (panic_on_warn) { + /* +* This thread may hit another WARN() in the panic path. +* Resetting this prevents additional WARN() from panicking the +* system on this thread. Other threads are blocked by the +* panic_mutex in panic(). +*/ + panic_on_warn = 0; + } + /* * Disable local interrupts. This will prevent panic_smp_self_stop * from deadlocking the first cpu that invokes the panic, since @@ -576,16 +586,8 @@ void __warn(const char *file, int line, void *caller, unsigned taint, if (regs) show_regs(regs); - if (panic_on_warn) { - /* -* This thread may hit another WARN() in the panic path. -* Resetting this prevents additional WARN() from panicking the -* system on this thread. Other threads are blocked by the -* panic_mutex in panic(). -*/ - panic_on_warn = 0; + if (panic_on_warn) panic("panic_on_warn set ...\n"); - } if (!regs) dump_stack(); diff --git a/lib/ubsan.c b/lib/ubsan.c index bdc380ff5d5c..36bd75e33426 100644 --- a/lib/ubsan.c +++ b/lib/ubsan.c @@ -154,16 +154,8 @@ static void ubsan_epilogue(void) current->in_ubsan--; - if (panic_on_warn) { - /* -* This thread may hit another WARN() in the panic path. -* Resetting this prevents additional WARN() from panicking the -* system on this thread. Other threads are blocked by the -* panic_mutex in panic(). -*/ - panic_on_warn = 0; + if (panic_on_warn) panic("panic_on_warn set ...\n"); - } } void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) diff --git a/mm/kasan/report.c b/mm/kasan/report.c index 3ad9624dcc56..f14146563d41 100644 --- a/mm/kasan/report.c +++ b/mm/kasan/report.c @@ -117,16 +117,8 @@ static void end_report(unsigned long *flags, unsigned long addr) pr_err("==\n"); add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE); spin_unlock_irqrestore(_lock, *flags); - if (panic_on_warn && !test_bit(KASAN_BIT_MULTI_SHOT, _flags)) { - /* -* This thread may hit another WARN() in the panic path. -* Resetting this prevents additional WARN() from panicking the -* system on this thread. Other threads are blocked by the -* panic_mutex in panic(). -*/ - panic_on_warn = 0; + if (panic_on_warn && !test_bit(KASAN_BIT_MULTI_SHOT, _flags)) panic("panic_on_warn set ...\n"); - } if (kasan_arg_fault == KASAN_ARG_FAULT_PANIC) panic("kasan.fault=panic set ...\n"); kasan_enable_current(); Thanks, Tiezhu I may be missing something obvious why this hasn't been
[PATCH 2/5] docs: kdump: add scp sample to write out the dump file
Except cp and makedumpfile, add scp sample to write out the dump file. Signed-off-by: Tiezhu Yang --- Documentation/admin-guide/kdump/kdump.rst | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst index d187df2..a748e7e 100644 --- a/Documentation/admin-guide/kdump/kdump.rst +++ b/Documentation/admin-guide/kdump/kdump.rst @@ -533,6 +533,10 @@ the following command:: cp /proc/vmcore +or use scp to write out the dump file between hosts on a network, e.g:: + + scp /proc/vmcore remote_username@remote_ip: + You can also use makedumpfile utility to write out the dump file with specified options to filter out unwanted contents, e.g:: -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 0/5] Update doc and fix some issues about kdump
Tiezhu Yang (5): docs: kdump: update description about sysfs file system support docs: kdump: add scp sample to write out the dump file kcsan: unset panic_on_warn before calling panic() sched: unset panic_on_warn before calling panic() kfence: unset panic_on_warn before calling panic() Documentation/admin-guide/kdump/kdump.rst | 10 +++--- kernel/kcsan/report.c | 10 +- kernel/sched/core.c | 11 ++- mm/kfence/report.c| 10 +- 4 files changed, 35 insertions(+), 6 deletions(-) -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 1/5] docs: kdump: update description about sysfs file system support
After commit 6a108a14fa35 ("kconfig: rename CONFIG_EMBEDDED to CONFIG_EXPERT"), "Configure standard kernel features (for small systems)" is not exist, we should use "Configure standard kernel features (expert users)" now. Signed-off-by: Tiezhu Yang --- Documentation/admin-guide/kdump/kdump.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst index cb30ca3d..d187df2 100644 --- a/Documentation/admin-guide/kdump/kdump.rst +++ b/Documentation/admin-guide/kdump/kdump.rst @@ -146,9 +146,9 @@ System kernel config options CONFIG_SYSFS=y Note that "sysfs file system support" might not appear in the "Pseudo - filesystems" menu if "Configure standard kernel features (for small - systems)" is not enabled in "General Setup." In this case, check the - .config file itself to ensure that sysfs is turned on, as follows:: + filesystems" menu if "Configure standard kernel features (expert users)" + is not enabled in "General Setup." In this case, check the .config file + itself to ensure that sysfs is turned on, as follows:: grep 'CONFIG_SYSFS' .config -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 4/5] sched: unset panic_on_warn before calling panic()
As done in the full WARN() handler, panic_on_warn needs to be cleared before calling panic() to avoid recursive panics. Signed-off-by: Tiezhu Yang --- kernel/sched/core.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 848eaa0..f5b0886 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -5524,8 +5524,17 @@ static noinline void __schedule_bug(struct task_struct *prev) pr_err("Preemption disabled at:"); print_ip_sym(KERN_ERR, preempt_disable_ip); } - if (panic_on_warn) + + if (panic_on_warn) { + /* +* This thread may hit another WARN() in the panic path. +* Resetting this prevents additional WARN() from panicking the +* system on this thread. Other threads are blocked by the +* panic_mutex in panic(). +*/ + panic_on_warn = 0; panic("scheduling while atomic\n"); + } dump_stack(); add_taint(TAINT_WARN, LOCKDEP_STILL_OK); -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 3/5] kcsan: unset panic_on_warn before calling panic()
As done in the full WARN() handler, panic_on_warn needs to be cleared before calling panic() to avoid recursive panics. Signed-off-by: Tiezhu Yang --- kernel/kcsan/report.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/kernel/kcsan/report.c b/kernel/kcsan/report.c index 6779440..752ab33 100644 --- a/kernel/kcsan/report.c +++ b/kernel/kcsan/report.c @@ -492,8 +492,16 @@ static void print_report(enum kcsan_value_change value_change, dump_stack_print_info(KERN_DEFAULT); pr_err("==\n"); - if (panic_on_warn) + if (panic_on_warn) { + /* +* This thread may hit another WARN() in the panic path. +* Resetting this prevents additional WARN() from panicking the +* system on this thread. Other threads are blocked by the +* panic_mutex in panic(). +*/ + panic_on_warn = 0; panic("panic_on_warn set ...\n"); + } } static void release_report(unsigned long *flags, struct other_info *other_info) -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 5/5] kfence: unset panic_on_warn before calling panic()
As done in the full WARN() handler, panic_on_warn needs to be cleared before calling panic() to avoid recursive panics. Signed-off-by: Tiezhu Yang --- mm/kfence/report.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/mm/kfence/report.c b/mm/kfence/report.c index f93a7b2..9d61a23 100644 --- a/mm/kfence/report.c +++ b/mm/kfence/report.c @@ -267,8 +267,16 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r lockdep_on(); - if (panic_on_warn) + if (panic_on_warn) { + /* +* This thread may hit another WARN() in the panic path. +* Resetting this prevents additional WARN() from panicking the +* system on this thread. Other threads are blocked by the +* panic_mutex in panic(). +*/ + panic_on_warn = 0; panic("panic_on_warn set ...\n"); + } /* We encountered a memory safety error, taint the kernel! */ add_taint(TAINT_BAD_PAGE, LOCKDEP_STILL_OK); -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH] kdump: Add support for crashkernel=auto
On 01/27/2022 11:53 PM, Petr Tesařík wrote: Hi Tiezhu Yang, I'm afraid the whole concept is broken by design. See below. Dne 27. 01. 22 v 10:31 Tiezhu Yang napsal(a): Set the reserved memory automatically for the crash kernel based on architecture. Most code of this patch come from: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-8/-/tree/c8s And that's the problem, I think. The solution might be good for this specific OS, but not for others. Hi Petr, Thank you for your reply. This is a RFC patch, the initial aim of this patch is to discuss what is the proper way to support crashkernel=auto. A moment ago, I find the following patch, it is more flexible, but it is not merged into the upstream kernel now. kernel/crash_core: Add crashkernel=auto for vmcore creation https://lore.kernel.org/lkml/20210223174153.72802-1-saeed.mirzamohamm...@oracle.com/ [...] diff --git a/kernel/crash_core.c b/kernel/crash_core.c index 256cf6d..32c51e2 100644 --- a/kernel/crash_core.c +++ b/kernel/crash_core.c @@ -252,6 +252,26 @@ static int __init __parse_crashkernel(char *cmdline, if (suffix) return parse_crashkernel_suffix(ck_cmdline, crash_size, suffix); + +if (strncmp(ck_cmdline, "auto", 4) == 0) { +#if defined(CONFIG_X86_64) || defined(CONFIG_S390) +ck_cmdline = "1G-4G:160M,4G-64G:192M,64G-1T:256M,1T-:512M"; +#elif defined(CONFIG_ARM64) +ck_cmdline = "2G-:448M"; +#elif defined(CONFIG_PPC64) +char *fadump_cmdline; + +fadump_cmdline = get_last_crashkernel(cmdline, "fadump=", NULL); +fadump_cmdline = fadump_cmdline ? +fadump_cmdline + strlen("fadump=") : NULL; +if (!fadump_cmdline || (strncmp(fadump_cmdline, "off", 3) == 0)) +ck_cmdline = "2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G"; +else +ck_cmdline = "4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"; +#endif +pr_info("Using crashkernel=auto, the size chosen is a best effort estimation.\n"); +} + How did you even arrive at the above numbers? Memory requirements for kdump: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/managing_monitoring_and_updating_the_kernel/supported-kdump-configurations-and-targets_managing-monitoring-and-updating-the-kernel#memory-requirements-for-kdump_supported-kdump-configurations-and-targets I've done some research on this topic recently (ie. during the last 7 years or so). My x86_64 system with 8G RAM running openSUSE Leap 15.3 seems needs 188M for saving to the local disk, and 203M to save over the network (using SFTP). My PPC64 LPAR with 16G RAM running latest Beta of SLES 15 SP4 needs 587M, i.e. with the above numbers it may run out of memory while saving the dump. Since this is not the first time, I'm trying to explain things, I've written a blog post now: https://sigillatum.tesarici.cz/2022-01-27-whats-wrong-with-crashkernel-auto.html Thank you, this is useful. Thanks, Tiezhu HTH Petr Tesarik ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[RFC PATCH] kdump: Add support for crashkernel=auto
Set the reserved memory automatically for the crash kernel based on architecture. Most code of this patch come from: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-8/-/tree/c8s Signed-off-by: Tiezhu Yang --- Documentation/admin-guide/kdump/kdump.rst | 13 + Documentation/admin-guide/kernel-parameters.txt | 5 + kernel/crash_core.c | 20 3 files changed, 38 insertions(+) diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst index cb30ca3d..8f8a9cc 100644 --- a/Documentation/admin-guide/kdump/kdump.rst +++ b/Documentation/admin-guide/kdump/kdump.rst @@ -335,6 +335,19 @@ crashkernel syntax crashkernel=0,low +4) crashkernel=auto + + You can use crashkernel=auto if you have enough memory. The threshold + is 1G on x86_64 and s390x, 2G on arm64, ppc64 and ppc64le. If your system + memory is less than the threshold crashkernel=auto will not reserve memory. + + The automatically reserved memory size varies based on architecture. + The size changes according to system memory size like below: + x86_64: 1G-4G:160M,4G-64G:192M,64G-1T:256M,1T-:512M + s390x: 1G-4G:160M,4G-64G:192M,64G-1T:256M,1T-:512M + arm64: 2G-:448M + ppc64: 2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G + Boot into System Kernel --- 1) Update the boot loader (such as grub, yaboot, or lilo) configuration diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index f5a27f0..14f052d 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -783,6 +783,11 @@ Format: ,,,[,] + crashkernel=auto + [KNL] Set the reserved memory automatically for the crash kernel + based on architecture. + See Documentation/admin-guide/kdump/kdump.rst for further details. + crashkernel=size[KMG][@offset[KMG]] [KNL] Using kexec, Linux can switch to a 'crash kernel' upon panic. This parameter reserves the physical diff --git a/kernel/crash_core.c b/kernel/crash_core.c index 256cf6d..32c51e2 100644 --- a/kernel/crash_core.c +++ b/kernel/crash_core.c @@ -252,6 +252,26 @@ static int __init __parse_crashkernel(char *cmdline, if (suffix) return parse_crashkernel_suffix(ck_cmdline, crash_size, suffix); + + if (strncmp(ck_cmdline, "auto", 4) == 0) { +#if defined(CONFIG_X86_64) || defined(CONFIG_S390) + ck_cmdline = "1G-4G:160M,4G-64G:192M,64G-1T:256M,1T-:512M"; +#elif defined(CONFIG_ARM64) + ck_cmdline = "2G-:448M"; +#elif defined(CONFIG_PPC64) + char *fadump_cmdline; + + fadump_cmdline = get_last_crashkernel(cmdline, "fadump=", NULL); + fadump_cmdline = fadump_cmdline ? + fadump_cmdline + strlen("fadump=") : NULL; + if (!fadump_cmdline || (strncmp(fadump_cmdline, "off", 3) == 0)) + ck_cmdline = "2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G"; + else + ck_cmdline = "4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-:180G"; +#endif + pr_info("Using crashkernel=auto, the size chosen is a best effort estimation.\n"); + } + /* * if the commandline contains a ':', then that's the extended * syntax -- if not, it must be the classic syntax -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 0/2] kexec-tools: mips: Modify some code
Tiezhu Yang (2): kexec-tools: mips: Add some debug info kexec-tools: mips: Concatenate --reuse-cmdline and --append kexec/arch/mips/kexec-mips.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 2/2] kexec-tools: mips: Concatenate --reuse-cmdline and --append
Use concat_cmdline() to concatenate the --append string and the --reuse-cmdline string, otherwise only one of the two options is valid. This is similar with commit 8b42c99aa3bc ("Fix --reuse-cmdline so it is usable."). Signed-off-by: Tiezhu Yang --- kexec/arch/mips/kexec-mips.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/kexec/arch/mips/kexec-mips.c b/kexec/arch/mips/kexec-mips.c index 10ae45b..d8cbea8 100644 --- a/kexec/arch/mips/kexec-mips.c +++ b/kexec/arch/mips/kexec-mips.c @@ -109,15 +109,17 @@ int arch_process_options(int argc, char **argv) }; static const char short_options[] = KEXEC_ARCH_OPT_STR; int opt; + char *cmdline = NULL; + const char *append = NULL; while ((opt = getopt_long(argc, argv, short_options, options, 0)) != -1) { switch (opt) { case OPT_APPEND: - arch_options.command_line = optarg; + append = optarg; break; case OPT_REUSE_CMDLINE: - arch_options.command_line = get_command_line(); + cmdline = get_command_line(); break; case OPT_DTB: arch_options.dtb_file = optarg; @@ -130,6 +132,8 @@ int arch_process_options(int argc, char **argv) } } + arch_options.command_line = concat_cmdline(cmdline, append); + dbgprintf("%s:%d: command_line: %s\n", __func__, __LINE__, arch_options.command_line); dbgprintf("%s:%d: initrd: %s\n", __func__, __LINE__, -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 1/2] kexec-tools: mips: Add some debug info
Use dbgprintf() to print command_line, initrd and dtb in arch_process_options() for debugging. Signed-off-by: Tiezhu Yang --- kexec/arch/mips/kexec-mips.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/kexec/arch/mips/kexec-mips.c b/kexec/arch/mips/kexec-mips.c index 5866e24..10ae45b 100644 --- a/kexec/arch/mips/kexec-mips.c +++ b/kexec/arch/mips/kexec-mips.c @@ -130,6 +130,13 @@ int arch_process_options(int argc, char **argv) } } + dbgprintf("%s:%d: command_line: %s\n", __func__, __LINE__, + arch_options.command_line); + dbgprintf("%s:%d: initrd: %s\n", __func__, __LINE__, + arch_options.initrd_file); + dbgprintf("%s:%d: dtb: %s\n", __func__, __LINE__, + arch_options.dtb_file); + return 0; } -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 0/2] kdump: simplify code
On 12/13/2021 10:43 PM, Matthew Wilcox wrote: On Mon, Dec 13, 2021 at 08:30:33AM +, David Laight wrote: From: Matthew Wilcox Sent: 12 December 2021 11:48 On Sat, Dec 11, 2021 at 05:53:46PM +, David Laight wrote: From: Tiezhu Yang Sent: 11 December 2021 03:33 v2: -- add copy_to_user_or_kernel() in lib/usercopy.c -- define userbuf as bool type Instead of having a flag to indicate whether the buffer is user or kernel, would it be better to have two separate buffer pointers. One for a user space buffer, the other for a kernel space buffer. Exactly one of the buffers should always be NULL. No. You should be using an iov_iter instead. See https://lore.kernel.org/all/ya4bdb0ubjczh...@casper.infradead.org/ for a start on this. iov_iter gets horribly expensive... Oh, right. Reading the kcore is a high-performance path, my mistake. Hi, Thank you for your discussions. The intention of this patchset is to simplify the related code with no functional changes and no side effects. At this moment, if you are OK, I will send v3 used with inline function copy_to_user_or_kernel() to keep it simple, maybe other more changes can be done in the future if no any side effect. The v3 will contain the following three patches to make the changes more clear: kdump: vmcore: remove copy_to() and add copy_to_user_or_kernel() kdump: crashdump: use copy_to_user_or_kernel() to simplify code kdump: vmcore: crashdump: make variable type of userbuf as bool ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 1/2] kdump: vmcore: remove copy_to() and add copy_to_user_or_kernel()
In arch/*/kernel/crash_dump*.c, there exist many similar code about copy_oldmem_page(), remove copy_to() in fs/proc/vmcore.c and add copy_to_user_or_kernel() in lib/usercopy.c, then we can use copy_to_user_or_kernel() to simplify the related code. Signed-off-by: Tiezhu Yang --- fs/proc/vmcore.c| 28 +++- include/linux/uaccess.h | 1 + lib/usercopy.c | 15 +++ 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c index 509f851..f67fd77 100644 --- a/fs/proc/vmcore.c +++ b/fs/proc/vmcore.c @@ -238,22 +238,8 @@ copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize, return copy_oldmem_page(pfn, buf, csize, offset, userbuf); } -/* - * Copy to either kernel or user space - */ -static int copy_to(void *target, void *src, size_t size, int userbuf) -{ - if (userbuf) { - if (copy_to_user((char __user *) target, src, size)) - return -EFAULT; - } else { - memcpy(target, src, size); - } - return 0; -} - #ifdef CONFIG_PROC_VMCORE_DEVICE_DUMP -static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int userbuf) +static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, bool userbuf) { struct vmcoredd_node *dump; u64 offset = 0; @@ -266,7 +252,7 @@ static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int userbuf) if (start < offset + dump->size) { tsz = min(offset + (u64)dump->size - start, (u64)size); buf = dump->buf + start - offset; - if (copy_to(dst, buf, tsz, userbuf)) { + if (copy_to_user_or_kernel(dst, buf, tsz, userbuf)) { ret = -EFAULT; goto out_unlock; } @@ -330,7 +316,7 @@ static int vmcoredd_mmap_dumps(struct vm_area_struct *vma, unsigned long dst, * returned otherwise number of bytes read are returned. */ static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos, -int userbuf) +bool userbuf) { ssize_t acc = 0, tmp; size_t tsz; @@ -347,7 +333,7 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos, /* Read ELF core header */ if (*fpos < elfcorebuf_sz) { tsz = min(elfcorebuf_sz - (size_t)*fpos, buflen); - if (copy_to(buffer, elfcorebuf + *fpos, tsz, userbuf)) + if (copy_to_user_or_kernel(buffer, elfcorebuf + *fpos, tsz, userbuf)) return -EFAULT; buflen -= tsz; *fpos += tsz; @@ -395,7 +381,7 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos, /* Read remaining elf notes */ tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, buflen); kaddr = elfnotes_buf + *fpos - elfcorebuf_sz - vmcoredd_orig_sz; - if (copy_to(buffer, kaddr, tsz, userbuf)) + if (copy_to_user_or_kernel(buffer, kaddr, tsz, userbuf)) return -EFAULT; buflen -= tsz; @@ -435,7 +421,7 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos, static ssize_t read_vmcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) { - return __read_vmcore((__force char *) buffer, buflen, fpos, 1); + return __read_vmcore((__force char *) buffer, buflen, fpos, true); } /* @@ -461,7 +447,7 @@ static vm_fault_t mmap_vmcore_fault(struct vm_fault *vmf) if (!PageUptodate(page)) { offset = (loff_t) index << PAGE_SHIFT; buf = __va((page_to_pfn(page) << PAGE_SHIFT)); - rc = __read_vmcore(buf, PAGE_SIZE, , 0); + rc = __read_vmcore(buf, PAGE_SIZE, , false); if (rc < 0) { unlock_page(page); put_page(page); diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index ac03940..a25e682e 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -283,6 +283,7 @@ __copy_from_user_inatomic_nocache(void *to, const void __user *from, #endif /* ARCH_HAS_NOCACHE_UACCESS */ extern __must_check int check_zeroed_user(const void __user *from, size_t size); +extern __must_check int copy_to_user_or_kernel(void *target, void *src, size_t size, bool userbuf); /** * copy_struct_from_user: copy a struct from userspace diff --git a/lib/usercopy.c b/lib/usercopy.c index 7413dd3..7431b1b 100644 --- a/lib/usercopy.c +++ b/lib/usercopy.c @@ -90,3 +90,18 @@ int check_zeroed_user(const void __user *from, size_t size) return -EFAULT; } EXPORT_SYMBOL(check_zeroed_user); + +/* + * Copy to either
[PATCH v2 2/2] kdump: crashdump: use copy_to_user_or_kernel() to simplify code
Use copy_to_user_or_kernel() to simplify the related code about copy_oldmem_page() in arch/*/kernel/crash_dump*.c files. Signed-off-by: Tiezhu Yang --- arch/arm/kernel/crash_dump.c | 12 +++- arch/arm64/kernel/crash_dump.c | 12 +++- arch/ia64/kernel/crash_dump.c| 12 +--- arch/mips/kernel/crash_dump.c| 11 +++ arch/powerpc/kernel/crash_dump.c | 11 --- arch/riscv/kernel/crash_dump.c | 11 +++ arch/sh/kernel/crash_dump.c | 11 +++ arch/x86/kernel/crash_dump_32.c | 11 +++ arch/x86/kernel/crash_dump_64.c | 15 +-- fs/proc/vmcore.c | 4 ++-- include/linux/crash_dump.h | 8 11 files changed, 38 insertions(+), 80 deletions(-) diff --git a/arch/arm/kernel/crash_dump.c b/arch/arm/kernel/crash_dump.c index 53cb924..a27c5df 100644 --- a/arch/arm/kernel/crash_dump.c +++ b/arch/arm/kernel/crash_dump.c @@ -29,7 +29,7 @@ */ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, unsigned long offset, -int userbuf) +bool userbuf) { void *vaddr; @@ -40,14 +40,8 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, if (!vaddr) return -ENOMEM; - if (userbuf) { - if (copy_to_user(buf, vaddr + offset, csize)) { - iounmap(vaddr); - return -EFAULT; - } - } else { - memcpy(buf, vaddr + offset, csize); - } + if (copy_to_user_or_kernel(buf, vaddr + offset, csize, userbuf)) + csize = -EFAULT; iounmap(vaddr); return csize; diff --git a/arch/arm64/kernel/crash_dump.c b/arch/arm64/kernel/crash_dump.c index 58303a9..d22988f 100644 --- a/arch/arm64/kernel/crash_dump.c +++ b/arch/arm64/kernel/crash_dump.c @@ -27,7 +27,7 @@ */ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, unsigned long offset, -int userbuf) +bool userbuf) { void *vaddr; @@ -38,14 +38,8 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, if (!vaddr) return -ENOMEM; - if (userbuf) { - if (copy_to_user((char __user *)buf, vaddr + offset, csize)) { - memunmap(vaddr); - return -EFAULT; - } - } else { - memcpy(buf, vaddr + offset, csize); - } + if (copy_to_user_or_kernel(buf, vaddr + offset, csize, userbuf)) + csize = -EFAULT; memunmap(vaddr); diff --git a/arch/ia64/kernel/crash_dump.c b/arch/ia64/kernel/crash_dump.c index 0ed3c3d..12128f8 100644 --- a/arch/ia64/kernel/crash_dump.c +++ b/arch/ia64/kernel/crash_dump.c @@ -33,19 +33,17 @@ */ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, - size_t csize, unsigned long offset, int userbuf) + size_t csize, unsigned long offset, bool userbuf) { void *vaddr; if (!csize) return 0; + vaddr = __va(pfn< static ssize_t __copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, - unsigned long offset, int userbuf, + unsigned long offset, bool userbuf, bool encrypted) { void *vaddr; @@ -29,13 +29,8 @@ static ssize_t __copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, if (!vaddr) return -ENOMEM; - if (userbuf) { - if (copy_to_user((void __user *)buf, vaddr + offset, csize)) { - iounmap((void __iomem *)vaddr); - return -EFAULT; - } - } else - memcpy(buf, vaddr + offset, csize); + if (copy_to_user_or_kernel(buf, vaddr + offset, csize, userbuf)) + csize = -EFAULT; set_iounmap_nonlazy(); iounmap((void __iomem *)vaddr); @@ -56,7 +51,7 @@ static ssize_t __copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, * mapped in the current kernel. We stitch up a pte, similar to kmap_atomic. */ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, -unsigned long offset, int userbuf) +unsigned long offset, bool userbuf) { return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, false); } @@ -67,7 +62,7 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, * machines. */ ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize, - unsigned long offset, int userbuf) + unsigned long offset, bool userbuf) { return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, true); } diff --git a/fs/proc/vmcore.c b/fs/p
[PATCH v2 0/2] kdump: simplify code
v2: -- add copy_to_user_or_kernel() in lib/usercopy.c -- define userbuf as bool type Tiezhu Yang (2): kdump: vmcore: remove copy_to() and add copy_to_user_or_kernel() kdump: crashdump: use copy_to_user_or_kernel() to simplify code arch/arm/kernel/crash_dump.c | 12 +++- arch/arm64/kernel/crash_dump.c | 12 +++- arch/ia64/kernel/crash_dump.c| 12 +--- arch/mips/kernel/crash_dump.c| 11 +++ arch/powerpc/kernel/crash_dump.c | 11 --- arch/riscv/kernel/crash_dump.c | 11 +++ arch/sh/kernel/crash_dump.c | 11 +++ arch/x86/kernel/crash_dump_32.c | 11 +++ arch/x86/kernel/crash_dump_64.c | 15 +-- fs/proc/vmcore.c | 32 +--- include/linux/crash_dump.h | 8 include/linux/uaccess.h | 1 + lib/usercopy.c | 15 +++ 13 files changed, 61 insertions(+), 101 deletions(-) -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/2] kdump: vmcore: move copy_to() from vmcore.c to uaccess.h
On 12/11/2021 12:59 AM, Andrew Morton wrote: On Fri, 10 Dec 2021 21:36:00 +0800 Tiezhu Yang wrote: In arch/*/kernel/crash_dump*.c, there exist similar code about copy_oldmem_page(), move copy_to() from vmcore.c to uaccess.h, and then we can use copy_to() to simplify the related code. ... --- a/fs/proc/vmcore.c +++ b/fs/proc/vmcore.c @@ -238,20 +238,6 @@ copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize, return copy_oldmem_page(pfn, buf, csize, offset, userbuf); } -/* - * Copy to either kernel or user space - */ -static int copy_to(void *target, void *src, size_t size, int userbuf) -{ - if (userbuf) { - if (copy_to_user((char __user *) target, src, size)) - return -EFAULT; - } else { - memcpy(target, src, size); - } - return 0; -} - #ifdef CONFIG_PROC_VMCORE_DEVICE_DUMP static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int userbuf) { diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index ac03940..4a6c3e4 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -201,6 +201,20 @@ copy_to_user(void __user *to, const void *from, unsigned long n) return n; } +/* + * Copy to either kernel or user space + */ +static inline int copy_to(void *target, void *src, size_t size, int userbuf) +{ + if (userbuf) { + if (copy_to_user((char __user *) target, src, size)) + return -EFAULT; + } else { + memcpy(target, src, size); + } + return 0; +} + Ordinarily I'd say "this is too large to be inlined". But the function has only a single callsite per architecture so inlining it won't cause bloat at present. But hopefully copy_to() will get additional callers in the future, in which case it shouldn't be inlined. So I'm thinking it would be best to start out with this as a regular non-inlined function, in lib/usercopy.c. Also, copy_to() is a very poor name for a globally-visible helper function. Better would be copy_to_user_or_kernel(), although that's perhaps a bit long. And the `userbuf' arg should have type bool, yes? Hi Andrew, Thank you very much for your reply and suggestion, I agree with you, I will send v2 later. Thanks, Tiezhu ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 2/2] kdump: crashdump: use copy_to() to simplify the related code
Use copy_to() to simplify the related code about copy_oldmem_page() in arch/*/kernel/crash_dump*.c files. Signed-off-by: Tiezhu Yang --- arch/arm/kernel/crash_dump.c | 10 ++ arch/arm64/kernel/crash_dump.c | 10 ++ arch/ia64/kernel/crash_dump.c| 10 -- arch/mips/kernel/crash_dump.c| 9 ++--- arch/powerpc/kernel/crash_dump.c | 7 ++- arch/riscv/kernel/crash_dump.c | 9 ++--- arch/sh/kernel/crash_dump.c | 9 ++--- arch/x86/kernel/crash_dump_32.c | 9 ++--- arch/x86/kernel/crash_dump_64.c | 9 ++--- 9 files changed, 20 insertions(+), 62 deletions(-) diff --git a/arch/arm/kernel/crash_dump.c b/arch/arm/kernel/crash_dump.c index 53cb924..6491f1d 100644 --- a/arch/arm/kernel/crash_dump.c +++ b/arch/arm/kernel/crash_dump.c @@ -40,14 +40,8 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, if (!vaddr) return -ENOMEM; - if (userbuf) { - if (copy_to_user(buf, vaddr + offset, csize)) { - iounmap(vaddr); - return -EFAULT; - } - } else { - memcpy(buf, vaddr + offset, csize); - } + if (copy_to(buf, vaddr + offset, csize, userbuf)) + csize = -EFAULT; iounmap(vaddr); return csize; diff --git a/arch/arm64/kernel/crash_dump.c b/arch/arm64/kernel/crash_dump.c index 58303a9..496e6a5 100644 --- a/arch/arm64/kernel/crash_dump.c +++ b/arch/arm64/kernel/crash_dump.c @@ -38,14 +38,8 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, if (!vaddr) return -ENOMEM; - if (userbuf) { - if (copy_to_user((char __user *)buf, vaddr + offset, csize)) { - memunmap(vaddr); - return -EFAULT; - } - } else { - memcpy(buf, vaddr + offset, csize); - } + if (copy_to(buf, vaddr + offset, csize, userbuf)) + csize = -EFAULT; memunmap(vaddr); diff --git a/arch/ia64/kernel/crash_dump.c b/arch/ia64/kernel/crash_dump.c index 0ed3c3d..20f4c4e 100644 --- a/arch/ia64/kernel/crash_dump.c +++ b/arch/ia64/kernel/crash_dump.c @@ -39,13 +39,11 @@ copy_oldmem_page(unsigned long pfn, char *buf, if (!csize) return 0; + vaddr = __va(pfn<http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 0/2] kdump: simplify code
Tiezhu Yang (2): kdump: vmcore: move copy_to() from vmcore.c to uaccess.h kdump: crashdump: use copy_to() to simplify the related code arch/arm/kernel/crash_dump.c | 10 ++ arch/arm64/kernel/crash_dump.c | 10 ++ arch/ia64/kernel/crash_dump.c| 10 -- arch/mips/kernel/crash_dump.c| 9 ++--- arch/powerpc/kernel/crash_dump.c | 7 ++- arch/riscv/kernel/crash_dump.c | 9 ++--- arch/sh/kernel/crash_dump.c | 9 ++--- arch/x86/kernel/crash_dump_32.c | 9 ++--- arch/x86/kernel/crash_dump_64.c | 9 ++--- fs/proc/vmcore.c | 14 -- include/linux/uaccess.h | 14 ++ 11 files changed, 34 insertions(+), 76 deletions(-) -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 1/2] kdump: vmcore: move copy_to() from vmcore.c to uaccess.h
In arch/*/kernel/crash_dump*.c, there exist similar code about copy_oldmem_page(), move copy_to() from vmcore.c to uaccess.h, and then we can use copy_to() to simplify the related code. Signed-off-by: Tiezhu Yang --- fs/proc/vmcore.c| 14 -- include/linux/uaccess.h | 14 ++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c index 509f851..c5976a8 100644 --- a/fs/proc/vmcore.c +++ b/fs/proc/vmcore.c @@ -238,20 +238,6 @@ copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize, return copy_oldmem_page(pfn, buf, csize, offset, userbuf); } -/* - * Copy to either kernel or user space - */ -static int copy_to(void *target, void *src, size_t size, int userbuf) -{ - if (userbuf) { - if (copy_to_user((char __user *) target, src, size)) - return -EFAULT; - } else { - memcpy(target, src, size); - } - return 0; -} - #ifdef CONFIG_PROC_VMCORE_DEVICE_DUMP static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int userbuf) { diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index ac03940..4a6c3e4 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -201,6 +201,20 @@ copy_to_user(void __user *to, const void *from, unsigned long n) return n; } +/* + * Copy to either kernel or user space + */ +static inline int copy_to(void *target, void *src, size_t size, int userbuf) +{ + if (userbuf) { + if (copy_to_user((char __user *) target, src, size)) + return -EFAULT; + } else { + memcpy(target, src, size); + } + return 0; +} + #ifndef copy_mc_to_kernel /* * Without arch opt-in this generic copy_mc_to_kernel() will not handle -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 RESEND] kexec: fix warnig of crash_zero_bytes in crash.c
Fix the following sparse warning: arch/x86/kernel/crash.c:59:15: warning: symbol 'crash_zero_bytes' was not declared. Should it be static? First, make crash_zero_bytes static. In addition, crash_zero_bytes is used when CONFIG_KEXEC_FILE is set, so make it only available under CONFIG_KEXEC_FILE. Otherwise, if CONFIG_KEXEC_FILE is not set, the following warning will appear when make crash_zero_bytes static: arch/x86/kernel/crash.c:59:22: warning: ‘crash_zero_bytes’ defined but not used [-Wunused-variable] Fixes: dd5f726076cc ("kexec: support for kexec on panic using new system call") Signed-off-by: Tiezhu Yang Acked-by: Dave Young Cc: Vivek Goyal Cc: kexec@lists.infradead.org --- arch/x86/kernel/crash.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c index 576b2e1..f13480e 100644 --- a/arch/x86/kernel/crash.c +++ b/arch/x86/kernel/crash.c @@ -56,7 +56,9 @@ struct crash_memmap_data { */ crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss = NULL; EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss); -unsigned long crash_zero_bytes; +#ifdef CONFIG_KEXEC_FILE +static unsigned long crash_zero_bytes; +#endif static inline void cpu_crash_vmclear_loaded_vmcss(void) { -- 1.8.3.1