Hi Peter,
[...]
> > >
> > > How about something like:
> > >
> > > #ifdef CONFIG_64BIT
> > >
> > > #define DEFINE_U64_U32(name) u64 name
> > > #define u64_u32_load(name) name
> > > #define u64_u32_store(name, val)name = val
> > >
> > > #else
> > >
> > > #define DEFINE_U64_U32(name) \
> > > struct { \
> > > u64 name; \
> > > u64 name##_copy; \
> > > }
> > >
> > > #define u64_u32_load(name) \
> > > ({ \
> > > u64 val, copy; \
> > > do { \
> > > val = name; \
> > > smb_rmb(); \
> > > copy = name##_copy; \
> > > } while (val != copy); \
> >
> > wrong order there; we should first read _copy and then the regular one
> > of course.
> >
> > > val;
> > > })
> > >
> > > #define u64_u32_store(name, val) \
> > > do { \
> > > typeof(val) __val = (val); \
> > > name = __val; \
> > > smp_wmb(); \
> > > name##_copy = __val; \
> > > } while (0)
> > >
> > > #endif
> >
> > The other approach is making it a full type and inline functions I
> > suppose.
>
> I didn't pick this approach originally, as I thought it would be way too
> invasive. If the API looks way cleaner, it nonetheless doesn't match
> last_update_time usage. The variable is declared in sched_avg while its copy
> is in struct cfs_rq.
>
> Moving the copy in sched_avg would mean:
>
> * Setting the copy for all struct sched_avg in ___update_load_sum(), instead
> of just the cfs_rq.avg in update_cfs_rq_load_avg().
>
> * Having the DEFINE_U64_U32 declaration in include/linux/sched.h to cover
> struct sched_avg.
Gentle ping
