Messages in this thread | | | Date | Fri, 29 Nov 2019 08:04:10 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH] spinlock_debug: Fix various data races |
| |
* Marco Elver <elver@google.com> wrote:
> On Thu, 21 Nov 2019, Ingo Molnar wrote: > > > > > * Marco Elver <elver@google.com> wrote: > > > > > static inline void debug_spin_lock_after(raw_spinlock_t *lock) > > > { > > > - lock->owner_cpu = raw_smp_processor_id(); > > > - lock->owner = current; > > > + WRITE_ONCE(lock->owner_cpu, raw_smp_processor_id()); > > > + WRITE_ONCE(lock->owner, current); > > > } > > > > debug_spin_lock_after() runs inside the spinlock itself - why do these > > writes have to be WRITE_ONCE()? > > > > > @@ -197,8 +197,8 @@ static inline void debug_write_unlock(rwlock_t *lock) > > > RWLOCK_BUG_ON(lock->owner != current, lock, "wrong owner"); > > > RWLOCK_BUG_ON(lock->owner_cpu != raw_smp_processor_id(), > > > lock, "wrong CPU"); > > > - lock->owner = SPINLOCK_OWNER_INIT; > > > - lock->owner_cpu = -1; > > > + WRITE_ONCE(lock->owner, SPINLOCK_OWNER_INIT); > > > + WRITE_ONCE(lock->owner_cpu, -1); > > > } > > > > This too is running inside the critical section of the spinlock - why are > > the WRITE_ONCE() calls necessary? > > Although the writes are inside the critical section, they are read > concurrently outside the critical section, e.g. in > debug_spin_lock_before(). In other words, the WRITE_ONCE pair with the > READ_ONCE that are *outside* the critical section.
Fair enough!
Thanks,
Ingo
| |