On Sun, Apr 10, 2022 at 6:27 AM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Sat, Apr 9, 2022 at 10:51 AM Bartosz Golaszewski <brgl@xxxxxxxx> wrote: > > > > Here's a single fix for a race condition between the GPIO core and consumers of > > GPIO IRQ chips. > > I've pulled this, but it's horribly broken. > > You can't just use a compiler barrier to make sure the compiler orders > the data at initialization time. > > That doesn't take care of CPU re-ordering, but it also doesn't take > care of re-ordering reads on the other side of the equation. > > Every write barrier needs to pair with a read barrier. > > And "barrier()" is only a barrier on that CPU, since it is only a > barrier for code generation, not for data. > > There are multiple ways to do proper hand-off of data, but the best > one is likely > > - on the initialization side, do > > .. initialize all the data, then do .. > smp_store_release(&initialized, 1); > > - on the reading side, do > > if (!smp_load_acquire(&initialized)) > return -EAGAIN; > > .. you can now rely on all the data having been initialized .. > > But honestly, the fact that you got this race condition so wrong makes > me suggest you use proper locks. Because the above gives you proper > ordering between the two sequences, but the sequences in question > still have to have a *lot* of guarantees about the accesses actually > then being valid in a lock-free environment (the only obviously safe > situation is a "initialize things once, everything afterwards is only > a read" - otherwise y ou need to make sure all the *updates* are > safely done too). > > With locking, all these issues go away. The lock will take care of > ordering, but also data consistency at updates. > > Without locking, you need to do the above kinds of careful things for > _all_ the accesses that can race, not just that "initialized" flag. > > Linus Cc'ing Shreeya Thanks, we'll see about a follow-up with a better solution. Bart