On Sat, Aug 22, 2015 at 02:35:24AM +0000, ???? / KAWAI?HIDEHIRO wrote: > > From: Peter Zijlstra [mailto:peterz at infradead.org] > > > > On Thu, Aug 06, 2015 at 02:45:43PM +0900, Hidehiro Kawai wrote: > > > void crash_kexec(struct pt_regs *regs) > > > { > > > + int old_cpu, this_cpu; > > > + > > > + /* > > > + * `old_cpu == -1' means we are the first comer and crash_kexec() > > > + * was called without entering panic(). > > > + * `old_cpu == this_cpu' means crash_kexec() was called from panic(). > > > + */ > > > + this_cpu = raw_smp_processor_id(); > > > + old_cpu = atomic_cmpxchg(&panic_cpu, -1, this_cpu); > > > + if (old_cpu != -1 && old_cpu != this_cpu) > > > + return; > > > > This allows recursive calling of crash_kexec(), the Changelog did not > > mention that. Is this really required? > > What part are you arguing? Recursive call of crash_kexec() doesn't > happen. In the first place, one of the purpose of this patch is > to prevent a recursive call of crash_kexec() in the following case > as I stated in the description: > > CPU 0: > oops_end() > crash_kexec() > mutex_trylock() // acquired > <NMI> > io_check_error() > panic() > crash_kexec() > mutex_trylock() // failed to acquire > infinite loop > Yes, but what to we want to do there? It seems to me that is wrong, we do not want to let a recursive crash_kexec() proceed. Whereas the condition you created explicitly allows this recursion by virtue of the 'old_cpu != this_cpu' check. You changelog does not explain why you want a recursive crash_kexec(). > Also, the logic doesn't change form V1 (although the implementation > changed), so I didn't add changelogs any more. I cannot remember V1, nor can any prior patch be relevant.
