Re: [RFC] seqlock,lockdep: Add lock primitives to read_seqbegin().

Tetsuo Handa
2011-03-30T12:17:51Z

Peter Zijlstra wrote:
> > Also, I assume you meant to call
> > spin_acquire() before entering the spin state (as with
> >
> > static inline void __raw_spin_lock(raw_spinlock_t *lock)
> > {
> > preempt_disable();
> > spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
> > LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
> > }
> >
> > . Otherwise, lockdep cannot report it when hit this bug upon the first call to
> > this function).
>
> Huh no, of course not, a seqlock read side cannot contend in the classic
> sense.

I couldn't understand what 'contend' means. I think

static __always_inline unsigned read_seqbegin(const seqlock_t *sl)
{
unsigned ret;
repeat:
ret = sl->sequence;
smp_rmb();
if (unlikely(ret & 1)) {
cpu_relax();
goto repeat;
}
return ret;
}

is equivalent (except that above one will not write to any kernel memory) to

static __always_inline unsigned read_seqbegin(seqlock_t *sl)
{
unsigned ret;
unsigned long flags;
spin_lock_irqsave(&sl->lock, flags);
ret = sl->sequence;
spin_unlock_irqrestore(&sl->lock, flags);
return ret;
}

because read_seqbegin() cannot return to the reader until the writer (if there
is one) calls write_sequnlock().

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);
}

Don't we call this situation (a reader thread temporarily behaves like a writer
thread who writes nothing) as 'contended'?

Anyway, could you show me read_seqbegin2()/read_seqretry2() for testing with
locktest module?

Regards.