Subject: Re: [PATCH v6 18/42] powerpc/powernv: Allocate PE# in deasending order

From: Alexey Kardashevskiy <[email-masked]>
Date: Tue, 11 Aug 2015 12:50:33 +1000

On 08/11/2015 10:43 AM, Gavin Shan wrote:
> On Tue, Aug 11, 2015 at 12:39:02AM +1000, Alexey Kardashevskiy wrote:
>> On 08/06/2015 02:11 PM, Gavin Shan wrote:
>>> The available PE#, represented by a bitmap in the PHB, is allocated
>>> in ascending order.
>>
>> Available PE# is available exactly because it is not allocated ;)
>>
>
> Yeah, will correct it.
>
>>> It conflicts with the fact that M64 segments are
>>> assigned in same order. In order to avoid the conflict, the patch
>>> allocates PE# in descending order.
>>
>> What kind of conflict?
>>
>
> On PHB3, the M64 segment is assigned to one PE whose PE number is
> determined. M64 segment are allocated in ascending order. It's why
> I would like to allocate PE# in deascending order.

 From previous lessons, I thought M64 segment number is PE# number as well :-/
Seems this is not the case, so what does store this seg#<->PE# mapping in PHB?

>
>>>
>>> Signed-off-by: Gavin Shan <[email-masked]>
>>> ---
>>>   arch/powerpc/platforms/powernv/pci-ioda.c | 11 ++++++++---
>>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>>> index 56b058c..1c950e8 100644
>>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>> @@ -161,13 +161,18 @@ static struct pnv_ioda_pe *pnv_ioda_reserve_pe(struct pnv_phb *phb, int pe_no)
>>>   static struct pnv_ioda_pe *pnv_ioda_alloc_pe(struct pnv_phb *phb)
>>>   {
>>>   	unsigned long pe;
>>> +	unsigned long limit = phb->ioda.total_pe_num - 1;
>>>
>>>   	do {
>>>   		pe = find_next_zero_bit(phb->ioda.pe_alloc,
>>> -					phb->ioda.total_pe_num, 0);
>>> -		if (pe >= phb->ioda.total_pe_num)
>>> +					phb->ioda.total_pe_num, limit);
>>> +		if (pe < phb->ioda.total_pe_num &&
>>> +		    !test_and_set_bit(pe, phb->ioda.pe_alloc))
>>> +			break;
>>> +
>>> +		if (--limit >= phb->ioda.total_pe_num)
>>>   			return NULL;
>>> -	} while(test_and_set_bit(pe, phb->ioda.pe_alloc));
>>> +	} while (1);
>>
>>
>> Usually, if it is "while(1)", then it is "while(1){}" rather than
>> "do{}while(1)" :)
>
> Agree, will change it.
>
>>
>>
>>>
>>>   	return pnv_ioda_init_pe(phb, pe);
>>>   }
>>>
>
> Thanks,
> Gavin
>

-- 
Alexey