Messages in this thread | | | Date | Fri, 27 Sep 2013 09:13:10 -0700 | Subject | Re: [PATCH 1/3] mutex,spinlock: rename arch_mutex_cpu_relax() to cpu_relax_simple() | From | Linus Torvalds <> |
| |
On Fri, Sep 27, 2013 at 1:03 AM, Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > s390 needs a special version of cpu_relax() for the new lockref code. > The new variant should be a no-op but also have memory barrier semantics, > since that is what the default cpu_relax() variant implements.
I'd actually prefer you do this without the renaming, and we just use arch_mutex_cpu_relax() in the lockref code.
We have another set of patches kind-of pending that uses arch_mutex_cpu_relax() (MCS lock for rwsem), and I don't want to screw them up.
The _one_ thing I'd suggest is that you replace this:
#ifndef CONFIG_HAVE_ARCH_MUTEX_CPU_RELAX #define arch_mutex_cpu_relax() cpu_relax() #endif
with just a simple
#ifndef arch_mutex_cpu_relax # define arch_mutex_cpu_relax() cpu_relax() #endif
and get rid of that unnecessary CONFIG variable. I'm ok with CONFIG variables, but they should be for bigger things than "I have this particular function". The "I have this particular function" is particularly easily solved with just using the function name as a preprocessor symbol (either _because_ it is a macro in the first place, or with a simple "#define fn_name fn_name" to let the generic code know it exists).
So if you just move the
#define arch_mutex_cpu_relax() barrier()
into arch/s390/include/asm/processor.h (so that you always have it when you have the regular cpu_relax() one, and people don't need to include <mutex.h> to get it), and then put that #ifdef into lib/lockref.c, and we're all ok. We already did the equivalent for the ARM use of cmpxchg64_relaxed().
And we can maybe rename it later, when there isn't any new use pending...
Linus
| |