On Wed, Oct 25, 2017 at 11:18:49AM +0200, Michael Ellerman wrote: > Ram Pai <linux...@us.ibm.com> writes: > > On Mon, Oct 23, 2017 at 02:17:39PM +0530, Aneesh Kumar K.V wrote: > >> Michael Ellerman <m...@ellerman.id.au> writes: > >> > Ram Pai <linux...@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