Date: Mon, 9 Feb 2015 17:00:49 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Oleksandr Tymoshenko <gonzo@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r278431 - head/sys/contrib/vchiq/interface/vchiq_arm Message-ID: <20150209170045.S1037@besplex.bde.org> In-Reply-To: <201502090231.t192VS6C060751@svn.freebsd.org> References: <201502090231.t192VS6C060751@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150209170045.S1037>