Re: [PATCH] fixes for the SLB shadow buffer

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Thu, 2007-08-02 at 09:32 +1000, Michael Neuling wrote:
> > On Wed, 2007-08-01 at 16:02 +1000, Michael Neuling wrote:
> > > We sometimes change the vmalloc segment in slb_flush_and_rebolt but we
> > > never updated with slb shadow buffer.  This fixes it.  Thanks to paulus
> > > for finding this.
> > > 
> > > Also added some write barriers to ensure the shadow buffer is always
> > > valid.
> > 
> > The shadow is global or per-cpu ?
> > 
> > Because in the later case, I think you need more than that.
> 
> It's per CPU.
> 
> > > @@ -759,6 +762,9 @@ int hash_page(unsigned long ea, unsigned
> > >  		   mmu_psize_defs[mmu_vmalloc_psize].sllp) {
> > >  		get_paca()->vmalloc_sllp =
> > >  			mmu_psize_defs[mmu_vmalloc_psize].sllp;
> > > +		vflags = SLB_VSID_KERNEL |
> > > +			mmu_psize_defs[mmu_vmalloc_psize].sllp;
> > > +		slb_shadow_update(VMALLOC_START, vflags, 1);
> > >  		slb_flush_and_rebolt();
> > >  	}
> > 
> > Later on:
> > 
> >         } else if (get_paca()->vmalloc_sllp !=
> >                    mmu_psize_defs[mmu_vmalloc_psize].sllp) {
> >                 get_paca()->vmalloc_sllp =
> >                         mmu_psize_defs[mmu_vmalloc_psize].sllp;
> >                 slb_flush_and_rebolt();
> >         }
> > 
> > If your shadow is per-cpu, you need to fix that up too.
> 
> I'm confused... isn't that the same section of code?

The first block of code detects the need for a demotion and changes the
global mmu_vmalloc_psize, along with changing the value on the local CPU
(and flushing it's SLB). But other CPUs still have the old values.

The second block of code checks if the global value matches the per-CPU
value and if not, proceeds to a local SLB flush. That's how the "other"
CPUs get the new value.

If your shadow is per-CPU, it thus needs to be updated in both cases.

Ben.


Ben.