lkml.org 
[lkml]   [2015]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [V3 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly
On Sat, Aug 22, 2015 at 02:35:24AM +0000, 河合英宏 / KAWAI,HIDEHIRO wrote:
> > From: Peter Zijlstra [mailto:peterz@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.



\
 
 \ /
  Last update: 2015-08-25 17:21    [W:0.101 / U:0.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site