netdev - RE: [PATCH 9/24] make atomic_read() behave consistently on ia64

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: <617E1C2C70743745A92448908E030B2A02213B3C@scsmsx411.amr.corp.intel.com>
Date:	Thu, 9 Aug 2007 14:03:33 -0700
From:	"Luck, Tony" <tony.luck@...el.com>
To:	"Chris Snook" <csnook@...hat.com>, <linux-kernel@...r.kernel.org>,
	<linux-arch@...r.kernel.org>, <torvalds@...ux-foundation.org>
Cc:	<netdev@...r.kernel.org>, <akpm@...ux-foundation.org>,
	<ak@...e.de>, <heiko.carstens@...ibm.com>, <davem@...emloft.net>,
	<schwidefsky@...ibm.com>, <wensong@...ux-vs.org>,
	<horms@...ge.net.au>, <wjiang@...ilience.com>,
	<cfriesen@...tel.com>, <zlynx@....org>, <rpjday@...dspring.com>,
	<jesper.juhl@...il.com>
Subject: RE: [PATCH 9/24] make atomic_read() behave consistently on ia64

> +#define atomic_read(v)		(*(volatile __s32 *)&(v)->counter)
> +#define atomic64_read(v)	(*(volatile __s64 *)&(v)->counter)
> 
>  #define atomic_set(v,i)		(((v)->counter) = (i))
>  #define atomic64_set(v,i)	(((v)->counter) = (i))

Losing the volatile from the "set" variants definitely changes
the code generated.  Before the patch gcc would give us:

	st4.rel [r37]=r9

after
	st4 [r37]=r9

It is unclear whether anyone relies on (or even whether they should
rely on) the release semantics that are provided by the current
version of atomic.h.  But making this change would require an
audit of all the uses of atomic_set() to find an answer.

There is a more worrying difference in the generated code (this
from the ancient and venerable gcc 3.4.6 that is on my build
machine).   In rwsem_down_failed_common I see this change (after
disassembling vmlinux, I used sed to zap the low 32-bits of addresses
to make the diff manageable ... that's why the addresses all end
in xxxxxxxx):

712868,712873c712913,712921
< a0000001xxxxxxxx:                 adds r16=-1,r30
< a0000001xxxxxxxx:     [MII]       ld8.acq r33=[r32]
< a0000001xxxxxxxx:                 nop.i 0x0;;
< a0000001xxxxxxxx:                 add r42=r33,r16
< a0000001xxxxxxxx:     [MMI]       mov.m ar.ccv=r33;;
< a0000001xxxxxxxx:                 cmpxchg8.acq r34=[r32],r42,ar.ccv
---
> a0000001xxxxxxxx:                 adds r16=-1,r31
> a0000001xxxxxxxx:     [MMI]       ld4.acq r14=[r32];;
> a0000001xxxxxxxx:                 nop.m 0x0
> a0000001xxxxxxxx:                 sxt4 r34=r14
> a0000001xxxxxxxx:     [MMI]       nop.m 0x0;;
> a0000001xxxxxxxx:                 nop.m 0x0
> a0000001xxxxxxxx:                 add r15=r34,r16
> a0000001xxxxxxxx:     [MMI]       mov.m ar.ccv=r34;;
> a0000001xxxxxxxx:                 cmpxchg8.acq r42=[r32],r15,ar.ccv

This code is probably from the rwsem_atomic_update(adjustment, sem) macro
which is cpp'd to atomic64_add_return().  It looks really bad that the new
code reads a 32-bit value and sign extends it rather than reading a 64-bit
value (but I'm perplexed as to why this patch provoked this change in the
generated code).

-Tony
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html