Re: sequence lock in Linux

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Fri, Jun 11, 2010 at 02:06:01PM -0700, H. Peter Anvin wrote:
> On 06/11/2010 01:36 PM, Paul E. McKenney wrote:
> > 
> > The reason that the C standard permits this is to allow for things like
> > 8-bit CPUs, which are simply unable to load or store 32-bit quantities
> > except by doing it chunkwise.  But I don't expect the Linux kernel to
> > boot on these, and certainly not on any of the ones that I have used!
> > 
> > I most definitely remember seeing a gcc guarantee that loads and stores
> > would be done in one instruction whenever the hardware supported this,
> > but I am not finding it today.  :-(
> 
> What gcc does not -- and should not -- guarantee is that accessing a
> non-volatile member is done exactly once.  As Mathieu pointed out, it
> can choose to drop it due to register pressure and load it again.
> 
> What is possibly a much bigger risk -- since this is an inline -- is
> that the value is cached from a previous piece of code, *or* that since
> the structure is const(!) that the second read in the repeat loop is
> elided.  Presumably current versions of gcc don't do that across a
> memory clobber, but that doesn't seem entirely out of the question.

Memory barriers in the sequence-lock code prevent this, assuming, as
you point out, that memory clobber works (but if it doesn't, it should
be fixed):

o	write_seqlock() and write_tryseqlock() each have an smp_wmb()
	following the increment.  Ditto for write_seqcount_begin().

o	write_sequnlock() has an smp_wmb() preceding the increment,
	and ditto for write_seqcount_end().  There are thus two smp_wmb()
	calls between the increments in the usual code sequence:

		write_seqlock(&l);
		do_something();
		write_sequnlock();

o	read_seqbegin() has an smp_rmb() following its read from
	->sequence.  Ditto for read_seqcount_begin().

o	read_seqretry() has an smp_rmb() preceding its read from
	->sequence, and ditto for read_seqcount_retry().  There are thus
	two smp_wmb() calls between the reads in the usual code sequence:

		do {
			s = read_seqbegin(&l);
			read_something();
		} while read_seqretry(&l, s);

So sequence locks should be pretty safe, at least as far as this
vulnerability is concerned. ;-)

							Thanx, Paul