Re: Re: [V4 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly
On Mon, Sep 28, 2015 at 07:08:19AM +, 河合英宏 / KAWAI，HIDEHIRO wrote: > > >> kernel/kexec_core.c:899:3: note: in expansion of macro 'atomic_xchg' > > atomic_xchg(_cpu, -1); > > ^ > > I changed to use atomic_xchg() instead of atomic_set() in V3 > because atomic_set() doesn't mean memory barrier. However, > I thought again and there is no need of barrier; there is no > problem if a competitor sees old value of panic_cpu or new one. > So, atomic_set() is sufficient and using it will remove this warning. > > I will resend the fixed version later. So if you rely on the memory barrier; you should have also put a comment on explaining the ordering requirements. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
RE: Re: [V4 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly
> On Mon, Sep 28, 2015 at 07:08:19AM +, 河合英宏 / KAWAI，HIDEHIRO wrote: > > > >> kernel/kexec_core.c:899:3: note: in expansion of macro 'atomic_xchg' > > > atomic_xchg(_cpu, -1); > > > ^ > > > > I changed to use atomic_xchg() instead of atomic_set() in V3 > > because atomic_set() doesn't mean memory barrier. However, > > I thought again and there is no need of barrier; there is no > > problem if a competitor sees old value of panic_cpu or new one. > > So, atomic_set() is sufficient and using it will remove this warning. > > > > I will resend the fixed version later. > > So if you rely on the memory barrier; you should have also put a comment > on explaining the ordering requirements. I don't intend to use an explicit memory barrier. There is no memory ordering requirement here. Also, atomic_set() which will be used instead of atomic_xchg() is used as a RELEASE operation, so I believe there is no problem. Documentation/memory-barriers.txt: > The following operations are potential problems as they do _not_ imply memory > barriers, but might be used for implementing such things as RELEASE-class > operations: > > atomic_set(); > ... ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
RE: Re: [V4 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly
> Hi Hidehiro,
>
> [auto build test results on v4.3-rc2 -- if it's inappropriate base, please
> ignore]
>
> config: ia64-allyesconfig (attached as .config)
> reproduce:
> wget
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> git checkout 0077681103150af584e5e592c0238fd010654c26
> # save the attached .config to linux build tree
> make.cross ARCH=ia64
[snip]
>arch/ia64/include/uapi/asm/cmpxchg.h:56:2: warning: value computed is not
> used [-Wunused-value]
> ((__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr
> ^
>arch/ia64/include/asm/atomic.h:135:30: note: in expansion of macro 'xchg'
> #define atomic_xchg(v, new) (xchg(&((v)->counter), new))
> ^
> >> kernel/kexec_core.c:899:3: note: in expansion of macro 'atomic_xchg'
> atomic_xchg(_cpu, -1);
> ^
I changed to use atomic_xchg() instead of atomic_set() in V3
because atomic_set() doesn't mean memory barrier. However,
I thought again and there is no need of barrier; there is no
problem if a competitor sees old value of panic_cpu or new one.
So, atomic_set() is sufficient and using it will remove this warning.
I will resend the fixed version later.
> vim +/atomic_xchg +899 kernel/kexec_core.c
>
>883
>884/*
>885 * Only one CPU is allowed to execute the crash_kexec()
> code as with
>886 * panic(). Otherwise parallel calls of panic() and
> crash_kexec()
>887 * may stop each other. To exclude them, we use
> panic_cpu here too.
>888 */
>889this_cpu = raw_smp_processor_id();
>890old_cpu = atomic_cmpxchg(_cpu, -1, this_cpu);
>891if (old_cpu == -1) {
>892/* This is the 1st CPU which comes here, so go
> ahead. */
>893__crash_kexec(regs);
>894
>895/*
>896 * Reset panic_cpu to allow another
> panic()/crash_kexec()
>897 * call.
>898 */
> > 899atomic_xchg(_cpu, -1);
>900}
>901}
>902
>903size_t crash_get_memory_size(void)
>904{
>905size_t size = 0;
>906
>907mutex_lock(_mutex);
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hidehiro Kawai
Hitachi, Ltd. Research & Development Group
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
RE: Re: [V4 PATCH 3/4] kexec: Fix race between panic() and crash_kexec() called directly
> Hi Hidehiro,
>
> [auto build test results on v4.3-rc2 -- if it's inappropriate base, please
> ignore]
>
> config: x86_64-allnoconfig (attached as .config)
> reproduce:
> git checkout 0077681103150af584e5e592c0238fd010654c26
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All error/warnings (new ones prefixed by >>):
>
>kernel/panic.c: In function 'panic':
> >> kernel/panic.c:140:3: error: implicit declaration of function
> >> '__crash_kexec' [-Werror=implicit-function-declaration]
> __crash_kexec(NULL);
> ^
Sorry, I missed to take into account the case of !CONFIG_KEXEC_CORE.
#else /* !CONFIG_KEXEC_CORE */
struct pt_regs;
struct task_struct;
+static inline void __crash_kexec(struct pt_regs *regs) { }
static inline void crash_kexec(struct pt_regs *regs) { }
static inline int kexec_should_crash(struct task_struct *p) { return 0; }
I'll resend the revised version later.
>cc1: some warnings being treated as errors
>
> vim +/__crash_kexec +140 kernel/panic.c
>
>134 * If we have crashed and we have a crash kernel loaded
> let it handle
>135 * everything else.
>136 * If we want to run this after calling
> panic_notifiers, pass
>137 * the "crash_kexec_post_notifiers" option to the
> kernel.
>138 */
>139if (!crash_kexec_post_notifiers)
> > 140__crash_kexec(NULL);
>141
>142/*
>143 * Note smp_send_stop is the usual smp shutdown
> function, which
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hidehiro Kawai
Hitachi, Ltd. Research & Development Group
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
