[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 12/36] KVM: x86: Restructure get_kvmclock()



On Sat, Jul 25, 2026, David Woodhouse wrote:
> On Fri, 2026-07-24 at 14:29 -0700, Sean Christopherson wrote:
> >  
> > > Given that, the get_cpu()/put_cpu() pinning is not needed either: both
> > > the TSC read and get_cpu_tsc_khz() are CPU-independent when the master
> > > clock is in use, so drop them.
> > > 
> > > Wrap the entire use_master_clock block in #ifdef CONFIG_X86_64, since
> > > use_master_clock is never true on 32-bit (host_tsc_clocksource is only
> > > set under CONFIG_X86_64), and declare hv_clock inside the block so it is
> > > not left as an unused variable on 32-bit.
> > > 
> > > Use 'continue' on the master-clock success path so the non-master-clock
> > > computation becomes the common tail, avoiding a goto and label. When the
> > > clock read fails (e.g. clocksource transitioning away from TSC), fall
> > > back to that path rather than proceeding with uninitialised data or
> > > spinning in the seqcount loop.
> > 
> > Please split this up.  The changelog suggests there are at least three 
> > logical
> > changes here.  Yeah, the series is big, but smaller patches helps with 
> > review,
> > even if it results in more total patches.
> 
> I don't think the latter two are separable; the refactoring of the
> ifdef and the loop and the way it breaks out to the tail are all
> intertwined. I may be able to pull the get_cpu()/get_cpu_tsc_khz() part
> out into a preliminary commit though. I'll take a look.

Yeah, that's totally fine.  I specifically want to isolate the removal
of get_cpu()/put_cpu() and the removal of __this_cpu_read(cpu_tsc_khz), if the
rest is an interwined mess, then so be it.




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.