From: Eric DeVolder <eric.devolder@oracle.com> To: Baoquan He <bhe@redhat.com> Cc: linux-kernel@vger.kernel.org, x86@kernel.org, kexec@lists.infradead.org, ebiederm@xmission.com, dyoung@redhat.com, vgoyal@redhat.com, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, nramas@linux.microsoft.com, thomas.lendacky@amd.com, robh@kernel.org, efault@gmx.de, rppt@kernel.org, david@redhat.com, sourabhjain@linux.ibm.com, konrad.wilk@oracle.com, boris.ostrovsky@oracle.com Subject: Re: [PATCH v16 7/7] x86/crash: add x86 crash hotplug support Date: Mon, 9 Jan 2023 13:43:39 -0600 [thread overview] Message-ID: <bc4ade74-dcbc-88f6-2d88-2bd4c2be38ba@oracle.com> (raw) In-Reply-To: <Y7vD6bm9xy9uyvRD@fedora> On 1/9/23 01:36, Baoquan He wrote: > On 01/05/23 at 10:17am, Eric DeVolder wrote: > ...... >> @@ -394,10 +409,37 @@ int crash_load_segments(struct kimage *image) >> if (ret) >> return ret; >> >> - image->elf_headers = kbuf.buffer; >> - image->elf_headers_sz = kbuf.bufsz; >> + image->elf_headers = kbuf.buffer; >> + image->elf_headers_sz = kbuf.bufsz; >> + kbuf.memsz = kbuf.bufsz; >> + >> + if (IS_ENABLED(CONFIG_CRASH_HOTPLUG)) { >> + /* >> + * Ensure the elfcorehdr segment large enough for hotplug changes. >> + * Start with VMCOREINFO and kernel_map. >> + */ >> + unsigned long pnum = 2; >> + >> + if (IS_ENABLED(CONFIG_HOTPLUG_CPU)) >> + pnum += CONFIG_NR_CPUS_DEFAULT; >> + >> + if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG)) >> + pnum += CRASH_MAX_MEMORY_RANGES; > > Logic of pnum calculating is a little confusing to me. If I only enable > one of CONFIG_HOTPLUG_CPU and CONFIG_MEMORY_HOTPLUG, is it OK? Say I > only enable CONFIG_HOTPLUG_CPU and CONFIG_SMP, on x86_64, > CONFIG_NR_CPUS_DEFAULT will be 64. pnum will be 64, is it OK. Am I miss > anything? Ah, your understanding is correct, and the issue you point out I need to fix. Specifically is only one of HOTPLUG_CPU or MEMORY_HOTPLUG is set, then I need to allow a default for the other. Using the example you have above, there would not be room for a single memory region, so I need to allow for a default number of memory regions. Good find! eric > >> + >> + if (pnum < (unsigned long)PN_XNUM) { >> + kbuf.memsz = pnum * sizeof(Elf64_Phdr); >> + kbuf.memsz += sizeof(Elf64_Ehdr); >> + >> + image->elfcorehdr_index = image->nr_segments; >> + image->elfcorehdr_index_valid = true; >> + >> + /* Mark as usable to crash kernel, else crash kernel fails on boot */ >> + image->elf_headers_sz = kbuf.memsz; >> + } else { >> + pr_err("number of Phdrs %lu exceeds max\n", pnum); >> + } >> + } >> >> - kbuf.memsz = kbuf.bufsz; >> kbuf.buf_align = ELF_CORE_HEADER_ALIGN; >> kbuf.mem = KEXEC_BUF_MEM_UNKNOWN; >> ret = kexec_add_buffer(&kbuf); >> @@ -412,3 +454,67 @@ int crash_load_segments(struct kimage *image) >> return ret; >> } >> #endif /* CONFIG_KEXEC_FILE */ > _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: Eric DeVolder <eric.devolder@oracle.com> To: Baoquan He <bhe@redhat.com> Cc: linux-kernel@vger.kernel.org, x86@kernel.org, kexec@lists.infradead.org, ebiederm@xmission.com, dyoung@redhat.com, vgoyal@redhat.com, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, nramas@linux.microsoft.com, thomas.lendacky@amd.com, robh@kernel.org, efault@gmx.de, rppt@kernel.org, david@redhat.com, sourabhjain@linux.ibm.com, konrad.wilk@oracle.com, boris.ostrovsky@oracle.com Subject: Re: [PATCH v16 7/7] x86/crash: add x86 crash hotplug support Date: Mon, 9 Jan 2023 13:43:39 -0600 [thread overview] Message-ID: <bc4ade74-dcbc-88f6-2d88-2bd4c2be38ba@oracle.com> (raw) In-Reply-To: <Y7vD6bm9xy9uyvRD@fedora> On 1/9/23 01:36, Baoquan He wrote: > On 01/05/23 at 10:17am, Eric DeVolder wrote: > ...... >> @@ -394,10 +409,37 @@ int crash_load_segments(struct kimage *image) >> if (ret) >> return ret; >> >> - image->elf_headers = kbuf.buffer; >> - image->elf_headers_sz = kbuf.bufsz; >> + image->elf_headers = kbuf.buffer; >> + image->elf_headers_sz = kbuf.bufsz; >> + kbuf.memsz = kbuf.bufsz; >> + >> + if (IS_ENABLED(CONFIG_CRASH_HOTPLUG)) { >> + /* >> + * Ensure the elfcorehdr segment large enough for hotplug changes. >> + * Start with VMCOREINFO and kernel_map. >> + */ >> + unsigned long pnum = 2; >> + >> + if (IS_ENABLED(CONFIG_HOTPLUG_CPU)) >> + pnum += CONFIG_NR_CPUS_DEFAULT; >> + >> + if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG)) >> + pnum += CRASH_MAX_MEMORY_RANGES; > > Logic of pnum calculating is a little confusing to me. If I only enable > one of CONFIG_HOTPLUG_CPU and CONFIG_MEMORY_HOTPLUG, is it OK? Say I > only enable CONFIG_HOTPLUG_CPU and CONFIG_SMP, on x86_64, > CONFIG_NR_CPUS_DEFAULT will be 64. pnum will be 64, is it OK. Am I miss > anything? Ah, your understanding is correct, and the issue you point out I need to fix. Specifically is only one of HOTPLUG_CPU or MEMORY_HOTPLUG is set, then I need to allow a default for the other. Using the example you have above, there would not be room for a single memory region, so I need to allow for a default number of memory regions. Good find! eric > >> + >> + if (pnum < (unsigned long)PN_XNUM) { >> + kbuf.memsz = pnum * sizeof(Elf64_Phdr); >> + kbuf.memsz += sizeof(Elf64_Ehdr); >> + >> + image->elfcorehdr_index = image->nr_segments; >> + image->elfcorehdr_index_valid = true; >> + >> + /* Mark as usable to crash kernel, else crash kernel fails on boot */ >> + image->elf_headers_sz = kbuf.memsz; >> + } else { >> + pr_err("number of Phdrs %lu exceeds max\n", pnum); >> + } >> + } >> >> - kbuf.memsz = kbuf.bufsz; >> kbuf.buf_align = ELF_CORE_HEADER_ALIGN; >> kbuf.mem = KEXEC_BUF_MEM_UNKNOWN; >> ret = kexec_add_buffer(&kbuf); >> @@ -412,3 +454,67 @@ int crash_load_segments(struct kimage *image) >> return ret; >> } >> #endif /* CONFIG_KEXEC_FILE */ >
next prev parent reply other threads:[~2023-01-09 19:44 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-05 15:17 [PATCH v16 0/7] crash: Kernel handling of CPU and memory hot un/plug Eric DeVolder 2023-01-05 15:17 ` Eric DeVolder 2023-01-05 15:17 ` [PATCH v16 1/7] crash: move crash_prepare_elf64_headers() Eric DeVolder 2023-01-05 15:17 ` Eric DeVolder 2023-01-05 15:17 ` [PATCH v16 2/7] crash: prototype change for crash_prepare_elf64_headers() Eric DeVolder 2023-01-05 15:17 ` Eric DeVolder 2023-01-05 15:17 ` [PATCH v16 3/7] crash: add generic infrastructure for crash hotplug support Eric DeVolder 2023-01-05 15:17 ` Eric DeVolder 2023-01-05 15:17 ` [PATCH v16 4/7] kexec: exclude elfcorehdr from the segment digest Eric DeVolder 2023-01-05 15:17 ` Eric DeVolder 2023-01-05 15:17 ` [PATCH v16 5/7] kexec: exclude hot remove cpu from elfcorehdr notes Eric DeVolder 2023-01-05 15:17 ` Eric DeVolder 2023-01-05 15:17 ` [PATCH v16 6/7] crash: memory and cpu hotplug sysfs attributes Eric DeVolder 2023-01-05 15:17 ` Eric DeVolder 2023-01-05 15:17 ` [PATCH v16 7/7] x86/crash: add x86 crash hotplug support Eric DeVolder 2023-01-05 15:17 ` Eric DeVolder 2023-01-09 7:36 ` Baoquan He 2023-01-09 7:36 ` Baoquan He 2023-01-09 19:43 ` Eric DeVolder [this message] 2023-01-09 19:43 ` Eric DeVolder 2023-01-09 23:47 ` Baoquan He 2023-01-09 23:47 ` Baoquan He
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=bc4ade74-dcbc-88f6-2d88-2bd4c2be38ba@oracle.com \ --to=eric.devolder@oracle.com \ --cc=bhe@redhat.com \ --cc=boris.ostrovsky@oracle.com \ --cc=bp@alien8.de \ --cc=dave.hansen@linux.intel.com \ --cc=david@redhat.com \ --cc=dyoung@redhat.com \ --cc=ebiederm@xmission.com \ --cc=efault@gmx.de \ --cc=hpa@zytor.com \ --cc=kexec@lists.infradead.org \ --cc=konrad.wilk@oracle.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mingo@redhat.com \ --cc=nramas@linux.microsoft.com \ --cc=robh@kernel.org \ --cc=rppt@kernel.org \ --cc=sourabhjain@linux.ibm.com \ --cc=tglx@linutronix.de \ --cc=thomas.lendacky@amd.com \ --cc=vgoyal@redhat.com \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.