On Mon, 9 Feb 2015, Oleksandr Tymoshenko wrote:

Log:
 Do not mark shared structures as __packed, it leads to race condition

 If structure packed as __packed clang (and probably gcc) generates
 code that loads word fields (e.g. tx_pos)  byte-by-byte and if it's
 modified by VideoCore in the same time as ARM loads the value result
 is going to be mixed combination of bytes from previous value and
 new one.

Most uses of __packed are bugs.  It gives pessimizations as well as
non-atomic accesses for sub-object accesses.

I think the full bugs only occur when arch has strict alignment
requirements and the alignment of the __packed objects is not known.
This means that only lesser bugs occur on x86 (unless you enable
alignment checking, but this arguably breaks the ABI).  The compiler
just generates possibly-misaligned full-width accesses if the arch
doesn't have strict alignment requirements.  Often the acceses turn
out to be aligned at runtime.  Otherwise, the hardware does them
atomically, with a smaller efficiency penalty than split accesses.

Many packed structs should also be declared as __aligned(N).  This is
rarely done.  Old networking code in <netinet> uses __packed just once,
and this also uses __aligned(4) (for struct ip)  Newer networking code
in <netinet> (for ipv6) is massively pessimized, with __packed used
extensively and __aligned(N) never used with __packed.  sctp is worse.
It uses its own macro that defeats grepping for __packed.  It has 82
instances of this in <netinet> where ipv6 has only 34.  Newer networking
should need __packed less than old ones, since no one would misdesign a
data struct so that it needs packing now.

gcc documents the -Wpacked flag for finding some cases of bogus packing.
It is rarely used.

Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to