diff mbox series

[3/3] powerpc: rewrite atomics to use ARCH_ATOMIC

Message ID 20201111110723.3148665-4-npiggin@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series powerpc: convert to use ARCH_ATOMIC | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (80ecbe16c827714ce3741ed1f1d34488b903e717)
snowpatch_ozlabs/build-ppc64le success Build succeeded
snowpatch_ozlabs/build-ppc64be success Build succeeded
snowpatch_ozlabs/build-ppc64e success Build succeeded
snowpatch_ozlabs/build-pmac32 warning Build succeeded but added 3 new sparse warnings
snowpatch_ozlabs/checkpatch warning total: 7 errors, 36 warnings, 28 checks, 877 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Nicholas Piggin Nov. 11, 2020, 11:07 a.m. UTC
All the cool kids are doing it.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/atomic.h  | 681 ++++++++++-------------------
 arch/powerpc/include/asm/cmpxchg.h |  62 +--
 2 files changed, 248 insertions(+), 495 deletions(-)

Comments

kernel test robot Nov. 11, 2020, 7:07 p.m. UTC | #1
Hi Nicholas,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on asm-generic/master linus/master v5.10-rc3 next-20201111]
[cannot apply to scottwood/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-convert-to-use-ARCH_ATOMIC/20201111-190941
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9e1bec8fe216b0745c647e52c40d1f0033fb4efd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nicholas-Piggin/powerpc-convert-to-use-ARCH_ATOMIC/20201111-190941
        git checkout 9e1bec8fe216b0745c647e52c40d1f0033fb4efd
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/powerpc/include/asm/atomic.h:11,
                    from include/linux/atomic.h:7,
                    from include/linux/rcupdate.h:25,
                    from include/linux/rculist.h:11,
                    from include/linux/sched/signal.h:5,
                    from drivers/gpu/drm/drm_lock.c:37:
   drivers/gpu/drm/drm_lock.c: In function 'drm_lock_take':
>> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers]
     463 |  (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr),   \
         |                                         ^~~~~
   include/linux/atomic.h:73:9: note: in expansion of macro 'arch_cmpxchg_relaxed'
      73 |  typeof(op##_relaxed(args)) __ret;    \
         |         ^~
   include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence'
      52 |  __atomic_op_fence(arch_cmpxchg, __VA_ARGS__)
         |  ^~~~~~~~~~~~~~~~~
   include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg'
    1685 |  arch_cmpxchg(__ai_ptr, __VA_ARGS__);    \
         |  ^~~~~~~~~~~~
   drivers/gpu/drm/drm_lock.c:75:10: note: in expansion of macro 'cmpxchg'
      75 |   prev = cmpxchg(lock, old, new);
         |          ^~~~~~~
   arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *'
     432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
         |                   ~~~~~~^~~
>> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers]
     463 |  (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr),   \
         |                                         ^~~~~
   include/linux/atomic.h:75:10: note: in expansion of macro 'arch_cmpxchg_relaxed'
      75 |  __ret = op##_relaxed(args);     \
         |          ^~
   include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence'
      52 |  __atomic_op_fence(arch_cmpxchg, __VA_ARGS__)
         |  ^~~~~~~~~~~~~~~~~
   include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg'
    1685 |  arch_cmpxchg(__ai_ptr, __VA_ARGS__);    \
         |  ^~~~~~~~~~~~
   drivers/gpu/drm/drm_lock.c:75:10: note: in expansion of macro 'cmpxchg'
      75 |   prev = cmpxchg(lock, old, new);
         |          ^~~~~~~
   arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *'
     432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
         |                   ~~~~~~^~~
   drivers/gpu/drm/drm_lock.c: In function 'drm_lock_transfer':
>> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers]
     463 |  (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr),   \
         |                                         ^~~~~
   include/linux/atomic.h:73:9: note: in expansion of macro 'arch_cmpxchg_relaxed'
      73 |  typeof(op##_relaxed(args)) __ret;    \
         |         ^~
   include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence'
      52 |  __atomic_op_fence(arch_cmpxchg, __VA_ARGS__)
         |  ^~~~~~~~~~~~~~~~~
   include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg'
    1685 |  arch_cmpxchg(__ai_ptr, __VA_ARGS__);    \
         |  ^~~~~~~~~~~~
   drivers/gpu/drm/drm_lock.c:118:10: note: in expansion of macro 'cmpxchg'
     118 |   prev = cmpxchg(lock, old, new);
         |          ^~~~~~~
   arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *'
     432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
         |                   ~~~~~~^~~
>> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers]
     463 |  (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr),   \
         |                                         ^~~~~
   include/linux/atomic.h:75:10: note: in expansion of macro 'arch_cmpxchg_relaxed'
      75 |  __ret = op##_relaxed(args);     \
         |          ^~
   include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence'
      52 |  __atomic_op_fence(arch_cmpxchg, __VA_ARGS__)
         |  ^~~~~~~~~~~~~~~~~
   include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg'
    1685 |  arch_cmpxchg(__ai_ptr, __VA_ARGS__);    \
         |  ^~~~~~~~~~~~
   drivers/gpu/drm/drm_lock.c:118:10: note: in expansion of macro 'cmpxchg'
     118 |   prev = cmpxchg(lock, old, new);
         |          ^~~~~~~
   arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *'
     432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
         |                   ~~~~~~^~~
   drivers/gpu/drm/drm_lock.c: In function 'drm_legacy_lock_free':
>> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers]
     463 |  (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr),   \
         |                                         ^~~~~
   include/linux/atomic.h:73:9: note: in expansion of macro 'arch_cmpxchg_relaxed'
      73 |  typeof(op##_relaxed(args)) __ret;    \
         |         ^~
   include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence'
      52 |  __atomic_op_fence(arch_cmpxchg, __VA_ARGS__)
         |  ^~~~~~~~~~~~~~~~~
   include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg'
    1685 |  arch_cmpxchg(__ai_ptr, __VA_ARGS__);    \
         |  ^~~~~~~~~~~~
   drivers/gpu/drm/drm_lock.c:141:10: note: in expansion of macro 'cmpxchg'
     141 |   prev = cmpxchg(lock, old, new);
         |          ^~~~~~~
   arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *'
     432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
         |                   ~~~~~~^~~
>> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers]
     463 |  (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr),   \
         |                                         ^~~~~
   include/linux/atomic.h:75:10: note: in expansion of macro 'arch_cmpxchg_relaxed'
      75 |  __ret = op##_relaxed(args);     \
         |          ^~
   include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence'
      52 |  __atomic_op_fence(arch_cmpxchg, __VA_ARGS__)
         |  ^~~~~~~~~~~~~~~~~
   include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg'
    1685 |  arch_cmpxchg(__ai_ptr, __VA_ARGS__);    \
         |  ^~~~~~~~~~~~
   drivers/gpu/drm/drm_lock.c:141:10: note: in expansion of macro 'cmpxchg'
     141 |   prev = cmpxchg(lock, old, new);
         |          ^~~~~~~
   arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *'
     432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
         |                   ~~~~~~^~~
   drivers/gpu/drm/drm_lock.c: In function 'drm_legacy_idlelock_release':
>> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers]
     463 |  (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr),   \
         |                                         ^~~~~
   include/linux/atomic.h:73:9: note: in expansion of macro 'arch_cmpxchg_relaxed'
      73 |  typeof(op##_relaxed(args)) __ret;    \
         |         ^~
   include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence'
      52 |  __atomic_op_fence(arch_cmpxchg, __VA_ARGS__)
         |  ^~~~~~~~~~~~~~~~~
   include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg'
    1685 |  arch_cmpxchg(__ai_ptr, __VA_ARGS__);    \
         |  ^~~~~~~~~~~~
   drivers/gpu/drm/drm_lock.c:319:12: note: in expansion of macro 'cmpxchg'
     319 |     prev = cmpxchg(lock, old, DRM_KERNEL_CONTEXT);
         |            ^~~~~~~
   arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *'
     432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
         |                   ~~~~~~^~~
>> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers]
     463 |  (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr),   \
         |                                         ^~~~~
   include/linux/atomic.h:75:10: note: in expansion of macro 'arch_cmpxchg_relaxed'
      75 |  __ret = op##_relaxed(args);     \
         |          ^~
   include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence'
      52 |  __atomic_op_fence(arch_cmpxchg, __VA_ARGS__)
         |  ^~~~~~~~~~~~~~~~~
   include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg'
    1685 |  arch_cmpxchg(__ai_ptr, __VA_ARGS__);    \
         |  ^~~~~~~~~~~~
   drivers/gpu/drm/drm_lock.c:319:12: note: in expansion of macro 'cmpxchg'
     319 |     prev = cmpxchg(lock, old, DRM_KERNEL_CONTEXT);
         |            ^~~~~~~
   arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *'
     432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
         |                   ~~~~~~^~~

vim +463 arch/powerpc/include/asm/cmpxchg.h

