svn commit: r278431 - head/sys/contrib/vchiq/interface/vchiq_arm

[Konstantin Belousov]

On Mon, Feb 09, 2015 at 05:00:49PM +1100, Bruce Evans wrote:
> 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.

On x86 unaligned access is non-atomic.  This was very visible on
Core2 CPUs where DPCPU code mishandled the alignment, resulting in
the mutexes from the per-cpu areas breaking badly.

Modern CPUs should not lock several cache lines simultaneously either.