diff mbox

[dpdk-dev,v3,18/28] eal/pci: Prevent double registrations for pci_device_list

Message ID 1418106629-22227-19-git-send-email-mukawa@igel.co.jp (mailing list archive)
State Superseded, archived
Headers

Commit Message

Tetsuya Mukawa Dec. 9, 2014, 6:30 a.m. UTC 
  The patch fixes pci_scan_one() not to register same pci devices twice.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Michael Qiu Dec. 11, 2014, 3:24 a.m. UTC | #1 
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

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@igel.co.jp>
> ---
>  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);
>  	}
Tetsuya Mukawa Dec. 11, 2014, 5:33 a.m. UTC | #2 
(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@igel.co.jp>
>> ---
>>  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);
>>  	}
diff mbox

Patch

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);
 	}