Messages in this thread | | | Date | Mon, 27 Jul 2020 13:05:21 +0100 | From | Vincent Donnefort <> | Subject | Re: [PATCH] sched/fair: provide u64 read for 32-bits arch helper |
| |
Hi,
On Mon, Jul 27, 2020 at 01:24:54PM +0200, Ingo Molnar wrote: > > * vincent.donnefort@arm.com <vincent.donnefort@arm.com> wrote: > > > From: Vincent Donnefort <vincent.donnefort@arm.com> > > > > Introducing two macro helpers u64_32read() and u64_32read_set_copy() to > > factorize the u64 vminruntime and last_update_time read on a 32-bits > > architecture. Those new helpers encapsulate smp_rmb() and smp_wmb() > > synchronization and therefore, have a small penalty in set_task_rq_fair() > > and init_cfs_rq(). > > > > The choice of using a macro over an inline function is driven by the > > conditional u64 variable copy declarations. > > Could you please explain how "conditional u64 variable copy > declarations" prevents us to use an inline function for this
I meant, as the "copy" variable [vminruntime|last_update_time]_copy, is declared only in the !CONFIG_64BIT case, a function call would fail when CONFIG_64BIT is set.
u64_32read(cfs_rq->min_vruntime, cfs_rq->min_vruntime_copy); ^ not declared
I could rephrase this part to something more understandable ?
> > > +/* > > + * u64_32read() / u64_32read_set_copy() > > + * > > + * Use the copied u64 value to protect against data race. This is only > > + * applicable for 32-bits architectures. > > + */ > > +#if !defined(CONFIG_64BIT) && defined(CONFIG_SMP) > > +# define u64_32read(val, copy) \ > > +({ \ > > + u64 _val; \ > > + u64 _val_copy; \ > > + \ > > + do { \ > > + _val_copy = copy; \ > > + /* \ > > + * paired with u64_32read_set_copy, ordering access \ > > + * to val and copy. \ > > + */ \ > > + smp_rmb(); \ > > + _val = val; \ > > + } while (_val != _val_copy); \ > > + \ > > + _val; \ > > +}) > > +# define u64_32read_set_copy(val, copy) \ > > +do { \ > > + /* paired with u64_32read, ordering access to val and copy */ \ > > + smp_wmb(); \ > > + copy = val; \ > > +} while (0) > > +#else > > +# define u64_32read(val, copy) (val) > > +# define u64_32read_set_copy(val, copy) do { } while (0) > > +#endif > > + > > Thanks, > > Ingo
-- Vincent
| |