[PATCH 3/7] powerpc: Free up four 64K PTE bits in 4K backed HPTE pages

[Ram Pai]

On Wed, Oct 25, 2017 at 11:18:49AM +0200, Michael Ellerman wrote:
> Ram Pai <linuxram at us.ibm.com> writes:
> > On Mon, Oct 23, 2017 at 02:17:39PM +0530, Aneesh Kumar K.V wrote:
> >> Michael Ellerman <mpe at ellerman.id.au> writes:
> >> > Ram Pai <linuxram at us.ibm.com> writes:
> ...
> >> > Should be:
> >> > 	/*
> >> > 	 * Initialize all hidx entries to invalid value, the first time
> >> >          * the PTE is about to allocate a 4K HPTE.
> >> > 	 */
> >> >
> >> >> +	if (!(old_pte & H_PAGE_COMBO))
> >> >> +		rpte.hidx = ~0x0UL;
> >> >> +
> >> >
> >> > Paul had the idea that if we biased the slot number by 1, we could make
> >> > the "invalid" value be == 0.
> >> >
> >> > That would avoid needing to that above, and also mean the value is
> >> > correctly invalid from the get-go, which would be good IMO.
> >> >
> >> > I think now that you've added the slot accessors it would be pretty easy
> >> > to do.
> >> 
> >> That would be imply, we loose one slot in primary group, which means we
> >> will do extra work in some case because our primary now has only 7
> >> slots. And in case of pseries, the hypervisor will always return the
> >> least available slot, which implie we will do extra hcalls in case of an
> >> hpte insert to an empty group?
> >
> > No. that is not the idea.  The idea is that slot 'F' in the seconday
> > will continue to be a invalid slot, but will be represented as
> > offset-by-one in the PTE.  In other words, 0 will be repesented as 1,
> > 1 as 2....   and  n as (n+1)%32
> 
> Right.
> 
> > The idea seems feasible.  It has the advantage -- where 0 in the PTE
> > means invalid slot. But it can be confusing to the casual code-
> > reader. Will need to put in a big-huge comment to explain that.
> 
> This code is already confusing to *any* reader, so I don't think it's a
> worry. :)

I just got it coded and working.  But I see no advantage implementing
the shifted-value. The hidx in the secondary-part of the pte, still
needs to be initialzed to all-zeros. Because it could contain the value of
the hidx corresponding the 64k-backed hpte, which needs to be cleared.

I will send the patch anyway.  But we should not apply it
for I see no apparent gain.

RP

> 
> cheers

-- 
Ram Pai