Re: [PATCH v3 18/28] eal/pci: Prevent double registrations for pci_device_list

[Tetsuya Mukawa <mukawa-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org>]

(2014/12/11 12:24), Qiu, Michael wrote:
> If you modify one function, make sure all places, where call the
> function, have been modified accordingly in same patch.
>
> Please do not split it.  Because we must make sure every patch could
> make dpdk work( I think without this patch, it will have some issue
> after you applied "Replace pci address comparison code by
> eal_compare_pci_addr").
>
> What's more, with your patch set, if someone wants to find out the
> commit, which result  in bugs or issues, use git bisect, it may be fail
> by your patch, although it's none of your patch's business.
>
> So please merge this patch to
> [PATCH v3 03/28] eal/pci: Replace pci address comparison code by
> eal_compare_pci_addr

I appreciate your comment.
You are right. The following patch has a runtime issue.
[PATCH v3 03/28] eal/pci: Replace pci address comparison code by
eal_compare_pci_addr
And it will be fixed in following.
[dpdk-dev] [PATCH v3 18/28] eal/pci: Prevent double registrations for
pci_device_list

I will merge it.

Thanks,
Tetsuya

> If other patch, I missed, with the same issue, please take same action.
>
> Thanks,
> Michael
> On 12/9/2014 2:33 PM, Tetsuya Mukawa wrote:
>> The patch fixes pci_scan_one() not to register same pci devices twice.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org>
>> ---
>>  lib/librte_eal/linuxapp/eal/eal_pci.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> index fe212d1..355c858 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> @@ -306,14 +306,17 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
>>  	}
>>  	else {
>>  		struct rte_pci_device *dev2 = NULL;
>> +		int ret;
>>  
>>  		TAILQ_FOREACH(dev2, &pci_device_list, next) {
>> -			if (eal_compare_pci_addr(&dev->addr, &dev2->addr) != 0)
>> +			ret = eal_compare_pci_addr(&dev->addr, &dev2->addr);
>> +			if (ret > 0)
>>  				continue;
>> -			else {
>> +			else if (ret < 0) {
>>  				TAILQ_INSERT_BEFORE(dev2, dev, next);
>>  				return 0;
>> -			}
>> +			} else	/* already registered */
>> +				return 0;
>>  		}
>>  		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
>>  	}