On 2015/5/11 21:36, Hanjun Guo wrote:
> Hi Jiang,
>
> I have comments inline.
>
> On 2015年05月05日 10:46, Jiang Liu wrote:
>> Introduce common interface acpi_pci_root_create() and related data
>> structures to create PCI root bus for ACPI PCI host bridges. It will
>> be used to kill duplicated arch specific code for IA64 and x86. It may
>> also help ARM64 in future.
>>
>> Tested-by: Tony Luck <tony.luck@xxxxxxxxx>
>> Signed-off-by: Jiang Liu <jiang.liu@xxxxxxxxxxxxxxx>
>> ---
>> +struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>> + struct acpi_pci_root_ops *ops,
>> + size_t extra_size)
>> +{
>> + int ret, busnum = root->secondary.start;
>> + struct acpi_device *device = root->device;
>> + struct acpi_pci_root_info_common *info;
>> + struct pci_bus *bus;
>> +
>> + info = kzalloc(sizeof(*info) + extra_size, GFP_KERNEL);
>
> For x86, the memory is allocated on its local numa node if memory
> is available by using kzalloc_node(), and if
> node = acpi_get_node(device->handle) is NUMA_NO_NODE, the code will
> get the numa node info by using x86_pci_root_bus_node() (which you
> consolidate them as a function pci_acpi_root_get_node() in later
> patch).
>
> but the code here just ignore that information, so the code
> here has functional change for x86 code since you didn't use numa
> information.
>
> I'm not sure how frequently used for the info after init, so
> not sure about the performance impact, but I think we should
> keep consistence with the code behavior as before.
Hi Hanjun,
Good catch, will change code to respect NUMA node
info when allocating memory.
>
> Further more, there is a implicit cast for
> struct acpi_pci_root_info_common *info to arch specific
> struct pci_root_info *info by using extra size, it's not
> easy to understand (at least for me :) ), so how about
> alloc the memory in arch specific function, and pass
> struct acpi_pci_root_info_common *info fr this function?
Good suggestion.
Thanks!
Gerry
> other than that, this code is pretty good, I reworked ARM64
> ACPI based PCI host bridge init code, and this patch simplified
> the coed a lot, will have a test tomorrow and let you know
> the result.
>
> Thanks
> Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
