Re: [PATCH v6 12/42] powerpc/powernv: Increase PE# capacity

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 08/11/2015 10:38 AM, Gavin Shan wrote:
> On Mon, Aug 10, 2015 at 07:53:02PM +1000, Alexey Kardashevskiy wrote:
>> On 08/06/2015 02:11 PM, Gavin Shan wrote:
>>> Each PHB maintains an array helping to translate RID (Request
>>> ID) to PE# with the assumption that PE# takes 8 bits, indicating
>>> that we can't have more than 256 PEs. However, pci_dn->pe_number
>>> already had 4-bytes for the PE#.
>>>
>>> The patch extends the PE# capacity so that each of them will be
>>> 4-bytes long. Then we can use IODA_INVALID_PE to check one entry
>>> in phb->pe_rmap[] is valid or not.
>>>
>>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>> ---
>>>   arch/powerpc/platforms/powernv/pci-ioda.c | 8 ++++++--
>>>   arch/powerpc/platforms/powernv/pci.h      | 7 +++----
>>>   2 files changed, 9 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>>> index 57ba8fd..3094c61 100644
>>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>> @@ -786,7 +786,7 @@ static int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
>>>
>>>   	/* Clear the reverse map */
>>>   	for (rid = pe->rid; rid < rid_end; rid++)
>>> -		phb->ioda.pe_rmap[rid] = 0;
>>> +		phb->ioda.pe_rmap[rid] = IODA_INVALID_PE;
>>>
>>>   	/* Release from all parents PELT-V */
>>>   	while (parent) {
>>> @@ -3134,7 +3134,7 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
>>>   	unsigned long size, pemap_off;
>>>   	const __be64 *prop64;
>>>   	const __be32 *prop32;
>>> -	int len;
>>> +	int len, i;
>>>   	u64 phb_id;
>>>   	void *aux;
>>>   	long rc;
>>> @@ -3201,6 +3201,10 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
>>>   	if (prop32)
>>>   		phb->ioda.reserved_pe = be32_to_cpup(prop32);
>>>
>>> +	/* Invalidate RID to PE# mapping */
>>> +	for (i = 0; i < ARRAY_SIZE(phb->ioda.pe_rmap); ++i)
>>> +		phb->ioda.pe_rmap[i] = IODA_INVALID_PE;
>>> +
>>>   	/* Parse 64-bit MMIO range */
>>>   	pnv_ioda_parse_m64_window(phb);
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
>>> index 1dc9578..6f8568e 100644
>>> --- a/arch/powerpc/platforms/powernv/pci.h
>>> +++ b/arch/powerpc/platforms/powernv/pci.h
>>> @@ -175,11 +175,10 @@ struct pnv_phb {
>>>   			struct list_head	pe_list;
>>>   			struct mutex            pe_list_mutex;
>>>
>>> -			/* Reverse map of PEs, will have to extend if
>>> -			 * we are to support more than 256 PEs, indexed
>>> -			 * bus { bus, devfn }
>>> +			/* Reverse map of PEs, indexed by
>>> +			 * { bus, devfn }
>>>   			 */
>>> -			unsigned char		pe_rmap[0x10000];
>>> +			int			pe_rmap[0x10000];
>>
>>
>> 256k seems to be waste when only tiny fraction of it will ever be used. Using
>> include/linux/hashtable.h makes sense here, and if you use a hashtable, you
>> won't have to initialize anything with IODA_INVALID_PE.
>>
>
> I'm not sure if I follow your idea completely. With hash table to trace
> RID mapping here, won't more memory needed if all PCI buse numbers (0
> to 255) are all valid? It means hash table doesn't have advantage in
> memory consumption.

You need 3 bytes - one for a bus and two for devfn - which makes it a 
perfect 32bit has key and you only store existing devices in a hash so you 
do not waste memory.


> On the other hand, searching in hash table buckets
> have to iterate list of conflicting items (keys), which is slow comparing
> to what we have.

How often do you expect this code to execute? Is not it setup-type and 
hotplug only? Unless it is thousands times per second, it is not an issue here.


> Actually, I like the idea, using array to map RID to PE#,
> which was implemented by Ben.

Where?



>
>>
>>>
>>>   			/* 32-bit TCE tables allocation */
>>>   			unsigned long		dma32_segcount;
>>>
>
> Thanks,
> Gavin
>


-- 
Alexey