Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756150AbdCUDiR (ORCPT ); Mon, 20 Mar 2017 23:38:17 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:47954 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754834AbdCUDiP (ORCPT ); Mon, 20 Mar 2017 23:38:15 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Xunlei Pang Cc: linux-kernel@vger.kernel.org, kexec@lists.infradead.org, akpm@linux-foundation.org, Dave Young , Baoquan He , Petr Tesarik , Mahesh Salgaonkar References: <1489989033-1179-1-git-send-email-xlpang@redhat.com> Date: Mon, 20 Mar 2017 22:33:07 -0500 In-Reply-To: <1489989033-1179-1-git-send-email-xlpang@redhat.com> (Xunlei Pang's message of "Mon, 20 Mar 2017 13:50:31 +0800") Message-ID: <87pohbz4lo.fsf@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1cqAcZ-0002dH-Ir;;;mid=<87pohbz4lo.fsf@xmission.com>;;;hst=in01.mta.xmission.com;;;ip=67.3.234.240;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/OVXUnvmMdGhOAqv9/2GYHbT0r1Vuzhe4= X-SA-Exim-Connect-IP: 67.3.234.240 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.7 XMSubLong Long Subject * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_02 5+ unique symbols in subject * 1.0 XM_Sft_Co_L33T No description available. * 0.0 T_TooManySym_01 4+ unique symbols in subject * 1.0 T_XMDrugObfuBody_00 obfuscated drug references X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **;Xunlei Pang X-Spam-Relay-Country: X-Spam-Timing: total 5545 ms - load_scoreonly_sql: 0.04 (0.0%), signal_user_changed: 3.5 (0.1%), b_tie_ro: 2.5 (0.0%), parse: 1.15 (0.0%), extract_message_metadata: 15 (0.3%), get_uri_detail_list: 4.2 (0.1%), tests_pri_-1000: 6 (0.1%), tests_pri_-950: 1.13 (0.0%), tests_pri_-900: 0.95 (0.0%), tests_pri_-400: 30 (0.5%), check_bayes: 29 (0.5%), b_tokenize: 11 (0.2%), b_tok_get_all: 9 (0.2%), b_comp_prob: 2.6 (0.0%), b_tok_touch_all: 4.2 (0.1%), b_finish: 0.66 (0.0%), tests_pri_0: 362 (6.5%), check_dkim_signature: 0.65 (0.0%), check_dkim_adsp: 3.5 (0.1%), tests_pri_500: 5123 (92.4%), poll_dns_idle: 5116 (92.3%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH v3 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5438 Lines: 148 Xunlei Pang writes: > As Eric said, > "what we need to do is move the variable vmcoreinfo_note out > of the kernel's .bss section. And modify the code to regenerate > and keep this information in something like the control page. > > Definitely something like this needs a page all to itself, and ideally > far away from any other kernel data structures. I clearly was not > watching closely the data someone decided to keep this silly thing > in the kernel's .bss section." > > This patch allocates extra pages for these vmcoreinfo_XXX variables, > one advantage is that it enhances some safety of vmcoreinfo, because > vmcoreinfo now is kept far away from other kernel data structures. Can you preceed this patch with a patch that removes CRASHTIME from vmcoreinfo? If someone actually cares we can add a separate note that holds a 64bit crashtime in the per cpu notes. As we are looking at reliability concerns removing CRASHTIME should make everything in vmcoreinfo a boot time constant. Which should simplify everything considerably. Which means we only need to worry abou the per-cpu notes being written at the time of a crash. > Suggested-by: Eric Biederman > Signed-off-by: Xunlei Pang > --- > arch/ia64/kernel/machine_kexec.c | 5 ----- > arch/x86/kernel/crash.c | 2 +- > include/linux/kexec.h | 2 +- > kernel/kexec_core.c | 29 ++++++++++++++++++++++++----- > kernel/ksysfs.c | 2 +- > 5 files changed, 27 insertions(+), 13 deletions(-) > > diff --git a/arch/ia64/kernel/machine_kexec.c b/arch/ia64/kernel/machine_kexec.c > index 599507b..c14815d 100644 > --- a/arch/ia64/kernel/machine_kexec.c > +++ b/arch/ia64/kernel/machine_kexec.c > @@ -163,8 +163,3 @@ void arch_crash_save_vmcoreinfo(void) > #endif > } > > -phys_addr_t paddr_vmcoreinfo_note(void) > -{ > - return ia64_tpa((unsigned long)(char *)&vmcoreinfo_note); > -} > - > diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c > index 3741461..4d35fbb 100644 > --- a/arch/x86/kernel/crash.c > +++ b/arch/x86/kernel/crash.c > @@ -456,7 +456,7 @@ static int prepare_elf64_headers(struct crash_elf_data *ced, > bufp += sizeof(Elf64_Phdr); > phdr->p_type = PT_NOTE; > phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note(); > - phdr->p_filesz = phdr->p_memsz = sizeof(vmcoreinfo_note); > + phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE; > (ehdr->e_phnum)++; > > #ifdef CONFIG_X86_64 > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > index e98e546..f1c601b 100644 > --- a/include/linux/kexec.h > +++ b/include/linux/kexec.h > @@ -317,7 +317,7 @@ extern void *kexec_purgatory_get_symbol_addr(struct kimage *image, > extern struct resource crashk_low_res; > typedef u32 note_buf_t[KEXEC_NOTE_BYTES/4]; > extern note_buf_t __percpu *crash_notes; > -extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4]; > +extern u32 *vmcoreinfo_note; > extern size_t vmcoreinfo_size; > extern size_t vmcoreinfo_max_size; > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c > index bfe62d5..e3a4bda 100644 > --- a/kernel/kexec_core.c > +++ b/kernel/kexec_core.c > @@ -52,10 +52,10 @@ > note_buf_t __percpu *crash_notes; > > /* vmcoreinfo stuff */ > -static unsigned char vmcoreinfo_data[VMCOREINFO_BYTES]; > -u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4]; > +static unsigned char *vmcoreinfo_data; > size_t vmcoreinfo_size; > -size_t vmcoreinfo_max_size = sizeof(vmcoreinfo_data); > +size_t vmcoreinfo_max_size = VMCOREINFO_BYTES; > +u32 *vmcoreinfo_note; > > /* Flag to indicate we are going to kexec a new kernel */ > bool kexec_in_progress = false; > @@ -1369,6 +1369,9 @@ static void update_vmcoreinfo_note(void) > > void crash_save_vmcoreinfo(void) > { > + if (!vmcoreinfo_note) > + return; > + > vmcoreinfo_append_str("CRASHTIME=%ld\n", get_seconds()); > update_vmcoreinfo_note(); > } > @@ -1397,13 +1400,29 @@ void vmcoreinfo_append_str(const char *fmt, ...) > void __weak arch_crash_save_vmcoreinfo(void) > {} > > -phys_addr_t __weak paddr_vmcoreinfo_note(void) > +phys_addr_t paddr_vmcoreinfo_note(void) > { > - return __pa_symbol((unsigned long)(char *)&vmcoreinfo_note); > + return __pa(vmcoreinfo_note); > } > > static int __init crash_save_vmcoreinfo_init(void) > { > + /* One page should be enough for VMCOREINFO_BYTES under all archs */ > + vmcoreinfo_data = (unsigned char *)get_zeroed_page(GFP_KERNEL); > + if (!vmcoreinfo_data) { > + pr_warn("Memory allocation for vmcoreinfo_data failed\n"); > + return -ENOMEM; > + } > + > + vmcoreinfo_note = alloc_pages_exact(VMCOREINFO_NOTE_SIZE, > + GFP_KERNEL | __GFP_ZERO); > + if (!vmcoreinfo_note) { > + free_page((unsigned long)vmcoreinfo_data); > + vmcoreinfo_data = NULL; > + pr_warn("Memory allocation for vmcoreinfo_note failed\n"); > + return -ENOMEM; > + } > + > VMCOREINFO_OSRELEASE(init_uts_ns.name.release); > VMCOREINFO_PAGESIZE(PAGE_SIZE); > > diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c > index ee1bc1b..9de6fcc 100644 > --- a/kernel/ksysfs.c > +++ b/kernel/ksysfs.c > @@ -130,7 +130,7 @@ static ssize_t vmcoreinfo_show(struct kobject *kobj, > { > phys_addr_t vmcore_base = paddr_vmcoreinfo_note(); > return sprintf(buf, "%pa %x\n", &vmcore_base, > - (unsigned int)sizeof(vmcoreinfo_note)); > + (unsigned int)VMCOREINFO_NOTE_SIZE); > } > KERNEL_ATTR_RO(vmcoreinfo);