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. :) cheers