diff mbox

[v6,18/42] powerpc/powernv: Allocate PE# in deasending order

Message ID 1438834307-26960-19-git-send-email-gwshan@linux.vnet.ibm.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Gavin Shan Aug. 6, 2015, 4:11 a.m. UTC
The available PE#, represented by a bitmap in the PHB, is allocated
in ascending order. 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.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Alexey Kardashevskiy Aug. 10, 2015, 2:39 p.m. UTC | #1
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 ;)

> 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?


>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>   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)" :)


>
>   	return pnv_ioda_init_pe(phb, pe);
>   }
>
Gavin Shan Aug. 11, 2015, 12:43 a.m. UTC | #2
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.

>>
>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>---
>>  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

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexey Kardashevskiy Aug. 11, 2015, 2:50 a.m. UTC | #3
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 <gwshan@linux.vnet.ibm.com>
>>> ---
>>>   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
>
Gavin Shan Aug. 13, 2015, 12:28 a.m. UTC | #4
On Tue, Aug 11, 2015 at 12:50:33PM +1000, Alexey Kardashevskiy wrote:
>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?
>

Your understanding is somewhat correct. Let me explain for more here. Taking
PHB3 as an example: it has 16 M64 BARs. The last BAR (15th) is running in
share mode. When one segment from this BAR is assigned to one PE, the PE number
is determined and that's equal to the segment number. However, it's still possible
one PE has multiple segments. We have "master" and "slave" PEs for the later case.

If any one left BARs (0 to 14) is running in single mode and assigned to one particular
PE. the PE number can be confiugred.

>>
>>>>
>>>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>>>---
>>>>  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

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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);
 
 	return pnv_ioda_init_pe(phb, pe);
 }