On Fri, Mar 14, 2025 at 2:08 PM Vlastimil Babka <vbabka@xxxxxxx> wrote: > > On 3/14/25 22:05, Alexei Starovoitov wrote: > > On Wed, Mar 12, 2025 at 1:29 AM Vlastimil Babka <vbabka@xxxxxxx> wrote: > > > > That's correct. > > > >> An if we e.g. have a pointer to memcg_stock_pcp through which we access the > >> stock_lock an the other (protected) fields and that pointer doesn't change > >> between that, I imagine gcc can reliably determine these can't alias? > > > > Though my last gcc commit was very long ago here is a simple example > > where compiler can reorder/combine stores: > > struct s { > > short a, b; > > } *p; > > p->a = 1; > > p->b = 2; > > The compiler can keep them as-is, combine or reorder even with > > -fno-strict-aliasing, because it can determine that a and b don't alias. > > > > But after re-reading gcc doc on volatiles again it's clear that > > extra barriers are not necessary. > > The main part: > > "The minimum requirement is that at a sequence point all previous > > accesses to volatile objects have stabilized" > > > > So anything after WRITE_ONCE(lt->acquired, 1); will not be hoisted up > > and that's what we care about here. > > OK, is there similar guarantee for the unlock side? No write will be moved > after WRITE_ONCE(lt->acquired, 0); there? Yes, because the first line: lt = this_cpu_ptr(lock); WRITE_ONCE(lt->acquired, 0); this_cpu_ptr is pretty much a black box for the compiler. 'lt' can alias with anything before it.