On Thu, Jul 28, 2016 at 09:01:32AM -0600, Alex Williamson wrote:
>On Thu, 28 Jul 2016 14:52:59 +0000
>Wei Yang <[email protected]> wrote:
>
>> According to the comments and code, get_pci_function_alias_group() and
>> get_pci_alias_group() do the search on the same pci bus. This means we can
>> just iterate on the bus the pci device attaches to.
>>
>> This patch narrows the search range by just iterating on the current bus
>> and fix one typo in comment.
>>
>> Signed-off-by: Wei Yang <[email protected]>
>> ---
>> drivers/iommu/iommu.c | 22 ++++++++--------------
>> 1 file changed, 8 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 3000051..0338a81 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -643,17 +643,15 @@ static struct iommu_group
>> *get_pci_function_alias_group(struct pci_dev *pdev,
>> if (!pdev->multifunction || pci_acs_enabled(pdev, REQ_ACS_FLAGS))
>> return NULL;
>>
>> - for_each_pci_dev(tmp) {
>> - if (tmp == pdev || tmp->bus != pdev->bus ||
>> + list_for_each_entry(tmp, &pdev->bus->devices, bus_list) {
>
>I believe this was using for_each_pci_dev() exactly because it takes a
>reference to the device and therefore we can use it to get a reference
>to the group. How do you justify removing these reference semantics?
>Thanks,
>
Alex,
With some investigation, I believe these reference are important. Thanks for
reminding.
Well, the pci_dev and the bus->devices list are protected by pci_bus_sem and
we have already had a function to walk the pci bus safely by holding this
semaphore. While the pci_walk_bus() will continue the process when it sees a
bridge device.
So I create another patch to export a function to walk just on the local bus
and then calling this from get_pci_function_alias_group() and
get_pci_alias_group().
Since this change is related to pci subsystem, I include Bjorn.
Hi, Bjorn,
Haven't seen you for a long time. Hope you would like the idea.
>Alex
>
>> + if (tmp == pdev ||
>> PCI_SLOT(tmp->devfn) != PCI_SLOT(pdev->devfn) ||
>> pci_acs_enabled(tmp, REQ_ACS_FLAGS))
>> continue;
>>
>> group = get_pci_alias_group(tmp, devfns);
>> - if (group) {
>> - pci_dev_put(tmp);
>> + if (group)
>> return group;
>> - }
>> }
>>
>> return NULL;
>> @@ -681,23 +679,19 @@ static struct iommu_group *get_pci_alias_group(struct
>> pci_dev *pdev,
>> if (group)
>> return group;
>>
>> - for_each_pci_dev(tmp) {
>> - if (tmp == pdev || tmp->bus != pdev->bus)
>> + list_for_each_entry(tmp, &pdev->bus->devices, bus_list) {
>> + if (tmp == pdev)
>> continue;
>>
>> /* We alias them or they alias us */
>> if (pci_devs_are_dma_aliases(pdev, tmp)) {
>> group = get_pci_alias_group(tmp, devfns);
>> - if (group) {
>> - pci_dev_put(tmp);
>> + if (group)
>> return group;
>> - }
>>
>> group = get_pci_function_alias_group(tmp, devfns);
>> - if (group) {
>> - pci_dev_put(tmp);
>> + if (group)
>> return group;
>> - }
>> }
>> }
>>
>> @@ -794,7 +788,7 @@ struct iommu_group *pci_device_group(struct device *dev)
>>
>> /*
>> * Look for existing groups on non-isolated functions on the same
>> - * slot and aliases of those funcions, if any. No need to clear
>> + * slot and aliases of those functions, if any. No need to clear
>> * the search bitmap, the tested devfns are still valid.
>> */
>> group = get_pci_function_alias_group(pdev, (unsigned long *)devfns);
--
Wei Yang
Help you, Help me
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
