Re: Question: Why does the modification of seq in write_seqlock not require WRITE_ONCE?

[Akira Yokosawa <akiyks@xxxxxxxxx>]

Hi Alan,

On Tue, 25 Apr 2023 21:52:48 +0800, Alan Huang wrote:
> Hi,
> 
> I noticed that the modifications of seq in write_seqlock and write_sequnlock of Listing 9.10 use plain ++ operation.
> But as Chapter 4 (especially 4.3.4.4) says, there will be store tearing since there are concurrent readers.
> 
> However, the kernel implementation of sequence lock also uses plain ++ operation.
> 
> I’m somewhat confused.
> 
> Why does the modification of seq in write_seqlock not require WRITE_ONCE?

Good question...

I don't have a straight answer, but the history of include/linux/seqlock.h
says those plain increments predate the introduction of ACCESS_ONCE
(predecessor to READ_ONCE/WRITE_ONCE) to the Linux kernel.

The code at the time (pre v2.6.0) looked like this:

static inline void write_seqlock(seqlock_t *sl)
{
	spin_lock(&sl->lock);
	++sl->sequence;
	smp_wmb();			
}	

static inline void write_sequnlock(seqlock_t *sl) 
{
	smp_wmb();
	sl->sequence++;
	spin_unlock(&sl->lock);
}

static inline int write_tryseqlock(seqlock_t *sl)
{
	int ret = spin_trylock(&sl->lock);

	if (ret) {
		++sl->sequence;
		smp_wmb();			
	}
	return ret;
}

The purpose of WRITE_ONCE() would be to suppress compiler optimization
of write accesses.  In the code above, smp_wmb(), spin_lock(),
spin_unlock(), and spin_trylock() all imply compiler barriers.
So, there is not much room for compilers to optimize the store part
of increments.

I think those plain-looking increments are safe as far as current
compilers are concerned.

Does this explanation help you?

Paul, please chime in if I'm missing something.

        Thanks, Akira

> 
> Thanks,
> Alan