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

Alexey Kardashevskiy
2015-08-11T02:47:25Z

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