56c08e6d226c860 Boqun Feng      2015-12-15  450  
9e1bec8fe216b07 Nicholas Piggin 2020-11-11  451  #define arch_cmpxchg_local(ptr, o, n)					 \
ae3a197e3d0bfe3 David Howells   2012-03-28  452    ({									 \
ae3a197e3d0bfe3 David Howells   2012-03-28  453       __typeof__(*(ptr)) _o_ = (o);					 \
ae3a197e3d0bfe3 David Howells   2012-03-28  454       __typeof__(*(ptr)) _n_ = (n);					 \
ae3a197e3d0bfe3 David Howells   2012-03-28  455       (__typeof__(*(ptr))) __cmpxchg_local((ptr), (unsigned long)_o_,	 \
ae3a197e3d0bfe3 David Howells   2012-03-28  456  				    (unsigned long)_n_, sizeof(*(ptr))); \
ae3a197e3d0bfe3 David Howells   2012-03-28  457    })
ae3a197e3d0bfe3 David Howells   2012-03-28  458  
9e1bec8fe216b07 Nicholas Piggin 2020-11-11  459  #define arch_cmpxchg_relaxed(ptr, o, n)					\
56c08e6d226c860 Boqun Feng      2015-12-15  460  ({									\
56c08e6d226c860 Boqun Feng      2015-12-15  461  	__typeof__(*(ptr)) _o_ = (o);					\
56c08e6d226c860 Boqun Feng      2015-12-15  462  	__typeof__(*(ptr)) _n_ = (n);					\
56c08e6d226c860 Boqun Feng      2015-12-15 @463  	(__typeof__(*(ptr))) __cmpxchg_relaxed((ptr),			\
56c08e6d226c860 Boqun Feng      2015-12-15  464  			(unsigned long)_o_, (unsigned long)_n_,		\
56c08e6d226c860 Boqun Feng      2015-12-15  465  			sizeof(*(ptr)));				\
56c08e6d226c860 Boqun Feng      2015-12-15  466  })
56c08e6d226c860 Boqun Feng      2015-12-15  467  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Nov. 13, 2020, 5:05 a.m. UTC | #2
Hi Nicholas,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on asm-generic/master linus/master v5.10-rc3 next-20201112]
[cannot apply to scottwood/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-convert-to-use-ARCH_ATOMIC/20201111-190941
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-s031-20201111 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-107-gaf3512a6-dirty
        # https://github.com/0day-ci/linux/commit/9e1bec8fe216b0745c647e52c40d1f0033fb4efd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nicholas-Piggin/powerpc-convert-to-use-ARCH_ATOMIC/20201111-190941
        git checkout 9e1bec8fe216b0745c647e52c40d1f0033fb4efd
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


"sparse warnings: (new ones prefixed by >>)"
>> drivers/gpu/drm/drm_lock.c:75:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected void *ptr @@     got unsigned int volatile *__ai_ptr @@
   drivers/gpu/drm/drm_lock.c:75:24: sparse:     expected void *ptr
>> drivers/gpu/drm/drm_lock.c:75:24: sparse:     got unsigned int volatile *__ai_ptr
>> drivers/gpu/drm/drm_lock.c:75:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected void *ptr @@     got unsigned int volatile *__ai_ptr @@
   drivers/gpu/drm/drm_lock.c:75:24: sparse:     expected void *ptr
>> drivers/gpu/drm/drm_lock.c:75:24: sparse:     got unsigned int volatile *__ai_ptr
   drivers/gpu/drm/drm_lock.c:118:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected void *ptr @@     got unsigned int volatile *__ai_ptr @@
   drivers/gpu/drm/drm_lock.c:118:24: sparse:     expected void *ptr
   drivers/gpu/drm/drm_lock.c:118:24: sparse:     got unsigned int volatile *__ai_ptr
   drivers/gpu/drm/drm_lock.c:118:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected void *ptr @@     got unsigned int volatile *__ai_ptr @@
   drivers/gpu/drm/drm_lock.c:118:24: sparse:     expected void *ptr
   drivers/gpu/drm/drm_lock.c:118:24: sparse:     got unsigned int volatile *__ai_ptr
   drivers/gpu/drm/drm_lock.c:141:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected void *ptr @@     got unsigned int volatile *__ai_ptr @@
   drivers/gpu/drm/drm_lock.c:141:24: sparse:     expected void *ptr
   drivers/gpu/drm/drm_lock.c:141:24: sparse:     got unsigned int volatile *__ai_ptr
   drivers/gpu/drm/drm_lock.c:141:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected void *ptr @@     got unsigned int volatile *__ai_ptr @@
   drivers/gpu/drm/drm_lock.c:141:24: sparse:     expected void *ptr
   drivers/gpu/drm/drm_lock.c:141:24: sparse:     got unsigned int volatile *__ai_ptr
   drivers/gpu/drm/drm_lock.c:319:40: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected void *ptr @@     got unsigned int volatile *__ai_ptr @@
   drivers/gpu/drm/drm_lock.c:319:40: sparse:     expected void *ptr
   drivers/gpu/drm/drm_lock.c:319:40: sparse:     got unsigned int volatile *__ai_ptr
   drivers/gpu/drm/drm_lock.c:319:40: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected void *ptr @@     got unsigned int volatile *__ai_ptr @@
   drivers/gpu/drm/drm_lock.c:319:40: sparse:     expected void *ptr
   drivers/gpu/drm/drm_lock.c:319:40: sparse:     got unsigned int volatile *__ai_ptr

vim +75 drivers/gpu/drm/drm_lock.c

4ac5ec40ec70022 Daniel Vetter     2010-08-23  48  
bd50d4a2168370b Benjamin Gaignard 2020-03-06  49  /*
1a75a222f5ca106 Daniel Vetter     2016-06-14  50   * Take the heavyweight lock.
1a75a222f5ca106 Daniel Vetter     2016-06-14  51   *
1a75a222f5ca106 Daniel Vetter     2016-06-14  52   * \param lock lock pointer.
1a75a222f5ca106 Daniel Vetter     2016-06-14  53   * \param context locking context.
1a75a222f5ca106 Daniel Vetter     2016-06-14  54   * \return one if the lock is held, or zero otherwise.
1a75a222f5ca106 Daniel Vetter     2016-06-14  55   *
1a75a222f5ca106 Daniel Vetter     2016-06-14  56   * Attempt to mark the lock as held by the given context, via the \p cmpxchg instruction.
1a75a222f5ca106 Daniel Vetter     2016-06-14  57   */
1a75a222f5ca106 Daniel Vetter     2016-06-14  58  static
1a75a222f5ca106 Daniel Vetter     2016-06-14  59  int drm_lock_take(struct drm_lock_data *lock_data,
1a75a222f5ca106 Daniel Vetter     2016-06-14  60  		  unsigned int context)
1a75a222f5ca106 Daniel Vetter     2016-06-14  61  {
1a75a222f5ca106 Daniel Vetter     2016-06-14  62  	unsigned int old, new, prev;
1a75a222f5ca106 Daniel Vetter     2016-06-14  63  	volatile unsigned int *lock = &lock_data->hw_lock->lock;
1a75a222f5ca106 Daniel Vetter     2016-06-14  64  
1a75a222f5ca106 Daniel Vetter     2016-06-14  65  	spin_lock_bh(&lock_data->spinlock);
1a75a222f5ca106 Daniel Vetter     2016-06-14  66  	do {
1a75a222f5ca106 Daniel Vetter     2016-06-14  67  		old = *lock;
1a75a222f5ca106 Daniel Vetter     2016-06-14  68  		if (old & _DRM_LOCK_HELD)
1a75a222f5ca106 Daniel Vetter     2016-06-14  69  			new = old | _DRM_LOCK_CONT;
1a75a222f5ca106 Daniel Vetter     2016-06-14  70  		else {
1a75a222f5ca106 Daniel Vetter     2016-06-14  71  			new = context | _DRM_LOCK_HELD |
1a75a222f5ca106 Daniel Vetter     2016-06-14  72  				((lock_data->user_waiters + lock_data->kernel_waiters > 1) ?
1a75a222f5ca106 Daniel Vetter     2016-06-14  73  				 _DRM_LOCK_CONT : 0);
1a75a222f5ca106 Daniel Vetter     2016-06-14  74  		}
1a75a222f5ca106 Daniel Vetter     2016-06-14 @75  		prev = cmpxchg(lock, old, new);
1a75a222f5ca106 Daniel Vetter     2016-06-14  76  	} while (prev != old);
1a75a222f5ca106 Daniel Vetter     2016-06-14  77  	spin_unlock_bh(&lock_data->spinlock);
1a75a222f5ca106 Daniel Vetter     2016-06-14  78  
1a75a222f5ca106 Daniel Vetter     2016-06-14  79  	if (_DRM_LOCKING_CONTEXT(old) == context) {
1a75a222f5ca106 Daniel Vetter     2016-06-14  80  		if (old & _DRM_LOCK_HELD) {
1a75a222f5ca106 Daniel Vetter     2016-06-14  81  			if (context != DRM_KERNEL_CONTEXT) {
1a75a222f5ca106 Daniel Vetter     2016-06-14  82  				DRM_ERROR("%d holds heavyweight lock\n",
1a75a222f5ca106 Daniel Vetter     2016-06-14  83  					  context);
1a75a222f5ca106 Daniel Vetter     2016-06-14  84  			}
1a75a222f5ca106 Daniel Vetter     2016-06-14  85  			return 0;
1a75a222f5ca106 Daniel Vetter     2016-06-14  86  		}
1a75a222f5ca106 Daniel Vetter     2016-06-14  87  	}
1a75a222f5ca106 Daniel Vetter     2016-06-14  88  
1a75a222f5ca106 Daniel Vetter     2016-06-14  89  	if ((_DRM_LOCKING_CONTEXT(new)) == context && (new & _DRM_LOCK_HELD)) {
1a75a222f5ca106 Daniel Vetter     2016-06-14  90  		/* Have lock */
1a75a222f5ca106 Daniel Vetter     2016-06-14  91  		return 1;
1a75a222f5ca106 Daniel Vetter     2016-06-14  92  	}
1a75a222f5ca106 Daniel Vetter     2016-06-14  93  	return 0;
1a75a222f5ca106 Daniel Vetter     2016-06-14  94  }
1a75a222f5ca106 Daniel Vetter     2016-06-14  95  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Boqun Feng Nov. 13, 2020, 3:30 p.m. UTC | #3
Hi Nicholas,

On Wed, Nov 11, 2020 at 09:07:23PM +1000, Nicholas Piggin wrote:
> All the cool kids are doing it.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/include/asm/atomic.h  | 681 ++++++++++-------------------
>  arch/powerpc/include/asm/cmpxchg.h |  62 +--
>  2 files changed, 248 insertions(+), 495 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
> index 8a55eb8cc97b..899aa2403ba7 100644
> --- a/arch/powerpc/include/asm/atomic.h
> +++ b/arch/powerpc/include/asm/atomic.h
> @@ -11,185 +11,285 @@
>  #include <asm/cmpxchg.h>
>  #include <asm/barrier.h>
>  
> +#define ARCH_ATOMIC
> +
> +#ifndef CONFIG_64BIT
> +#include <asm-generic/atomic64.h>
> +#endif
> +
>  /*
>   * Since *_return_relaxed and {cmp}xchg_relaxed are implemented with
>   * a "bne-" instruction at the end, so an isync is enough as a acquire barrier
>   * on the platform without lwsync.
>   */
>  #define __atomic_acquire_fence()					\
> -	__asm__ __volatile__(PPC_ACQUIRE_BARRIER "" : : : "memory")
> +	asm volatile(PPC_ACQUIRE_BARRIER "" : : : "memory")
>  
>  #define __atomic_release_fence()					\
> -	__asm__ __volatile__(PPC_RELEASE_BARRIER "" : : : "memory")
> +	asm volatile(PPC_RELEASE_BARRIER "" : : : "memory")
>  
> -static __inline__ int atomic_read(const atomic_t *v)
> -{
> -	int t;
> +#define __atomic_pre_full_fence		smp_mb
>  
> -	__asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
> +#define __atomic_post_full_fence	smp_mb
>  

Do you need to define __atomic_{pre,post}_full_fence for PPC? IIRC, they
are default smp_mb__{before,atomic}_atomic(), so are smp_mb() defautly
on PPC.

> -	return t;
> +#define arch_atomic_read(v)			__READ_ONCE((v)->counter)
> +#define arch_atomic_set(v, i)			__WRITE_ONCE(((v)->counter), (i))
> +#ifdef CONFIG_64BIT
> +#define ATOMIC64_INIT(i)			{ (i) }
> +#define arch_atomic64_read(v)			__READ_ONCE((v)->counter)
> +#define arch_atomic64_set(v, i)			__WRITE_ONCE(((v)->counter), (i))
> +#endif
> +
[...]
>  
> +#define ATOMIC_FETCH_OP_UNLESS_RELAXED(name, type, dtype, width, asm_op) \
> +static inline int arch_##name##_relaxed(type *v, dtype a, dtype u)	\

I don't think we have atomic_fetch_*_unless_relaxed() at atomic APIs,
ditto for:

	atomic_fetch_add_unless_relaxed()
	atomic_inc_not_zero_relaxed()
	atomic_dec_if_positive_relaxed()

, and we don't have the _acquire() and _release() variants for them
either, and if you don't define their fully-ordered version (e.g.
atomic_inc_not_zero()), atomic-arch-fallback.h will use read and cmpxchg
to implement them, and I think not what we want.

[...]
>  
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_ATOMIC_H_ */
> diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h
> index cf091c4c22e5..181f7e8b3281 100644
> --- a/arch/powerpc/include/asm/cmpxchg.h
> +++ b/arch/powerpc/include/asm/cmpxchg.h
> @@ -192,7 +192,7 @@ __xchg_relaxed(void *ptr, unsigned long x, unsigned int size)
>       		(unsigned long)_x_, sizeof(*(ptr))); 			     \
>    })
>  
> -#define xchg_relaxed(ptr, x)						\
> +#define arch_xchg_relaxed(ptr, x)					\
>  ({									\
>  	__typeof__(*(ptr)) _x_ = (x);					\
>  	(__typeof__(*(ptr))) __xchg_relaxed((ptr),			\
> @@ -448,35 +448,7 @@ __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
>  	return old;
>  }
>  
> -static __always_inline unsigned long
> -__cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new,
> -		  unsigned int size)
> -{
> -	switch (size) {
> -	case 1:
> -		return __cmpxchg_u8_acquire(ptr, old, new);
> -	case 2:
> -		return __cmpxchg_u16_acquire(ptr, old, new);
> -	case 4:
> -		return __cmpxchg_u32_acquire(ptr, old, new);
> -#ifdef CONFIG_PPC64
> -	case 8:
> -		return __cmpxchg_u64_acquire(ptr, old, new);
> -#endif
> -	}
> -	BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg_acquire");
> -	return old;
> -}
> -#define cmpxchg(ptr, o, n)						 \
> -  ({									 \
> -     __typeof__(*(ptr)) _o_ = (o);					 \
> -     __typeof__(*(ptr)) _n_ = (n);					 \
> -     (__typeof__(*(ptr))) __cmpxchg((ptr), (unsigned long)_o_,		 \
> -				    (unsigned long)_n_, sizeof(*(ptr))); \
> -  })
> -
> -

If you remove {atomic_}_cmpxchg_{,_acquire}() and use the version
provided by atomic-arch-fallback.h, then a fail cmpxchg or
cmpxchg_acquire() will still result into a full barrier or a acquire
barrier after the RMW operation, the barrier is not necessary and
probably this is not what we want?

Regards,
Boqun

> -#define cmpxchg_local(ptr, o, n)					 \
> +#define arch_cmpxchg_local(ptr, o, n)					 \
>    ({									 \
>       __typeof__(*(ptr)) _o_ = (o);					 \
>       __typeof__(*(ptr)) _n_ = (n);					 \
> @@ -484,7 +456,7 @@ __cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new,
>  				    (unsigned long)_n_, sizeof(*(ptr))); \
>    })
>  
> -#define cmpxchg_relaxed(ptr, o, n)					\
> +#define arch_cmpxchg_relaxed(ptr, o, n)					\
>  ({									\
>  	__typeof__(*(ptr)) _o_ = (o);					\
>  	__typeof__(*(ptr)) _n_ = (n);					\
> @@ -493,38 +465,20 @@ __cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new,
>  			sizeof(*(ptr)));				\
>  })
>  
> -#define cmpxchg_acquire(ptr, o, n)					\
> -({									\
> -	__typeof__(*(ptr)) _o_ = (o);					\
> -	__typeof__(*(ptr)) _n_ = (n);					\
> -	(__typeof__(*(ptr))) __cmpxchg_acquire((ptr),			\
> -			(unsigned long)_o_, (unsigned long)_n_,		\
> -			sizeof(*(ptr)));				\
> -})
>  #ifdef CONFIG_PPC64
> -#define cmpxchg64(ptr, o, n)						\
> -  ({									\
> -	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
> -	cmpxchg((ptr), (o), (n));					\
> -  })
> -#define cmpxchg64_local(ptr, o, n)					\
> +#define arch_cmpxchg64_local(ptr, o, n)					\
>    ({									\
>  	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
> -	cmpxchg_local((ptr), (o), (n));					\
> +	arch_cmpxchg_local((ptr), (o), (n));				\
>    })
> -#define cmpxchg64_relaxed(ptr, o, n)					\
> -({									\
> -	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
> -	cmpxchg_relaxed((ptr), (o), (n));				\
> -})
> -#define cmpxchg64_acquire(ptr, o, n)					\
> +#define arch_cmpxchg64_relaxed(ptr, o, n)				\
>  ({									\
>  	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
> -	cmpxchg_acquire((ptr), (o), (n));				\
> +	arch_cmpxchg_relaxed((ptr), (o), (n));				\
>  })
>  #else
>  #include <asm-generic/cmpxchg-local.h>
> -#define cmpxchg64_local(ptr, o, n) __cmpxchg64_local_generic((ptr), (o), (n))
> +#define arch_cmpxchg64_local(ptr, o, n) __cmpxchg64_local_generic((ptr), (o), (n))
>  #endif
>  
>  #endif /* __KERNEL__ */
> -- 
> 2.23.0
>
Nicholas Piggin Dec. 22, 2020, 3:52 a.m. UTC | #4
Excerpts from Boqun Feng's message of November 14, 2020 1:30 am:
> Hi Nicholas,
> 
> On Wed, Nov 11, 2020 at 09:07:23PM +1000, Nicholas Piggin wrote:
>> All the cool kids are doing it.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  arch/powerpc/include/asm/atomic.h  | 681 ++++++++++-------------------
>>  arch/powerpc/include/asm/cmpxchg.h |  62 +--
>>  2 files changed, 248 insertions(+), 495 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
>> index 8a55eb8cc97b..899aa2403ba7 100644
>> --- a/arch/powerpc/include/asm/atomic.h
>> +++ b/arch/powerpc/include/asm/atomic.h
>> @@ -11,185 +11,285 @@
>>  #include <asm/cmpxchg.h>
>>  #include <asm/barrier.h>
>>  
>> +#define ARCH_ATOMIC
>> +
>> +#ifndef CONFIG_64BIT
>> +#include <asm-generic/atomic64.h>
>> +#endif
>> +
>>  /*
>>   * Since *_return_relaxed and {cmp}xchg_relaxed are implemented with
>>   * a "bne-" instruction at the end, so an isync is enough as a acquire barrier
>>   * on the platform without lwsync.
>>   */
>>  #define __atomic_acquire_fence()					\
>> -	__asm__ __volatile__(PPC_ACQUIRE_BARRIER "" : : : "memory")
>> +	asm volatile(PPC_ACQUIRE_BARRIER "" : : : "memory")
>>  
>>  #define __atomic_release_fence()					\
>> -	__asm__ __volatile__(PPC_RELEASE_BARRIER "" : : : "memory")
>> +	asm volatile(PPC_RELEASE_BARRIER "" : : : "memory")
>>  
>> -static __inline__ int atomic_read(const atomic_t *v)
>> -{
>> -	int t;
>> +#define __atomic_pre_full_fence		smp_mb
>>  
>> -	__asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
>> +#define __atomic_post_full_fence	smp_mb
>>  

Thanks for the review.

> Do you need to define __atomic_{pre,post}_full_fence for PPC? IIRC, they
> are default smp_mb__{before,atomic}_atomic(), so are smp_mb() defautly
> on PPC.

Okay I didn't realise that's not required.

>> -	return t;
>> +#define arch_atomic_read(v)			__READ_ONCE((v)->counter)
>> +#define arch_atomic_set(v, i)			__WRITE_ONCE(((v)->counter), (i))
>> +#ifdef CONFIG_64BIT
>> +#define ATOMIC64_INIT(i)			{ (i) }
>> +#define arch_atomic64_read(v)			__READ_ONCE((v)->counter)
>> +#define arch_atomic64_set(v, i)			__WRITE_ONCE(((v)->counter), (i))
>> +#endif
>> +
> [...]
>>  
>> +#define ATOMIC_FETCH_OP_UNLESS_RELAXED(name, type, dtype, width, asm_op) \
>> +static inline int arch_##name##_relaxed(type *v, dtype a, dtype u)	\
> 
> I don't think we have atomic_fetch_*_unless_relaxed() at atomic APIs,
> ditto for:
> 
> 	atomic_fetch_add_unless_relaxed()
> 	atomic_inc_not_zero_relaxed()
> 	atomic_dec_if_positive_relaxed()
> 
> , and we don't have the _acquire() and _release() variants for them
> either, and if you don't define their fully-ordered version (e.g.
> atomic_inc_not_zero()), atomic-arch-fallback.h will use read and cmpxchg
> to implement them, and I think not what we want.

