Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751864AbdFKIlp (ORCPT ); Sun, 11 Jun 2017 04:41:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38070 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751743AbdFKIln (ORCPT ); Sun, 11 Jun 2017 04:41:43 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 13BDF80C04 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=xpang@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 13BDF80C04 Reply-To: xlpang@redhat.com Subject: Re: [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition References: <1496974625-10891-1-git-send-email-xlpang@redhat.com> <20170609022904.GB3688@dhcp-128-65.nay.redhat.com> <20170609074529.GA9485@dhcp-128-65.nay.redhat.com> To: Dave Young , Xunlei Pang Cc: linux-kernel@vger.kernel.org, kexec@lists.infradead.org, akpm@linux-foundation.org, Eric Biederman , Baoquan He , Michael Holzheu , linux-s390@vger.kernel.org, Dave Anderson , Hari Bathini , Gustavo Luiz Duarte From: Xunlei Pang Message-ID: <593D02AB.1030706@redhat.com> Date: Sun, 11 Jun 2017 16:43:23 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20170609074529.GA9485@dhcp-128-65.nay.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Sun, 11 Jun 2017 08:41:43 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4806 Lines: 129 On 06/09/2017 at 03:45 PM, Dave Young wrote: > On 06/09/17 at 10:29am, Dave Young wrote: >> On 06/09/17 at 10:17am, Xunlei Pang wrote: >>> S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which >>> is now defined as follows: >>> typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4]; >>> It was changed by the CONFIG_CRASH_CORE feature. >>> >>> This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and >>> renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390. >>> >>> Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE") >>> Cc: Dave Young >>> Cc: Dave Anderson >>> Cc: Hari Bathini >>> Cc: Gustavo Luiz Duarte >>> Signed-off-by: Xunlei Pang >>> --- >>> arch/s390/include/asm/kexec.h | 2 +- >>> include/linux/crash_core.h | 7 +++++++ >>> include/linux/kexec.h | 11 +---------- >>> 3 files changed, 9 insertions(+), 11 deletions(-) >>> >>> diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h >>> index 2f924bc..352deb8 100644 >>> --- a/arch/s390/include/asm/kexec.h >>> +++ b/arch/s390/include/asm/kexec.h >>> @@ -47,7 +47,7 @@ >>> * Seven notes plus zero note at the end: prstatus, fpregset, timer, >>> * tod_cmp, tod_reg, control regs, and prefix >>> */ >>> -#define KEXEC_NOTE_BYTES \ >>> +#define CRASH_CORE_NOTE_BYTES \ >>> (ALIGN(sizeof(struct elf_note), 4) * 8 + \ >>> ALIGN(sizeof("CORE"), 4) * 7 + \ >>> ALIGN(sizeof(struct elf_prstatus), 4) + \ > I found that in mainline since below commit, above define should be > useless, but if distribution with older kernel does need your fix, so in > mainline the right fix should be dropping the s390 part about these > macros usage. Indeed, then I think we can remove this special definition of S390 to avoid confusion. Regards, Xunlei > > Anyway this need a comment from Michael. > > commit 8a07dd02d7615d91d65d6235f7232e3f9b5d347f > Author: Martin Schwidefsky > Date: Wed Oct 14 15:53:06 2015 +0200 > > s390/kdump: remove code to create ELF notes in the crashed system > > The s390 architecture can store the CPU registers of the crashed > system > after the kdump kernel has been started and this is the preferred > way. > Remove the remaining code fragments that deal with storing CPU > registers > while the crashed system is still active. > > Acked-by: Michael Holzheu > Signed-off-by: Martin Schwidefsky > > >>> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h >>> index e9de6b4..dbc6e5c 100644 >>> --- a/include/linux/crash_core.h >>> +++ b/include/linux/crash_core.h >>> @@ -10,9 +10,16 @@ >>> #define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4) >>> #define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4) >>> >>> +/* >>> + * The per-cpu notes area is a list of notes terminated by a "NULL" >>> + * note header. For kdump, the code in vmcore.c runs in the context >>> + * of the second kernel to combine them into one note. >>> + */ >>> +#ifndef CRASH_CORE_NOTE_BYTES >>> #define CRASH_CORE_NOTE_BYTES ((CRASH_CORE_NOTE_HEAD_BYTES * 2) + \ >>> CRASH_CORE_NOTE_NAME_BYTES + \ >>> CRASH_CORE_NOTE_DESC_BYTES) >>> +#endif >>> >>> #define VMCOREINFO_BYTES PAGE_SIZE >>> #define VMCOREINFO_NOTE_NAME "VMCOREINFO" >>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h >>> index 3ea8275..133df03 100644 >>> --- a/include/linux/kexec.h >>> +++ b/include/linux/kexec.h >>> @@ -14,7 +14,6 @@ >>> >>> #if !defined(__ASSEMBLY__) >>> >>> -#include >>> #include >>> >>> #include >>> @@ -25,6 +24,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> /* Verify architecture specific macros are defined */ >>> >>> @@ -63,15 +63,6 @@ >>> #define KEXEC_CORE_NOTE_NAME CRASH_CORE_NOTE_NAME >>> >>> /* >>> - * The per-cpu notes area is a list of notes terminated by a "NULL" >>> - * note header. For kdump, the code in vmcore.c runs in the context >>> - * of the second kernel to combine them into one note. >>> - */ >>> -#ifndef KEXEC_NOTE_BYTES >>> -#define KEXEC_NOTE_BYTES CRASH_CORE_NOTE_BYTES >>> -#endif >> It is still not clear how does s390 use the crash_notes except this macro. >> But from code point of view we do need to update this as well after the >> crash_core splitting. >> >> Acked-by: Dave Young > Hold on the ack because of the new findings, wait for Michael's > feedback. > > Thanks > Dave