> Date: Thu, 3 Oct 2024 10:13:17 +0200 > From: "J. Hannken-Illjes" <[email protected]> > > > On 1. Oct 2024, at 18:57, Taylor R Campbell > > <[email protected]> wrote: > > > > I think the answer is no, nothing here precludes a concurrent > > vcache_reclaim from writing to fp->f_vnode->v_mount at the same time > > do_sys_fstatvfs is reading from it. Although both the before and > > after states are OK for dostatvfs, if this concurrent read/write is > > supposed to be allowed, it should be done with atomic_load/store_* -- > > and the same needs to apply to all other use of v_mount that isn't, > > e.g., serialized by the vnode lock. > > Always thought while writing a pointer to aligned storage a > concurrent reader will get either the old or the new value but > never gets garbage. Is this assumption wrong?
We have long made that assumption, and on all the architectures we support that I'm aware of, it is true of a load or store instruction executed by the CPU. But the compiler might play other shenanigans, if you don't use atomic_load/store_*. It might, in principle, fuse or tear loads or stores (mainly relevant for larger-size or misaligned load/stores). It might, in practice, issue multiple load or store instructions on the same location, and assume they produce the same results -- e.g., under register pressure, it might just load the same location twice instead of spilling a register to the stack. And sanitizers like tsan are apt to interpret this as a data race, leading to false alarms even if it is otherwise harmless. And, for the reader's sake in understanding the code, it is clearer to use atomic_load/store_*. So we should always use atomic_load/store_* if the intent is that the location can be concurrently written to while we're reading from it.
