On 09/10/19 18:05, John Stultz wrote: > On Wed, Oct 9, 2019 at 2:13 AM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: >> John (Stultz), does that sound good to you? The context is that >> Jianyong would like to add a hypercall that returns a (cycles, >> nanoseconds) pair to the guest. On x86 we're relying on the vclock_mode >> field that is already there for the vDSO, but being able to just use >> ktime_get_snapshot would be much nicer. > > I've not really looked at the code closely in awhile, so I'm not sure > my suggestions will be too useful. > > My only instinct is maybe to not include the clocksource pointer in > the system_time_snapshot, as I worry that structure will then be > abused by the interface users. If you're just wanting to make sure > the clocksource is what you're expecting, would instead putting only > the clocksource name in the structure suffice? Well, it would suffice but it would be quite ugly to do a string comparison later. What kind of abuse are you thinking of? We already have struct system_counterval_t for a clocksource+cycles tuple, so it seemed obvious to me to make system_time_snapshot a superset of it... In fact, system_time_snapshot's cycles member is even unused currently, so it could even be easily replaced by a struct system_counterval_t, instead of adding an extra field. Paolo