Okay. How can those be added? The atoimc generation is pretty 
complicated.

> [...]
>>  
>>  #endif /* __KERNEL__ */
>>  #endif /* _ASM_POWERPC_ATOMIC_H_ */
>> diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h
>> index cf091c4c22e5..181f7e8b3281 100644
>> --- a/arch/powerpc/include/asm/cmpxchg.h
>> +++ b/arch/powerpc/include/asm/cmpxchg.h
>> @@ -192,7 +192,7 @@ __xchg_relaxed(void *ptr, unsigned long x, unsigned int size)
>>       		(unsigned long)_x_, sizeof(*(ptr))); 			     \
>>    })
>>  
>> -#define xchg_relaxed(ptr, x)						\
>> +#define arch_xchg_relaxed(ptr, x)					\
>>  ({									\
>>  	__typeof__(*(ptr)) _x_ = (x);					\
>>  	(__typeof__(*(ptr))) __xchg_relaxed((ptr),			\
>> @@ -448,35 +448,7 @@ __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
>>  	return old;
>>  }
>>  
>> -static __always_inline unsigned long
>> -__cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new,
>> -		  unsigned int size)
>> -{
>> -	switch (size) {
>> -	case 1:
>> -		return __cmpxchg_u8_acquire(ptr, old, new);
>> -	case 2:
>> -		return __cmpxchg_u16_acquire(ptr, old, new);
>> -	case 4:
>> -		return __cmpxchg_u32_acquire(ptr, old, new);
>> -#ifdef CONFIG_PPC64
>> -	case 8:
>> -		return __cmpxchg_u64_acquire(ptr, old, new);
>> -#endif
>> -	}
>> -	BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg_acquire");
>> -	return old;
>> -}
>> -#define cmpxchg(ptr, o, n)						 \
>> -  ({									 \
>> -     __typeof__(*(ptr)) _o_ = (o);					 \
>> -     __typeof__(*(ptr)) _n_ = (n);					 \
>> -     (__typeof__(*(ptr))) __cmpxchg((ptr), (unsigned long)_o_,		 \
>> -				    (unsigned long)_n_, sizeof(*(ptr))); \
>> -  })
>> -
>> -
> 
> If you remove {atomic_}_cmpxchg_{,_acquire}() and use the version
> provided by atomic-arch-fallback.h, then a fail cmpxchg or
> cmpxchg_acquire() will still result into a full barrier or a acquire
> barrier after the RMW operation, the barrier is not necessary and
> probably this is not what we want?

Why is that done? That seems like a very subtle difference. Shouldn't
the fallback version skip the barrier?

Thanks,
Nick
Boqun Feng Dec. 23, 2020, 2:45 a.m. UTC | #5
On Tue, Dec 22, 2020 at 01:52:50PM +1000, Nicholas Piggin wrote:
> Excerpts from Boqun Feng's message of November 14, 2020 1:30 am:
> > Hi Nicholas,
> > 
> > On Wed, Nov 11, 2020 at 09:07:23PM +1000, Nicholas Piggin wrote:
> >> All the cool kids are doing it.
> >> 
> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >> ---
> >>  arch/powerpc/include/asm/atomic.h  | 681 ++++++++++-------------------
> >>  arch/powerpc/include/asm/cmpxchg.h |  62 +--
> >>  2 files changed, 248 insertions(+), 495 deletions(-)
> >> 
> >> diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
> >> index 8a55eb8cc97b..899aa2403ba7 100644
> >> --- a/arch/powerpc/include/asm/atomic.h
> >> +++ b/arch/powerpc/include/asm/atomic.h
> >> @@ -11,185 +11,285 @@
> >>  #include <asm/cmpxchg.h>
> >>  #include <asm/barrier.h>
> >>  
> >> +#define ARCH_ATOMIC
> >> +
> >> +#ifndef CONFIG_64BIT
> >> +#include <asm-generic/atomic64.h>
> >> +#endif
> >> +
> >>  /*
> >>   * Since *_return_relaxed and {cmp}xchg_relaxed are implemented with
> >>   * a "bne-" instruction at the end, so an isync is enough as a acquire barrier
> >>   * on the platform without lwsync.
> >>   */
> >>  #define __atomic_acquire_fence()					\
> >> -	__asm__ __volatile__(PPC_ACQUIRE_BARRIER "" : : : "memory")
> >> +	asm volatile(PPC_ACQUIRE_BARRIER "" : : : "memory")
> >>  
> >>  #define __atomic_release_fence()					\
> >> -	__asm__ __volatile__(PPC_RELEASE_BARRIER "" : : : "memory")
> >> +	asm volatile(PPC_RELEASE_BARRIER "" : : : "memory")
> >>  
> >> -static __inline__ int atomic_read(const atomic_t *v)
> >> -{
> >> -	int t;
> >> +#define __atomic_pre_full_fence		smp_mb
> >>  
> >> -	__asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
> >> +#define __atomic_post_full_fence	smp_mb
> >>  
> 
> Thanks for the review.
> 
> > Do you need to define __atomic_{pre,post}_full_fence for PPC? IIRC, they
> > are default smp_mb__{before,atomic}_atomic(), so are smp_mb() defautly
> > on PPC.
> 
> Okay I didn't realise that's not required.
> 
> >> -	return t;
> >> +#define arch_atomic_read(v)			__READ_ONCE((v)->counter)
> >> +#define arch_atomic_set(v, i)			__WRITE_ONCE(((v)->counter), (i))
> >> +#ifdef CONFIG_64BIT
> >> +#define ATOMIC64_INIT(i)			{ (i) }
> >> +#define arch_atomic64_read(v)			__READ_ONCE((v)->counter)
> >> +#define arch_atomic64_set(v, i)			__WRITE_ONCE(((v)->counter), (i))
> >> +#endif
> >> +
> > [...]
> >>  
> >> +#define ATOMIC_FETCH_OP_UNLESS_RELAXED(name, type, dtype, width, asm_op) \
> >> +static inline int arch_##name##_relaxed(type *v, dtype a, dtype u)	\
> > 
> > I don't think we have atomic_fetch_*_unless_relaxed() at atomic APIs,
> > ditto for:
> > 
> > 	atomic_fetch_add_unless_relaxed()
> > 	atomic_inc_not_zero_relaxed()
> > 	atomic_dec_if_positive_relaxed()
> > 
> > , and we don't have the _acquire() and _release() variants for them
> > either, and if you don't define their fully-ordered version (e.g.
> > atomic_inc_not_zero()), atomic-arch-fallback.h will use read and cmpxchg
> > to implement them, and I think not what we want.
> 
> Okay. How can those be added? The atoimc generation is pretty 
> complicated.
> 

Yeah, I know ;-) I think you can just implement and define fully-ordered
verions:

	arch_atomic_fetch_*_unless()
	arch_atomic_inc_not_zero()
	arch_atomic_dec_if_postive()

