Re: [PATCH v2 06/28] kernel: Define gettimeofday vdso common code

Arnd Bergmann
2018-12-11T21:41:35Z

On Tue, Dec 11, 2018 at 2:39 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
> On 29/11/2018 20:42, Arnd Bergmann wrote:
> > On Thu, Nov 29, 2018 at 6:06 PM Vincenzo Frascino
> > <vincenzo.frascino@arm.com> wrote:
> >
> >> +/*
> >> + * The definitions below are required to overcome the limitations
> >> + * of time_t on 32 bit architectures, which overflows in 2038.
> >> + * The new code should use the replacements based on time64_t and
> >> + * timespec64.
> >> + *
> >> + * The abstraction below will be updated once the migration to
> >> + * time64_t is complete.
> >> + */
> >> +#ifdef CONFIG_GENERIC_VDSO_32
> >> +#define __vdso_timespec old_timespec32
> >> +#define __vdso_timeval old_timeval32
> >> +#else
> >> +#ifdef ENABLE_COMPAT_VDSO
> >> +#define __vdso_timespec old_timespec32
> >> +#define __vdso_timeval old_timeval32
> >> +#else
> >> +#define __vdso_timespec __kernel_timespec
> >> +#define __vdso_timeval __kernel_old_timeval
> >> +#endif /* CONFIG_COMPAT_VDSO */
> >> +#endif /* CONFIG_GENERIC_VDSO_32 */
> >>
> >
> > Have you considered doing this in the reverse way, by including
> > the common parts from multiple implementations (32 and 64
> > bit), instead of compiling the same source file multiple
> > times with different macros set? I think that would make it
> > easier to understand.
> >
>
> The common code is never compiled as standalone. It includes arch specific code
> (for the fallbacks) and it is included in the arch specific vdso library (for
> both 32 and 64 bit where it makes sense). Hence it is built once or twice.
>
> If I understand correctly your question, seems inline with what I am doing.

The result is very similar, it's just a question of which file includes which
other file. If I understand your current method right, you use Makefile
logic to build multiple object files from a single source file, and setting
ENABLE_COMPAT_VDSO on one of them, right?

My preference would be to keep that Makefile simpler and have one
source file per object file, but have each one define a set of macros
before including the common source file, similarly to how we deal
with fs/compat_binfmt_elf.c. If that turns out to be worse than what
you have here, I'm not overly attached to that solution since including
C files is still ugly, but I think it's worth trying if that lets you end
up with more easily understandable source code.

Arnd