On Wed, 8 Nov 2017, Joao Martins wrote: > On 11/08/2017 11:06 AM, Thomas Gleixner wrote: > > The only nit-pick I have are the convoluted function names: > > > > pvclock_set_pvti_cpu0_va() pvclock_pvti_cpu0_va() > > > > What on earth does that mean? > > > Those two functions respectively set and get in pvclock common code the address > of a page for vCPU 0 containing time info (pvti, which is periodically updated > by hypervisor). This region is guest memory and registered with hypervisor by > guest PV clocksource and set in pvclock if certain conditions are met (i.e. > PVCLOCK_TSC_STABLE_BIT is supported by hypervisor), and the getter is afterwards > used by vdso and ptp_kvm. > > FWIW I merely followed the current style/code of the existent function but there > could be a better name like "pvclock_set_data() pvclock_get_data()". Albeit the > current names are more explicit on what we should expect to set or return from > the functions. Fair enough. > > > Aside of that can you please make it at least symetric, i.e. _set_ and > > _get_ ? > > > OK - Provided this is changing an exported symbol (pvclock_pvti_cpu0_va in use > by ptp_kvm) and a non-functional change would you want me to address in a > separate patch or it is OK to have in this one? Just fixup the ptp_kvm call site in the very same patch. Thanks, tglx