From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-pf0-x234.google.com ([2607:f8b0:400e:c00::234]) by merlin.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bKxKl-0002SI-AT for kexec@lists.infradead.org; Thu, 07 Jul 2016 00:38:28 +0000 Received: by mail-pf0-x234.google.com with SMTP id h14so904812pfe.1 for ; Wed, 06 Jul 2016 17:38:06 -0700 (PDT) Date: Thu, 7 Jul 2016 09:43:31 +0900 From: AKASHI Takahiro Subject: Re: [PATCH v21 3/8] arm64: kdump: implement machine_crash_shutdown() Message-ID: <20160707004330.GU20774@linaro.org> References: <20160706075226.27609-1-takahiro.akashi@linaro.org> <20160706075226.27609-4-takahiro.akashi@linaro.org> <1467826137.8880.35.camel@infradead.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1467826137.8880.35.camel@infradead.org> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Geoff Levand Cc: catalin.marinas@arm.com, will.deacon@arm.com, james.morse@arm.com, linux-arm-kernel@lists.infradead.org, kexec@lists.infradead.org On Wed, Jul 06, 2016 at 10:28:57AM -0700, Geoff Levand wrote: > On Wed, 2016-07-06 at 16:52 +0900, AKASHI Takahiro wrote: > > --- a/arch/arm64/include/asm/smp.h > > +++ b/arch/arm64/include/asm/smp.h > > @@ -136,6 +136,10 @@ static inline void cpu_panic_kernel(void) > > */ > > bool cpus_are_stuck_in_kernel(void); > > > > +#ifdef CONFIG_KEXEC_CORE > > +extern void smp_send_crash_stop(void); > > +#endif > > It seems this could be simplified by removing these > conditionals, then move the conditionals that are > around the smp_send_crash_stop definition to inside > that definition. OK. > > diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c > > index 62ff3c0..51e0b73 100644 > > --- a/arch/arm64/kernel/smp.c > > +++ b/arch/arm64/kernel/smp.c > > @@ -800,6 +803,28 @@ static void ipi_cpu_stop(unsigned int cpu) > > > > > cpu_relax(); > > } > > > > +#ifdef CONFIG_KEXEC_CORE > > +static atomic_t waiting_for_crash_ipi; > > + > > +static void ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs) > > +{ > > +> > crash_save_cpu(regs, cpu); > > + > > +> > atomic_dec(&waiting_for_crash_ipi); > > + > > +> > local_irq_disable(); > > + > > +#ifdef CONFIG_HOTPLUG_CPU > > +> > if (cpu_ops[cpu]->cpu_die) > > +> > > cpu_ops[cpu]->cpu_die(cpu); > > +#endif > > + > > +> > /* just in case */ > > +> > while (1) > > +> > > wfi(); > > +} > > +#endif > > /* > > * Main handler for inter-processor interrupts > > */ > > @@ -830,6 +855,14 @@ void handle_IPI(int ipinr, struct pt_regs *regs) > > > > > irq_exit(); > > > > > break; > > > > +#ifdef CONFIG_KEXEC_CORE > > +> > case IPI_CPU_CRASH_STOP: > > +> > > irq_enter(); > > +> > > ipi_cpu_crash_stop(cpu, regs); > > + > > +> > > unreachable(); > > +#endif > > It seems like you could simplifiy with something like: > > case IPI_CPU_CRASH_STOP: > if (IS_ENABLED(CONFIG_KEXEC_CORE) { > irq_enter(); > ipi_cpu_crash_stop(cpu, regs); > unreachable(); > } OK, I will take this. > Then move the CONFIG_KEXEC_CORE conditional around > ipi_cpu_crash_stop to just around crash_save_cpu(). Yeah, we can do that, but I added these conditionals just because the kernel size can be reduced (a bit, obviously) if kexec is not needed. So instead of removing the conditional around ipi_cpu_crash_stop(), an empty definition will be added for !CONFIG_KEXEC_CORE. Thanks, -Takahiro AKASHI > -Geoff _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec