Re: [PATCH 1/3] mutex,spinlock: rename arch_mutex_cpu_relax() to cpu_relax_simple()

[Linus Torvalds]

On Fri, Sep 27, 2013 at 1:03 AM, Heiko Carstens
<heiko.carstens@xxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/