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

Re: [PATCH] x86/smp: Factor out parts of native_smp_prepare_cpus()



On Mon, Nov 08, 2021 at 12:20:26PM -0500, Boris Ostrovsky wrote:
> 
> On 11/8/21 10:11 AM, Peter Zijlstra wrote:
> > On Tue, Nov 02, 2021 at 07:36:36PM -0400, Boris Ostrovsky wrote:
> > > Commit 66558b730f25 ("sched: Add cluster scheduler level for x86")
> > > introduced cpu_l2c_shared_map mask which is expected to be initialized
> > > by smp_op.smp_prepare_cpus(). That commit only updated
> > > native_smp_prepare_cpus() version but not xen_pv_smp_prepare_cpus().
> > > As result Xen PV guests crash in set_cpu_sibling_map().
> > > 
> > > While the new mask can be allocated in xen_pv_smp_prepare_cpus() one can
> > > see that both versions of smp_prepare_cpus ops share a number of common
> > > operations that can be factored out. So do that instead.
> > > 
> > > Fixes: 66558b730f25 ("sched: Add cluster scheduler level for x86")
> > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > Thanks! I'll go stick that somewhere /urgent (I've had another report on
> > that here:
> > 
> >    
> > https://lkml.kernel.org/r/20211105074139.GE174703@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> > )
> 
> 
> Thank you. (I don't see this message btw)

Urgh, that thread never went to lkml :/

> > But looking at those functions; there seems to be more spurious
> > differences. For example, the whole sched_topology thing.
> 
> 
> I did look at that and thought this should be benign given that Xen PV
> is not really topology-aware. I didn't see anything that would be a
> cause for concern but perhaps you can point me to things I missed.

And me not being Xen aware... What does Xen-PV guests see of the CPUID
topology fields? Does it fully sanitize the CPUID data, or is it a clean
pass-through from whatever CPU the vCPU happens to run on at the time?


> > Should we re-architect this whole smp_prepare_cpus() thing instead? Have
> > a common function and a guest function? HyperV for instance seems to
> > call native_smp_prepare_cpus() and then does something extra (as does
> > xen_hvm).
> 
> 
> Something like
> 
> 
> void smp_prepare_cpus()
> 
> {
> 
>     // Code that this patch moved to smp_prepare_cpus_common();
> 
> 
>    smp_ops.smp_prepare_cpus();  // Including baremetal
> 
> }
> 
> 
> ?
> 
> 
> XenHVM and hyperV will need to call native smp_op too. Not sure this
> will be prettier than what it is now?

Hurmph, yeah. I was thinking it would allow pre and post common code,
but yeah, doesn't seem to make sense for now.



 


Rackspace

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