Re: [Xen-devel] [PATCH v7 2/5] x86/pvclock: add setter for pvclock_pvti_cpu0_va

[Thomas Gleixner <tglx@xxxxxxxxxxxxx>]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel