On 11/10/18 4:33 PM, Arnd Bergmann wrote: > On 11/9/18, Vincenzo Frascino <vincenzo.frascino@xxxxxxx> wrote: >> vDSO (virtual dynamic shared object) is a mechanism that the Linux >> kernel provides as an alternative to system calls to reduce where >> possible the costs in terms of cycles. >> This is possible because certain syscalls like gettimeofday() do >> not write any data and return one or more values that are stored >> in the kernel, which makes relatively safe calling them directly >> as a library function. >> >> Even if the mechanism is pretty much standard, every architecture >> in the last few years ended up implementing their own vDSO library >> in the architectural code. >> >> The purpose of this patch-set is to identify the commonalities in >> between the architectures and try to consolidate the common code >> paths, starting with gettimeofday(). > > I'm very happy to see the generalization overall, as this is going > to make it much easier to add the 64-bit time_t on the 32-bit > vdso implementations, as well as other extensions we may want > in the future. > Thanks, >> This implementation contains the following design choices: >> * Every architecture defines the arch specific code in an header in >> "asm/vdso/". >> * The generic implementation includes the arch specific one and lives >> in "lib/vdso". > > Makes sense. > >> * The arch specific code for gettimeofday lives in >> "<arch path>/vdso/gettimeofday.c" and includes the generic code only. >> * This approach allows to consolidate the common code in a single place >> with the benefit of avoiding code duplication. > > I would prefer to have that named clock_gettime.c and only implement > that one system call there. We can easily have the gettimeofday(), > time() and clock_getres() in the common code where that is needed > for backwards compatibility. As I already mentioned in my comment > for one of the patches, we won't have time() or gettimeofday() system > calls on new architectures, and 32-bit architectures won't have them > with 64-bit time_t. > The reason why I used gettimeofday.c as a filename is because it is the naming convention that I could see more often in the kernel and I feel that, if we have even the implementations of gettimeofday(), time() and clock_getres() in the same file is probably the most representative. > I suppose we can have the time() and gettimeofday() calls > implemented in the generic vdso for everyone, but then > only hook those up on architectures that already had > them. The implementation of course is trivial based on > clock_gettime() with CLOCK_REALTIME or CLOCK_REALTIME_COARSE > respectively. > I agree, and in the long term would be my preferred approach as well, but I would prefer to keep this "optimization" in a separate patchset for what I mentioned in my previous email. >> This implementation contains the portings to the common library for: arm64, >> compat mode for arm64, arm and mips. >> >> The mips porting has been tested on qemu for mips32el. A configuration to >> repeat the tests can be found at [4]. > > Ah, I missed the fact that you didn't do this for x86, so I assumed that > this work was triggered by Thomas' recent cleanup of the x86 clock_gettime > vdso code. I think it would be best to get x86 to use the same code and > get the recent improvements ported to the others as well that way. > Yes, I agree it would be great to have have the other architectures to join the effort because I believe that the whole ecosystem would benefit from it. > Arnd > -- Regards, Vincenzo