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

Reply via email to