linux-kernel - Re: [tip:locking/core] locking/atomics: Simplify the op definitions in atomic.h some more

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 15 May 2018 10:35:56 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
        will.deacon@....com, mark.rutland@....com,
        torvalds@...ux-foundation.org, paulmck@...ux.vnet.ibm.com,
        tglx@...utronix.de, hpa@...or.com,
        linux-tip-commits@...r.kernel.org
Subject: Re: [tip:locking/core] locking/atomics: Simplify the op definitions
 in atomic.h some more


* Peter Zijlstra <peterz@...radead.org> wrote:

> And if we're going to do codegen, we might as well all generate this
> anyway, so all this mucking about is a complete waste of time.

I'm not yet convinced that it will be cleaner, but can be convinced in principle, 
but meanwhile the existing code is arguably butt-ugly and bloaty.

Regarding these cleanups, we had this before:

 /* atomic_add_return_relaxed */
 #ifndef atomic_add_return_relaxed
 #define  atomic_add_return_relaxed	atomic_add_return
 #define  atomic_add_return_acquire	atomic_add_return
 #define  atomic_add_return_release	atomic_add_return

 #else /* atomic_add_return_relaxed */

 #ifndef atomic_add_return_acquire
 #define  atomic_add_return_acquire(...)					\
	__atomic_op_acquire(atomic_add_return, __VA_ARGS__)
 #endif

 #ifndef atomic_add_return_release
 #define  atomic_add_return_release(...)					\
	__atomic_op_release(atomic_add_return, __VA_ARGS__)
 #endif

 #ifndef atomic_add_return
 #define  atomic_add_return(...)						\
	__atomic_op_fence(atomic_add_return, __VA_ARGS__)
 #endif
 #endif /* atomic_add_return_relaxed */

Which is 23 lines per definition.

Now we have this much more compact definition:

 #ifndef atomic_add_return_relaxed
 # define atomic_add_return_relaxed		atomic_add_return
 # define atomic_add_return_acquire		atomic_add_return
 # define atomic_add_return_release		atomic_add_return
 #else
 # ifndef atomic_add_return
 #  define atomic_add_return(...)		__op_fence(atomic_add_return, __VA_ARGS__)
 #  define atomic_add_return_acquire(...)	__op_acquire(atomic_add_return, __VA_ARGS__)
 #  define atomic_add_return_release(...)	__op_release(atomic_add_return, __VA_ARGS__)
 # endif
 #endif

Which is just _half_ the linecount.

Automated code generation might improve this some more, but the net effect on the 
core <linux/atomic.h> code right now is 373 lines removed:

  include/linux/atomic.h | 1109 ++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------
  1 file changed, 368 insertions(+), 741 deletions(-)

... <linux/atomic.h> shrunk to just 709 lines.

The x86/include/asm/atomic64_64.h file got smaller as well due to the cleanups:

  arch/x86/include/asm/atomic64_64.h | 216 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------
  1 file changed, 97 insertions(+), 119 deletions(-)

So unless you can clean this up and shrink this even more, these changes are 
obviously justified on their own.

Thanks,

	Ingo