lkml.org 
[lkml]   [2020]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2020-07-27 14:05    [W:0.101 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site