linux-kernel - Re: Linux 3.19-rc3

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, 06 Jan 2015 12:58:36 -0500
From:	Peter Hurley <peter@...leysoftware.com>
To:	paulmck@...ux.vnet.ibm.com
CC:	Peter Zijlstra <peterz@...radead.org>,
	Kent Overstreet <kmo@...erainc.com>,
	Sedat Dilek <sedat.dilek@...il.com>,
	Dave Jones <davej@...emonkey.org.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>, Chris Mason <clm@...com>
Subject: Re: Linux 3.19-rc3

On 01/06/2015 12:38 PM, Paul E. McKenney wrote:
> On Tue, Jan 06, 2015 at 07:55:39AM -0500, Peter Hurley wrote:
>> [ +cc Paul McKenney ]
>>
>> On 01/06/2015 07:20 AM, Peter Zijlstra wrote:
>>> On Tue, Jan 06, 2015 at 04:01:21AM -0800, Kent Overstreet wrote:
>>>> On Tue, Jan 06, 2015 at 12:48:42PM +0100, Peter Zijlstra wrote:
>>>>>
>>>>>
>>>>> Looking at that closure stuff, why is there an smp_mb() in
>>>>> closure_wake_up() ? Typically wakeup only needs to imply a wmb.
>>>>>
>>>>> Also note that __closure_wake_up() starts with a fully serializing
>>>>> instruction (xchg) and thereby already implies the full barrier.
>>>>
>>>> Probably no good reason, that code is pretty old :)
>>>>
>>>> If I was to hazard a guess, I had my own lockless linked lists before llist.h
>>>> existed and perhaps I did it with atomic_xchg() - which was at least documented
>>>> to not imply a barrier. I suppose it should just be dropped.
>>>
>>> We (probably me) should probably audit all the atomic_xchg()
>>> implementations and documentation and fix that. I was very much under
>>> the impression it should imply a full barrier (and it certainly does on
>>> x86), the documentation should state the rule that any atomic_ function
>>> that returns a result is fully serializing, therefore, because
>>> atomic_xchg() has a return value, it should too.
>>
>> memory-barriers.txt and atomic_ops.txt appear to contradict each other here,
>> but I think that's because atomic_ops.txt has drifted toward an
>> arch-implementer's POV:
>>
>> 260:atomic_xchg requires explicit memory barriers around the operation.
>>
>> All the serializing atomic operations have descriptions like this.
> 
> I am not seeing the contradiction.
> 
> You posted the relevant line from atomic_ops.txt.  The relevant passage
> from memory-barriers.txt is as follows:
> 
> 	Any atomic operation that modifies some state in memory and
> 	returns information about the state (old or new) implies an
> 	SMP-conditional general memory barrier (smp_mb()) on each side
> 	of the actual operation (with the exception of explicit lock
> 	operations, described later).  These include:
> 
> 		xchg();
> 		...
> 		atomic_xchg();			atomic_long_xchg();
> 
> So it appears to me that both documents require full barriers before and
> after any atomic exchange operation in SMP builds.  Therefore, any
> SMP-capable architecture that omits these barriers is buggy.

Sure, I understand that, but I think the atomic_ops.txt is easy to
misinterpret.

> So, what am I missing here?

Well, it's a matter of the intended audience. There is a significant
difference between:

static inline int atomic_xchg(atomic_t *v, int new)
{
	/* this arch doesn't have serializing xchg() */
	smp_mb();
	/* arch xchg */
	smp_mb();
}

and

	smp_mb();
	atomic_xchg(&v, 1);
	smp_mb();

but both have "explicit memory barriers around the operation."

Regards,
Peter Hurley
--
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/