Messages in this thread |  | | Date | Tue, 18 Jan 2022 07:44:54 +0800 | | From | Tao Zhou <> | | Subject | Re: [PATCH v2 1/7] sched/fair: Provide u64 read for 32-bits arch helper |
| |
On Mon, Jan 17, 2022 at 07:42:04PM +0000, Vincent Donnefort wrote: > [...] > > > > +/* > > > + * u64_u32_load/u64_u32_store > > > + * > > > + * Use a copy of a u64 value to protect against data race. This is only > > > + * applicable for 32-bits architectures. > > > + */ > > > +#ifdef CONFIG_64BIT > > > +# define u64_u32_load_copy(var, copy) var > > > +# define u64_u32_store_copy(var, copy, val) (var = val) > > > +#else > > > +# define u64_u32_load_copy(var, copy) \ > > > +({ \ > > > + u64 __val, __val_copy; \ > > > + do { \ > > > + __val_copy = copy; \ > > > + /* \ > > > + * paired with u64_u32_store, ordering access \ > > > + * to var and copy. \ > > > + */ \ > > > + smp_rmb(); \ > > > + __val = var; \ > > > + } while (__val != __val_copy); \ > > > + __val; \ > > > +}) > > > +# define u64_u32_store_copy(var, copy, val) \ > > > +do { \ > > > + typeof(val) __val = (val); \ > > > + var = __val; \ > > > + /* \ > > > + * paired with u64_u32_load, ordering access to var and \ > > > + * copy. \ > > > + */ \ > > > + smp_wmb(); \ > > > + copy = __val; \ > > > +} while (0) > > > > Code stay there some time from me. Just from my crude review; > > The above macro need a variable to load @var temporarily for > > later store; that means the @copy value is from @var not @val. > > > > # define u64_u32_store_copy(var, copy, val) \ > > do { \ > > typeof(val) __val = (val), __var = (var); \ > > var = __val; \ > > /* \ > > * paired with u64_u32_load, ordering access to var and \ > > * copy. \ > > */ \ > > smp_wmb(); \ > > copy = __var; \ > > } while (0) > > > Hi Tao, > > __var would then contain the previous value of @var, wouldn't it? We need > both @var and @copy to be equal to @val.
Sorry for the noise, I'm wrong here.
> > > > > > > > > Thanks, > > Tao
|  |