linux-kernel - Re: linux-next: build warnings after merge of the access

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]

Message-ID: <20150326163647.GC21418@twins.programming.kicks-ass.net>
Date:	Thu, 26 Mar 2015 17:36:47 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Will Deacon <will.deacon@....com>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	Christian Borntraeger <borntraeger@...ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	"linux-next@...r.kernel.org" <linux-next@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Davidlohr Bueso <dave@...olabs.net>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>
Subject: Re: linux-next: build warnings after merge of the access_once tree

On Thu, Mar 26, 2015 at 09:21:50AM -0700, Linus Torvalds wrote:
> So the proper patch looks something like this:
> 
>     diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>     index 1b45e4a0519b..f36e1abf56ea 100644
>     --- a/include/linux/compiler.h
>     +++ b/include/linux/compiler.h
>     @@ -198,10 +198,6 @@ __compiletime_warning("data access exceeds
> word size and won't be atomic")
>      #endif
>      ;

You also want to get rid of that ^^^ declaration which includes the
compiletime warning.

>     -static __always_inline void data_access_exceeds_word_size(void)
>     -{
>     -}
>     -
>      static __always_inline void __read_once_size(const volatile void
> *p, void *res, int size)
>      {
>              switch (size) {
>     @@ -214,7 +210,6 @@ static __always_inline void
> __read_once_size(const volatile void *p, void *res,
>              default:
>                      barrier();
>                      __builtin_memcpy((void *)res, (const void *)p, size);
>     -                data_access_exceeds_word_size();
>                      barrier();
>              }
>      }
>     @@ -231,7 +226,6 @@ static __always_inline void
> __write_once_size(volatile void *p, void *res, int s
>              default:
>                      barrier();
>                      __builtin_memcpy((void *)p, (const void *)res, size);
>     -                data_access_exceeds_word_size();
>                      barrier();
>              }
>      }

So this still has the potential to generate significantly worse code
than the previous ACCESS_ONCE() because of the dual barrier() and god
only knows how memcpy gets implemented.

As it turns out, the ARM version of __builtin_memcpy() does result in a
single double word load because its all smart and such, but a 'trivial'
implementation might just end up doing 8 byte copies.

Can't we make an argument that these barrier calls are not required? The
memcpy() call already guarantees we emit the loads and its opaque so the
compiler cannot 'cache' the value. So I see not immediate reason for the
dual memory clobber.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/