, that should work.

Rules of atomic generation, IIRC:

1.	If you define _relaxed, _acquire, _release or fully-ordered
	version, atomic generation will use that version

2.	If you define _relaxed, atomic generation will use that and
	barriers to generate _acquire, _release and fully-ordered
	versions, unless they are already defined (as Rule #1 says)

3.	If you don't define _relaxed, but define the fully-ordered
	version, atomic generation will use the fully-ordered version
	and use it as _relaxed variants and generate the rest using Rule
	#2.

> > [...]
> >>  
> >>  #endif /* __KERNEL__ */
> >>  #endif /* _ASM_POWERPC_ATOMIC_H_ */
> >> diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h
> >> index cf091c4c22e5..181f7e8b3281 100644
> >> --- a/arch/powerpc/include/asm/cmpxchg.h
> >> +++ b/arch/powerpc/include/asm/cmpxchg.h
> >> @@ -192,7 +192,7 @@ __xchg_relaxed(void *ptr, unsigned long x, unsigned int size)
> >>       		(unsigned long)_x_, sizeof(*(ptr))); 			     \
> >>    })
> >>  
> >> -#define xchg_relaxed(ptr, x)						\
> >> +#define arch_xchg_relaxed(ptr, x)					\
> >>  ({									\
> >>  	__typeof__(*(ptr)) _x_ = (x);					\
> >>  	(__typeof__(*(ptr))) __xchg_relaxed((ptr),			\
> >> @@ -448,35 +448,7 @@ __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
> >>  	return old;
> >>  }
> >>  
> >> -static __always_inline unsigned long
> >> -__cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new,
> >> -		  unsigned int size)
> >> -{
> >> -	switch (size) {
> >> -	case 1:
> >> -		return __cmpxchg_u8_acquire(ptr, old, new);
> >> -	case 2:
> >> -		return __cmpxchg_u16_acquire(ptr, old, new);
> >> -	case 4:
> >> -		return __cmpxchg_u32_acquire(ptr, old, new);
> >> -#ifdef CONFIG_PPC64
> >> -	case 8:
> >> -		return __cmpxchg_u64_acquire(ptr, old, new);
> >> -#endif
> >> -	}
> >> -	BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg_acquire");
> >> -	return old;
> >> -}
> >> -#define cmpxchg(ptr, o, n)						 \
> >> -  ({									 \
> >> -     __typeof__(*(ptr)) _o_ = (o);					 \
> >> -     __typeof__(*(ptr)) _n_ = (n);					 \
> >> -     (__typeof__(*(ptr))) __cmpxchg((ptr), (unsigned long)_o_,		 \
> >> -				    (unsigned long)_n_, sizeof(*(ptr))); \
> >> -  })
> >> -
> >> -
> > 
> > If you remove {atomic_}_cmpxchg_{,_acquire}() and use the version
> > provided by atomic-arch-fallback.h, then a fail cmpxchg or
> > cmpxchg_acquire() will still result into a full barrier or a acquire
> > barrier after the RMW operation, the barrier is not necessary and
> > probably this is not what we want?
> 
> Why is that done? That seems like a very subtle difference. Shouldn't
> the fallback version skip the barrier?
> 

The fallback version is something like:

	smp_mb__before_atomic();
	cmpxchg_relaxed();
	smp_mb__after_atomic();

, so there will be a full barrier on PPC after the cmpxchg no matter
whether the cmpxchg succeed or not. And the fallback version cannot skip
the barrier, because there is no way the fallback version tells whether
the cmpxchg_relaxed() succeed or not. So in my previous version of PPC
atomic variants support, I defined cmpxchg_acquire() in asm header
instead of using atomic generation.

That said, now I think about this, maybe we can implement the fallback
version as:

	smp_mb__before_atomic();
	ret = cmpxchg_relaxed(ptr, old, new);
	if (old == ret)
		smp_mb__after_atomic();

, in this way, the fallback version can handle the barrier skipping
better.

Regards,
Boqun

> Thanks,
> Nick
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
index 8a55eb8cc97b..899aa2403ba7 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -11,185 +11,285 @@ 
 #include <asm/cmpxchg.h>
 #include <asm/barrier.h>
 
+#define ARCH_ATOMIC
+
+#ifndef CONFIG_64BIT
+#include <asm-generic/atomic64.h>
+#endif
+
 /*
  * Since *_return_relaxed and {cmp}xchg_relaxed are implemented with
  * a "bne-" instruction at the end, so an isync is enough as a acquire barrier
  * on the platform without lwsync.
  */
 #define __atomic_acquire_fence()					\
-	__asm__ __volatile__(PPC_ACQUIRE_BARRIER "" : : : "memory")
+	asm volatile(PPC_ACQUIRE_BARRIER "" : : : "memory")
 
 #define __atomic_release_fence()					\
-	__asm__ __volatile__(PPC_RELEASE_BARRIER "" : : : "memory")
+	asm volatile(PPC_RELEASE_BARRIER "" : : : "memory")
 
-static __inline__ int atomic_read(const atomic_t *v)
-{
-	int t;
+#define __atomic_pre_full_fence		smp_mb
 
-	__asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
+#define __atomic_post_full_fence	smp_mb
 
-	return t;
+#define arch_atomic_read(v)			__READ_ONCE((v)->counter)
+#define arch_atomic_set(v, i)			__WRITE_ONCE(((v)->counter), (i))
+#ifdef CONFIG_64BIT
+#define ATOMIC64_INIT(i)			{ (i) }
+#define arch_atomic64_read(v)			__READ_ONCE((v)->counter)
+#define arch_atomic64_set(v, i)			__WRITE_ONCE(((v)->counter), (i))
+#endif
+
+#define ATOMIC_OP(name, type, dtype, width, asm_op)			\
+static inline void arch_##name(dtype a, type *v)			\
+{									\
+	dtype t;							\
+									\
+	asm volatile(							\
+"1:	l" #width "arx	%0,0,%3		# " #name		"\n"	\
+"\t"	#asm_op " %0,%2,%0					\n"	\
+"	st" #width "cx.	%0,0,%3					\n"	\
+"	bne-	1b						\n"	\
+	: "=&r" (t), "+m" (v->counter)					\
+	: "r" (a), "r" (&v->counter)					\
+	: "cr0", "xer");						\
 }
 
-static __inline__ void atomic_set(atomic_t *v, int i)
-{
-	__asm__ __volatile__("stw%U0%X0 %1,%0" : "=m"(v->counter) : "r"(i));
+#define ATOMIC_OP_IMM(name, type, dtype, width, asm_op, imm)		\
+static inline void arch_##name(type *v)					\
+{									\
+	dtype t;							\
+									\
+	asm volatile(							\
+"1:	l" #width "arx	%0,0,%3		# " #name		"\n"	\
+"\t"	#asm_op " %0,%0,%2					\n"	\
+"	st" #width "cx.	%0,0,%3					\n"	\
+"	bne-	1b						\n"	\
+	: "=&r" (t), "+m" (v->counter)					\
+	: "i" (imm), "r" (&v->counter)					\
+	: "cr0", "xer");						\
 }
 
-#define ATOMIC_OP(op, asm_op)						\
-static __inline__ void atomic_##op(int a, atomic_t *v)			\
+#define ATOMIC_OP_RETURN_RELAXED(name, type, dtype, width, asm_op)	\
+static inline dtype arch_##name##_relaxed(dtype a, type *v)		\
 {									\
-	int t;								\
+	dtype t;							\
 									\
-	__asm__ __volatile__(						\
-"1:	lwarx	%0,0,%3		# atomic_" #op "\n"			\
-	#asm_op " %0,%2,%0\n"						\
-"	stwcx.	%0,0,%3 \n"						\
-"	bne-	1b\n"							\
+	asm volatile(							\
+"1:	l" #width "arx	%0,0,%3		# " #name 		"\n"	\
+"\t"	#asm_op " %0,%2,%0					\n"	\
+"	st" #width "cx.	%0,0,%3					\n"	\
+"	bne-	1b						\n"	\
 	: "=&r" (t), "+m" (v->counter)					\
 	: "r" (a), "r" (&v->counter)					\
-	: "cc");							\
-}									\
+	: "cr0", "xer");						\
+									\
+	return t;							\
+}
 
-#define ATOMIC_OP_RETURN_RELAXED(op, asm_op)				\
-static inline int atomic_##op##_return_relaxed(int a, atomic_t *v)	\
+#define ATOMIC_OP_IMM_RETURN_RELAXED(name, type, dtype, width, asm_op, imm) \
+static inline dtype arch_##name##_relaxed(type *v)			\
 {									\
-	int t;								\
+	dtype t;							\
 									\
-	__asm__ __volatile__(						\
-"1:	lwarx	%0,0,%3		# atomic_" #op "_return_relaxed\n"	\
-	#asm_op " %0,%2,%0\n"						\
-"	stwcx.	%0,0,%3\n"						\
-"	bne-	1b\n"							\
+	asm volatile(							\
+"1:	l" #width "arx	%0,0,%3		# " #name		"\n"	\
+"\t"	#asm_op " %0,%0,%2					\n"	\
+"	st" #width "cx.	%0,0,%3					\n"	\
+"	bne-	1b						\n"	\
 	: "=&r" (t), "+m" (v->counter)					\
-	: "r" (a), "r" (&v->counter)					\
-	: "cc");							\
+	: "i" (imm), "r" (&v->counter)					\
+	: "cr0", "xer");						\
 									\
 	return t;							\
 }
 
-#define ATOMIC_FETCH_OP_RELAXED(op, asm_op)				\
-static inline int atomic_fetch_##op##_relaxed(int a, atomic_t *v)	\
+#define ATOMIC_FETCH_OP_RELAXED(name, type, dtype, width, asm_op)	\
+static inline dtype arch_##name##_relaxed(dtype a, type *v)		\
 {									\
-	int res, t;							\
+	dtype res, t;							\
 									\
-	__asm__ __volatile__(						\
-"1:	lwarx	%0,0,%4		# atomic_fetch_" #op "_relaxed\n"	\
-	#asm_op " %1,%3,%0\n"						\
-"	stwcx.	%1,0,%4\n"						\
-"	bne-	1b\n"							\
+	asm volatile(							\
+"1:	l" #width "arx	%0,0,%4		# " #name		"\n"	\
+"\t"	#asm_op " %1,%3,%0					\n"	\
+"	st" #width "cx.	%1,0,%4					\n"	\
+"	bne-	1b						\n"	\
 	: "=&r" (res), "=&r" (t), "+m" (v->counter)			\
 	: "r" (a), "r" (&v->counter)					\
-	: "cc");							\
+	: "cr0", "xer");						\
 									\
 	return res;							\
 }
 
+#define ATOMIC_FETCH_OP_UNLESS_RELAXED(name, type, dtype, width, asm_op) \
+static inline int arch_##name##_relaxed(type *v, dtype a, dtype u)	\
+{									\
+	dtype res, t;							\
+									\
+	asm volatile(							\
+"1:	l" #width "arx	%0,0,%5		# " #name		"\n"	\
+"	cmp" #width "	0,%0,%3					\n"	\
+"	beq-	2f						\n"	\
+"\t"	#asm_op " %1,%2,%0					\n"	\
+"	st" #width "cx.	%1,0,%5					\n"	\
+"	bne-	1b						\n"	\
+"2:								\n"	\
+	: "=&r" (res), "=&r" (t), "+m" (v->counter)			\
+	: "r" (a), "r" (u), "r" (&v->counter)				\
+	: "cr0", "xer");						\
+									\
+	return res;							\
+}
+
+#define ATOMIC_INC_NOT_ZERO_RELAXED(name, type, dtype, width)		\
+static inline dtype arch_##name##_relaxed(type *v)			\
+{									\
+	dtype t1, t2;							\
+									\
+	asm volatile(							\
+"1:	l" #width "arx	%0,0,%3		# " #name		"\n"	\
+"	cmp" #width "i	0,%0,0					\n"	\
+"	beq-	2f						\n"	\
+"	addic	%1,%2,1						\n"	\
+"	st" #width "cx.	%1,0,%3					\n"	\
+"	bne-	1b						\n"	\
+"2:								\n"	\
+	: "=&r" (t1), "=&r" (t2), "+m" (v->counter)			\
+	: "r" (&v->counter)						\
+	: "cr0", "xer");						\
+									\
+	return t1;							\
+}
+
+#undef ATOMIC_OPS
 #define ATOMIC_OPS(op, asm_op)						\
-	ATOMIC_OP(op, asm_op)						\
-	ATOMIC_OP_RETURN_RELAXED(op, asm_op)				\
-	ATOMIC_FETCH_OP_RELAXED(op, asm_op)
+ATOMIC_OP(atomic_##op, atomic_t, int, w, asm_op)			\
+ATOMIC_OP_RETURN_RELAXED(atomic_##op##_return, atomic_t, int, w, asm_op) \
+ATOMIC_FETCH_OP_RELAXED(atomic_fetch_##op, atomic_t, int, w, asm_op)	\
+ATOMIC_FETCH_OP_UNLESS_RELAXED(atomic_fetch_##op##_unless, atomic_t, int, w, asm_op)
+
+#undef ATOMIC64_OPS
+#define ATOMIC64_OPS(op, asm_op)					\
+ATOMIC_OP(atomic64_##op, atomic64_t, u64, d, asm_op)			\
+ATOMIC_OP_RETURN_RELAXED(atomic64_##op##_return, atomic64_t, u64, d, asm_op) \
+ATOMIC_FETCH_OP_RELAXED(atomic64_fetch_##op, atomic64_t, u64, d, asm_op) \
+ATOMIC_FETCH_OP_UNLESS_RELAXED(atomic64_fetch_##op##_unless, atomic64_t, u64, d, asm_op)
 
 ATOMIC_OPS(add, add)
+#define arch_atomic_add arch_atomic_add
+#define arch_atomic_add_return_relaxed arch_atomic_add_return_relaxed
+#define arch_atomic_fetch_add_relaxed arch_atomic_fetch_add_relaxed
+#define arch_atomic_fetch_add_unless_relaxed arch_atomic_fetch_add_unless_relaxed
+
 ATOMIC_OPS(sub, subf)
+#define arch_atomic_sub arch_atomic_sub
+#define arch_atomic_sub_return_relaxed arch_atomic_sub_return_relaxed
+#define arch_atomic_fetch_sub_relaxed arch_atomic_fetch_sub_relaxed
+/* skip atomic_fetch_sub_unless_relaxed */
 
-#define atomic_add_return_relaxed atomic_add_return_relaxed
-#define atomic_sub_return_relaxed atomic_sub_return_relaxed
+#ifdef CONFIG_64BIT
+ATOMIC64_OPS(add, add)
+#define arch_atomic64_add arch_atomic64_add
+#define arch_atomic64_add_return_relaxed arch_atomic64_add_return_relaxed
+#define arch_atomic64_fetch_add_relaxed arch_atomic64_fetch_add_relaxed
+#define arch_atomic64_fetch_add_unless_relaxed arch_atomic64_fetch_add_unless_relaxed
 
-#define atomic_fetch_add_relaxed atomic_fetch_add_relaxed
-#define atomic_fetch_sub_relaxed atomic_fetch_sub_relaxed
+ATOMIC64_OPS(sub, subf)
+#define arch_atomic64_sub arch_atomic64_sub
+#define arch_atomic64_sub_return_relaxed arch_atomic64_sub_return_relaxed
+#define arch_atomic64_fetch_sub_relaxed arch_atomic64_fetch_sub_relaxed
+/* skip atomic64_fetch_sub_unless_relaxed */
+#endif
 
 #undef ATOMIC_OPS
 #define ATOMIC_OPS(op, asm_op)						\
-	ATOMIC_OP(op, asm_op)						\
-	ATOMIC_FETCH_OP_RELAXED(op, asm_op)
+ATOMIC_OP(atomic_##op, atomic_t, int, w, asm_op)			\
+ATOMIC_FETCH_OP_RELAXED(atomic_fetch_##op, atomic_t, int, w, asm_op)
+
+#undef ATOMIC64_OPS
+#define ATOMIC64_OPS(op, asm_op)					\
+ATOMIC_OP(atomic64_##op, atomic64_t, u64, d, asm_op)			\
+ATOMIC_FETCH_OP_RELAXED(atomic64_fetch_##op, atomic64_t, u64, d, asm_op)
 
 ATOMIC_OPS(and, and)
+#define arch_atomic_and arch_atomic_and
+#define arch_atomic_fetch_and_relaxed arch_atomic_fetch_and_relaxed
+
 ATOMIC_OPS(or, or)
+#define arch_atomic_or arch_atomic_or
+#define arch_atomic_fetch_or_relaxed  arch_atomic_fetch_or_relaxed
+
 ATOMIC_OPS(xor, xor)
+#define arch_atomic_xor arch_atomic_xor
+#define arch_atomic_fetch_xor_relaxed arch_atomic_fetch_xor_relaxed
+
+#ifdef CONFIG_64BIT
+ATOMIC64_OPS(and, and)
+#define arch_atomic64_and arch_atomic64_and
+#define arch_atomic64_fetch_and_relaxed arch_atomic64_fetch_and_relaxed
 
-#define atomic_fetch_and_relaxed atomic_fetch_and_relaxed
-#define atomic_fetch_or_relaxed  atomic_fetch_or_relaxed
-#define atomic_fetch_xor_relaxed atomic_fetch_xor_relaxed
+ATOMIC64_OPS(or, or)
+#define arch_atomic64_or arch_atomic64_or
+#define arch_atomic64_fetch_or_relaxed  arch_atomic64_fetch_or_relaxed
+
+ATOMIC64_OPS(xor, xor)
+#define arch_atomic64_xor arch_atomic64_xor
+#define arch_atomic64_fetch_xor_relaxed arch_atomic64_fetch_xor_relaxed
+#endif
 
 #undef ATOMIC_OPS
+#define ATOMIC_OPS(op, asm_op, imm)					\
+ATOMIC_OP_IMM(atomic_##op, atomic_t, int, w, asm_op, imm)		\
+ATOMIC_OP_IMM_RETURN_RELAXED(atomic_##op##_return, atomic_t, int, w, asm_op, imm)
+
+#undef ATOMIC64_OPS
+#define ATOMIC64_OPS(op, asm_op, imm)					\
+ATOMIC_OP_IMM(atomic64_##op, atomic64_t, u64, d, asm_op, imm)		\
+ATOMIC_OP_IMM_RETURN_RELAXED(atomic64_##op##_return, atomic64_t, u64, d, asm_op, imm)
+
+ATOMIC_OPS(inc, addic, 1)
+#define arch_atomic_inc arch_atomic_inc
+#define arch_atomic_inc_return_relaxed arch_atomic_inc_return_relaxed
+
+ATOMIC_OPS(dec, addic, -1)
+#define arch_atomic_dec arch_atomic_dec
+#define arch_atomic_dec_return_relaxed arch_atomic_dec_return_relaxed
+
+#ifdef CONFIG_64BIT
+ATOMIC64_OPS(inc, addic, 1)
+#define arch_atomic64_inc arch_atomic64_inc
+#define arch_atomic64_inc_return_relaxed arch_atomic64_inc_return_relaxed
+
+ATOMIC64_OPS(dec, addic, -1)
+#define arch_atomic64_dec arch_atomic64_dec
+#define arch_atomic64_dec_return_relaxed arch_atomic64_dec_return_relaxed
+#endif
+
+ATOMIC_INC_NOT_ZERO_RELAXED(atomic_inc_not_zero, atomic_t, int, w)
+#define arch_atomic_inc_not_zero_relaxed(v) arch_atomic_inc_not_zero_relaxed(v)
+
+#ifdef CONFIG_64BIT
+ATOMIC_INC_NOT_ZERO_RELAXED(atomic64_inc_not_zero, atomic64_t, u64, d)
+#define arch_atomic64_inc_not_zero_relaxed(v) arch_atomic64_inc_not_zero_relaxed(v)
+#endif
+
+#undef ATOMIC_INC_NOT_ZERO_RELAXED
+#undef ATOMIC_FETCH_OP_UNLESS_RELAXED
 #undef ATOMIC_FETCH_OP_RELAXED
+#undef ATOMIC_OP_IMM_RETURN_RELAXED
 #undef ATOMIC_OP_RETURN_RELAXED
+#undef ATOMIC_OP_IMM
 #undef ATOMIC_OP
+#undef ATOMIC_OPS
+#undef ATOMIC64_OPS
 
-static __inline__ void atomic_inc(atomic_t *v)
-{
-	int t;
-
-	__asm__ __volatile__(
-"1:	lwarx	%0,0,%2		# atomic_inc\n\
-	addic	%0,%0,1\n"
-"	stwcx.	%0,0,%2 \n\
-	bne-	1b"
-	: "=&r" (t), "+m" (v->counter)
-	: "r" (&v->counter)
-	: "cc", "xer");
-}
-#define atomic_inc atomic_inc
-
-static __inline__ int atomic_inc_return_relaxed(atomic_t *v)
-{
-	int t;
-
-	__asm__ __volatile__(
-"1:	lwarx	%0,0,%2		# atomic_inc_return_relaxed\n"
-"	addic	%0,%0,1\n"
-"	stwcx.	%0,0,%2\n"
-"	bne-	1b"
-	: "=&r" (t), "+m" (v->counter)
-	: "r" (&v->counter)
-	: "cc", "xer");
-
-	return t;
-}
-
-static __inline__ void atomic_dec(atomic_t *v)
-{
-	int t;
-
-	__asm__ __volatile__(
-"1:	lwarx	%0,0,%2		# atomic_dec\n\
-	addic	%0,%0,-1\n"
-"	stwcx.	%0,0,%2\n\
-	bne-	1b"
-	: "=&r" (t), "+m" (v->counter)
-	: "r" (&v->counter)
-	: "cc", "xer");
-}
-#define atomic_dec atomic_dec
-
-static __inline__ int atomic_dec_return_relaxed(atomic_t *v)
-{
-	int t;
-
-	__asm__ __volatile__(
-"1:	lwarx	%0,0,%2		# atomic_dec_return_relaxed\n"
-"	addic	%0,%0,-1\n"
-"	stwcx.	%0,0,%2\n"
-"	bne-	1b"
-	: "=&r" (t), "+m" (v->counter)
-	: "r" (&v->counter)
-	: "cc", "xer");
-
-	return t;
-}
-
-#define atomic_inc_return_relaxed atomic_inc_return_relaxed
-#define atomic_dec_return_relaxed atomic_dec_return_relaxed
-
-#define atomic_cmpxchg(v, o, n) (cmpxchg(&((v)->counter), (o), (n)))
-#define atomic_cmpxchg_relaxed(v, o, n) \
-	cmpxchg_relaxed(&((v)->counter), (o), (n))
-#define atomic_cmpxchg_acquire(v, o, n) \
-	cmpxchg_acquire(&((v)->counter), (o), (n))
+#define arch_atomic_cmpxchg_relaxed(v, o, n) arch_cmpxchg_relaxed(&((v)->counter), (o), (n))
+#define arch_atomic_xchg_relaxed(v, new) arch_xchg_relaxed(&((v)->counter), (new))
 
-#define atomic_xchg(v, new) (xchg(&((v)->counter), new))
-#define atomic_xchg_relaxed(v, new) xchg_relaxed(&((v)->counter), (new))
+#ifdef CONFIG_64BIT
+#define arch_atomic64_cmpxchg_relaxed(v, o, n) arch_cmpxchg_relaxed(&((v)->counter), (o), (n))
+#define arch_atomic64_xchg_relaxed(v, new) arch_xchg_relaxed(&((v)->counter), (new))
+#endif
 
 /*
  * Don't want to override the generic atomic_try_cmpxchg_acquire, because
@@ -203,7 +303,7 @@  atomic_try_cmpxchg_lock(atomic_t *v, int *old, int new)
 	int r, o = *old;
 
 	__asm__ __volatile__ (
-"1:\t"	PPC_LWARX(%0,0,%2,1) "	# atomic_try_cmpxchg_acquire	\n"
+"1:\t"	PPC_LWARX(%0,0,%2,1) "	# atomic_try_cmpxchg_lock		\n"
 "	cmpw	0,%0,%3							\n"
 "	bne-	2f							\n"
 "	stwcx.	%4,0,%2							\n"
@@ -219,270 +319,41 @@  atomic_try_cmpxchg_lock(atomic_t *v, int *old, int new)
 	return likely(r == o);
 }
 
-/**
- * atomic_fetch_add_unless - add unless the number is a given value
- * @v: pointer of type atomic_t
- * @a: the amount to add to v...
- * @u: ...unless v is equal to u.
- *
- * Atomically adds @a to @v, so long as it was not @u.
- * Returns the old value of @v.
- */
-static __inline__ int atomic_fetch_add_unless(atomic_t *v, int a, int u)
-{
-	int t;
-
-	__asm__ __volatile__ (
-	PPC_ATOMIC_ENTRY_BARRIER
-"1:	lwarx	%0,0,%1		# atomic_fetch_add_unless\n\
-	cmpw	0,%0,%3 \n\
-	beq	2f \n\
-	add	%0,%2,%0 \n"
-"	stwcx.	%0,0,%1 \n\
-	bne-	1b \n"
-	PPC_ATOMIC_EXIT_BARRIER
-"	subf	%0,%2,%0 \n\
-2:"
-	: "=&r" (t)
-	: "r" (&v->counter), "r" (a), "r" (u)
-	: "cc", "memory");
-
-	return t;
-}
-#define atomic_fetch_add_unless atomic_fetch_add_unless
-
-/**
- * atomic_inc_not_zero - increment unless the number is zero
- * @v: pointer of type atomic_t
- *
- * Atomically increments @v by 1, so long as @v is non-zero.
- * Returns non-zero if @v was non-zero, and zero otherwise.
- */
-static __inline__ int atomic_inc_not_zero(atomic_t *v)
-{
-	int t1, t2;
-
-	__asm__ __volatile__ (
-	PPC_ATOMIC_ENTRY_BARRIER
-"1:	lwarx	%0,0,%2		# atomic_inc_not_zero\n\
-	cmpwi	0,%0,0\n\
-	beq-	2f\n\
-	addic	%1,%0,1\n"
-"	stwcx.	%1,0,%2\n\
-	bne-	1b\n"
-	PPC_ATOMIC_EXIT_BARRIER
-	"\n\
-2:"
-	: "=&r" (t1), "=&r" (t2)
-	: "r" (&v->counter)
-	: "cc", "xer", "memory");
-
-	return t1;
-}
-#define atomic_inc_not_zero(v) atomic_inc_not_zero((v))
-
 /*
  * Atomically test *v and decrement if it is greater than 0.
  * The function returns the old value of *v minus 1, even if
  * the atomic variable, v, was not decremented.
  */
-static __inline__ int atomic_dec_if_positive(atomic_t *v)
+static inline int atomic_dec_if_positive_relaxed(atomic_t *v)
 {
 	int t;
 
-	__asm__ __volatile__(
-	PPC_ATOMIC_ENTRY_BARRIER
-"1:	lwarx	%0,0,%1		# atomic_dec_if_positive\n\
-	cmpwi	%0,1\n\
-	addi	%0,%0,-1\n\
-	blt-	2f\n"
-"	stwcx.	%0,0,%1\n\
-	bne-	1b"
-	PPC_ATOMIC_EXIT_BARRIER
-	"\n\
-2:"	: "=&b" (t)
+	asm volatile(
+"1:	lwarx	%0,0,%1		# atomic_dec_if_positive		\n"
+"	cmpwi	%0,1							\n"
+"	addi	%0,%0,-1						\n"
+"	blt-	2f							\n"
+"	stwcx.	%0,0,%1							\n"
+"	bne-	1b							\n"
+"2:									\n"
+	: "=&b" (t)
 	: "r" (&v->counter)
 	: "cc", "memory");
 
 	return t;
 }
-#define atomic_dec_if_positive atomic_dec_if_positive
-
-#ifdef __powerpc64__
-
-#define ATOMIC64_INIT(i)	{ (i) }
-
-static __inline__ s64 atomic64_read(const atomic64_t *v)
-{
-	s64 t;
-
-	__asm__ __volatile__("ld%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
-
-	return t;
-}
-
-static __inline__ void atomic64_set(atomic64_t *v, s64 i)
-{
-	__asm__ __volatile__("std%U0%X0 %1,%0" : "=m"(v->counter) : "r"(i));
-}
-
-#define ATOMIC64_OP(op, asm_op)						\
-static __inline__ void atomic64_##op(s64 a, atomic64_t *v)		\
-{									\
-	s64 t;								\
-									\
-	__asm__ __volatile__(						\
-"1:	ldarx	%0,0,%3		# atomic64_" #op "\n"			\
-	#asm_op " %0,%2,%0\n"						\
-"	stdcx.	%0,0,%3 \n"						\
-"	bne-	1b\n"							\
-	: "=&r" (t), "+m" (v->counter)					\
-	: "r" (a), "r" (&v->counter)					\
-	: "cc");							\
-}
-
-#define ATOMIC64_OP_RETURN_RELAXED(op, asm_op)				\
-static inline s64							\
-atomic64_##op##_return_relaxed(s64 a, atomic64_t *v)			\
-{									\
-	s64 t;								\
-									\
-	__asm__ __volatile__(						\
-"1:	ldarx	%0,0,%3		# atomic64_" #op "_return_relaxed\n"	\
-	#asm_op " %0,%2,%0\n"						\
-"	stdcx.	%0,0,%3\n"						\
-"	bne-	1b\n"							\
-	: "=&r" (t), "+m" (v->counter)					\
-	: "r" (a), "r" (&v->counter)					\
-	: "cc");							\
-									\
-	return t;							\
-}
-
-#define ATOMIC64_FETCH_OP_RELAXED(op, asm_op)				\
-static inline s64							\
-atomic64_fetch_##op##_relaxed(s64 a, atomic64_t *v)			\
-{									\
-	s64 res, t;							\
-									\
-	__asm__ __volatile__(						\
-"1:	ldarx	%0,0,%4		# atomic64_fetch_" #op "_relaxed\n"	\
-	#asm_op " %1,%3,%0\n"						\
-"	stdcx.	%1,0,%4\n"						\
-"	bne-	1b\n"							\
-	: "=&r" (res), "=&r" (t), "+m" (v->counter)			\
-	: "r" (a), "r" (&v->counter)					\
-	: "cc");							\
-									\
-	return res;							\
-}
-
-#define ATOMIC64_OPS(op, asm_op)					\
-	ATOMIC64_OP(op, asm_op)						\
-	ATOMIC64_OP_RETURN_RELAXED(op, asm_op)				\
-	ATOMIC64_FETCH_OP_RELAXED(op, asm_op)
-
-ATOMIC64_OPS(add, add)
-ATOMIC64_OPS(sub, subf)
-
-#define atomic64_add_return_relaxed atomic64_add_return_relaxed
-#define atomic64_sub_return_relaxed atomic64_sub_return_relaxed
-
-#define atomic64_fetch_add_relaxed atomic64_fetch_add_relaxed
-#define atomic64_fetch_sub_relaxed atomic64_fetch_sub_relaxed
-
-#undef ATOMIC64_OPS
-#define ATOMIC64_OPS(op, asm_op)					\
-	ATOMIC64_OP(op, asm_op)						\
-	ATOMIC64_FETCH_OP_RELAXED(op, asm_op)
-
-ATOMIC64_OPS(and, and)
-ATOMIC64_OPS(or, or)
-ATOMIC64_OPS(xor, xor)
-
-#define atomic64_fetch_and_relaxed atomic64_fetch_and_relaxed
-#define atomic64_fetch_or_relaxed  atomic64_fetch_or_relaxed
-#define atomic64_fetch_xor_relaxed atomic64_fetch_xor_relaxed
-
-#undef ATOPIC64_OPS
-#undef ATOMIC64_FETCH_OP_RELAXED
-#undef ATOMIC64_OP_RETURN_RELAXED
-#undef ATOMIC64_OP
-
-static __inline__ void atomic64_inc(atomic64_t *v)
-{
-	s64 t;
-
-	__asm__ __volatile__(
-"1:	ldarx	%0,0,%2		# atomic64_inc\n\
-	addic	%0,%0,1\n\
-	stdcx.	%0,0,%2 \n\
-	bne-	1b"
-	: "=&r" (t), "+m" (v->counter)
-	: "r" (&v->counter)
-	: "cc", "xer");
-}
-#define atomic64_inc atomic64_inc
-
-static __inline__ s64 atomic64_inc_return_relaxed(atomic64_t *v)
-{
-	s64 t;
-
-	__asm__ __volatile__(
-"1:	ldarx	%0,0,%2		# atomic64_inc_return_relaxed\n"
-"	addic	%0,%0,1\n"
-"	stdcx.	%0,0,%2\n"
-"	bne-	1b"
-	: "=&r" (t), "+m" (v->counter)
-	: "r" (&v->counter)
-	: "cc", "xer");
-
-	return t;
-}
-
-static __inline__ void atomic64_dec(atomic64_t *v)
-{
-	s64 t;
-
-	__asm__ __volatile__(
-"1:	ldarx	%0,0,%2		# atomic64_dec\n\
-	addic	%0,%0,-1\n\
-	stdcx.	%0,0,%2\n\
-	bne-	1b"
-	: "=&r" (t), "+m" (v->counter)
-	: "r" (&v->counter)
-	: "cc", "xer");
-}
-#define atomic64_dec atomic64_dec
-
-static __inline__ s64 atomic64_dec_return_relaxed(atomic64_t *v)
-{
-	s64 t;
-
-	__asm__ __volatile__(
-"1:	ldarx	%0,0,%2		# atomic64_dec_return_relaxed\n"
-"	addic	%0,%0,-1\n"
-"	stdcx.	%0,0,%2\n"
-"	bne-	1b"
-	: "=&r" (t), "+m" (v->counter)
-	: "r" (&v->counter)
-	: "cc", "xer");
-
-	return t;
-}
-
-#define atomic64_inc_return_relaxed atomic64_inc_return_relaxed
-#define atomic64_dec_return_relaxed atomic64_dec_return_relaxed
+#define atomic_dec_if_positive_relaxed atomic_dec_if_positive_relaxed
 
+#ifdef CONFIG_64BIT
 /*
  * Atomically test *v and decrement if it is greater than 0.
  * The function returns the old value of *v minus 1.
  */
-static __inline__ s64 atomic64_dec_if_positive(atomic64_t *v)
+static inline s64 atomic64_dec_if_positive_relaxed(atomic64_t *v)
 {
 	s64 t;
 
-	__asm__ __volatile__(
+	asm volatile(
 	PPC_ATOMIC_ENTRY_BARRIER
 "1:	ldarx	%0,0,%1		# atomic64_dec_if_positive\n\
 	addic.	%0,%0,-1\n\
@@ -497,80 +368,8 @@  static __inline__ s64 atomic64_dec_if_positive(atomic64_t *v)
 
 	return t;
 }
-#define atomic64_dec_if_positive atomic64_dec_if_positive
-
-#define atomic64_cmpxchg(v, o, n) (cmpxchg(&((v)->counter), (o), (n)))
-#define atomic64_cmpxchg_relaxed(v, o, n) \
-	cmpxchg_relaxed(&((v)->counter), (o), (n))
-#define atomic64_cmpxchg_acquire(v, o, n) \
-	cmpxchg_acquire(&((v)->counter), (o), (n))
-
-#define atomic64_xchg(v, new) (xchg(&((v)->counter), new))
-#define atomic64_xchg_relaxed(v, new) xchg_relaxed(&((v)->counter), (new))
-
-/**
- * atomic64_fetch_add_unless - add unless the number is a given value
- * @v: pointer of type atomic64_t
- * @a: the amount to add to v...
- * @u: ...unless v is equal to u.
- *
- * Atomically adds @a to @v, so long as it was not @u.
- * Returns the old value of @v.
- */
-static __inline__ s64 atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u)
-{
-	s64 t;
-
-	__asm__ __volatile__ (
-	PPC_ATOMIC_ENTRY_BARRIER
-"1:	ldarx	%0,0,%1		# atomic64_fetch_add_unless\n\
-	cmpd	0,%0,%3 \n\
-	beq	2f \n\
-	add	%0,%2,%0 \n"
-"	stdcx.	%0,0,%1 \n\
-	bne-	1b \n"
-	PPC_ATOMIC_EXIT_BARRIER
-"	subf	%0,%2,%0 \n\
-2:"
-	: "=&r" (t)
-	: "r" (&v->counter), "r" (a), "r" (u)
-	: "cc", "memory");
-
-	return t;
-}
-#define atomic64_fetch_add_unless atomic64_fetch_add_unless
-
-/**
- * atomic_inc64_not_zero - increment unless the number is zero
- * @v: pointer of type atomic64_t
- *
- * Atomically increments @v by 1, so long as @v is non-zero.
- * Returns non-zero if @v was non-zero, and zero otherwise.
- */
-static __inline__ int atomic64_inc_not_zero(atomic64_t *v)
-{
-	s64 t1, t2;
-
-	__asm__ __volatile__ (
-	PPC_ATOMIC_ENTRY_BARRIER
-"1:	ldarx	%0,0,%2		# atomic64_inc_not_zero\n\
-	cmpdi	0,%0,0\n\
-	beq-	2f\n\
-	addic	%1,%0,1\n\
-	stdcx.	%1,0,%2\n\
-	bne-	1b\n"
-	PPC_ATOMIC_EXIT_BARRIER
-	"\n\
-2:"
-	: "=&r" (t1), "=&r" (t2)
-	: "r" (&v->counter)
-	: "cc", "xer", "memory");
-
-	return t1 != 0;
-}
-#define atomic64_inc_not_zero(v) atomic64_inc_not_zero((v))
-
-#endif /* __powerpc64__ */
+#define atomic64_dec_if_positive_relaxed atomic64_dec_if_positive_relaxed
+#endif
 
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_ATOMIC_H_ */
diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h
index cf091c4c22e5..181f7e8b3281 100644
--- a/arch/powerpc/include/asm/cmpxchg.h
+++ b/arch/powerpc/include/asm/cmpxchg.h
@@ -192,7 +192,7 @@  __xchg_relaxed(void *ptr, unsigned long x, unsigned int size)
      		(unsigned long)_x_, sizeof(*(ptr))); 			     \
   })
 
-#define xchg_relaxed(ptr, x)						\
+#define arch_xchg_relaxed(ptr, x)					\
 ({									\
 	__typeof__(*(ptr)) _x_ = (x);					\
 	(__typeof__(*(ptr))) __xchg_relaxed((ptr),			\
@@ -448,35 +448,7 @@  __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new,
 	return old;
 }
 
-static __always_inline unsigned long
-__cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new,
-		  unsigned int size)
-{
-	switch (size) {
-	case 1:
-		return __cmpxchg_u8_acquire(ptr, old, new);
-	case 2:
-		return __cmpxchg_u16_acquire(ptr, old, new);
-	case 4:
-		return __cmpxchg_u32_acquire(ptr, old, new);
-#ifdef CONFIG_PPC64
-	case 8:
-		return __cmpxchg_u64_acquire(ptr, old, new);
-#endif
-	}
-	BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg_acquire");
-	return old;
-}
-#define cmpxchg(ptr, o, n)						 \
-  ({									 \
-     __typeof__(*(ptr)) _o_ = (o);					 \
-     __typeof__(*(ptr)) _n_ = (n);					 \
-     (__typeof__(*(ptr))) __cmpxchg((ptr), (unsigned long)_o_,		 \
-				    (unsigned long)_n_, sizeof(*(ptr))); \
-  })
-
-
-#define cmpxchg_local(ptr, o, n)					 \
+#define arch_cmpxchg_local(ptr, o, n)					 \
   ({									 \
      __typeof__(*(ptr)) _o_ = (o);					 \
      __typeof__(*(ptr)) _n_ = (n);					 \
@@ -484,7 +456,7 @@  __cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new,
 				    (unsigned long)_n_, sizeof(*(ptr))); \
   })
 
-#define cmpxchg_relaxed(ptr, o, n)					\
+#define arch_cmpxchg_relaxed(ptr, o, n)					\
 ({									\
 	__typeof__(*(ptr)) _o_ = (o);					\
 	__typeof__(*(ptr)) _n_ = (n);					\
@@ -493,38 +465,20 @@  __cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new,
 			sizeof(*(ptr)));				\
 })
 
-#define cmpxchg_acquire(ptr, o, n)					\
-({									\
-	__typeof__(*(ptr)) _o_ = (o);					\
-	__typeof__(*(ptr)) _n_ = (n);					\
-	(__typeof__(*(ptr))) __cmpxchg_acquire((ptr),			\
-			(unsigned long)_o_, (unsigned long)_n_,		\
-			sizeof(*(ptr)));				\
-})
 #ifdef CONFIG_PPC64
-#define cmpxchg64(ptr, o, n)						\
-  ({									\
-	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
-	cmpxchg((ptr), (o), (n));					\
-  })
-#define cmpxchg64_local(ptr, o, n)					\
+#define arch_cmpxchg64_local(ptr, o, n)					\
   ({									\
 	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
-	cmpxchg_local((ptr), (o), (n));					\
+	arch_cmpxchg_local((ptr), (o), (n));				\
   })
-#define cmpxchg64_relaxed(ptr, o, n)					\
-({									\
-	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
-	cmpxchg_relaxed((ptr), (o), (n));				\
-})
-#define cmpxchg64_acquire(ptr, o, n)					\
+#define arch_cmpxchg64_relaxed(ptr, o, n)				\
 ({									\
 	BUILD_BUG_ON(sizeof(*(ptr)) != 8);				\
-	cmpxchg_acquire((ptr), (o), (n));				\
+	arch_cmpxchg_relaxed((ptr), (o), (n));				\
 })
 #else
 #include <asm-generic/cmpxchg-local.h>
-#define cmpxchg64_local(ptr, o, n) __cmpxchg64_local_generic((ptr), (o), (n))
+#define arch_cmpxchg64_local(ptr, o, n) __cmpxchg64_local_generic((ptr), (o), (n))
 #endif
 
 #endif /* __KERNEL__ */