pci/vfio: fixup rte_intr_callback_unregister() handling

Message ID 20181024101131.17756-1-dariusz.stojaczyk@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series pci/vfio: fixup rte_intr_callback_unregister() handling |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Stojaczyk, Dariusz Oct. 24, 2018, 10:11 a.m. UTC
  This function is documented to return the number of unregistered
callbacks or negative numbers on error, but pci_vfio checks for
ret != 0 to detect failures. Not anymore.

Fixes: c115fd000c32 ("vfio: handle hotplug request notifier")
Cc: jia.guo@intel.com
Cc: stable@dpdk.org

Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
---
 drivers/bus/pci/linux/pci_vfio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Anatoly Burakov Oct. 26, 2018, 2:23 p.m. UTC | #1
On 24-Oct-18 11:11 AM, Darek Stojaczyk wrote:
> This function is documented to return the number of unregistered
> callbacks or negative numbers on error, but pci_vfio checks for
> ret != 0 to detect failures. Not anymore.
> 
> Fixes: c115fd000c32 ("vfio: handle hotplug request notifier")
> Cc: jia.guo@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
  
Thomas Monjalon Oct. 29, 2018, 12:10 a.m. UTC | #2
26/10/2018 16:23, Burakov, Anatoly:
> On 24-Oct-18 11:11 AM, Darek Stojaczyk wrote:
> > This function is documented to return the number of unregistered
> > callbacks or negative numbers on error, but pci_vfio checks for
> > ret != 0 to detect failures. Not anymore.
> > 
> > Fixes: c115fd000c32 ("vfio: handle hotplug request notifier")
> > Cc: jia.guo@intel.com
> > Cc: stable@dpdk.org

No need for stable backport here.

> > Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks
  
Kevin Traynor Nov. 20, 2018, 11:04 a.m. UTC | #3
On 10/24/2018 11:11 AM, Darek Stojaczyk wrote:
> This function is documented to return the number of unregistered
> callbacks or negative numbers on error, but pci_vfio checks for
> ret != 0 to detect failures. Not anymore.
> 
> Fixes: c115fd000c32 ("vfio: handle hotplug request notifier")
> Cc: jia.guo@intel.com
> Cc: stable@dpdk.org

The Fixes: commit indicates this is not for stable, and the modified
function was added in that commit. If there is some equivalent fix for
stable, please send a backport for that.

> 
> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> ---
>  drivers/bus/pci/linux/pci_vfio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
> index b1f0683fe..305cc0601 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -341,7 +341,7 @@ pci_vfio_enable_notifier(struct rte_pci_device *dev, int vfio_dev_fd)
>  		ret = rte_intr_callback_unregister(&dev->vfio_req_intr_handle,
>  						 pci_vfio_req_handler,
>  						 (void *)&dev->device);
> -		if (ret)
> +		if (ret < 0)
>  			RTE_LOG(ERR, EAL,
>  				"Fail to unregister req notifier handler.\n");
>  		goto error;
> @@ -373,7 +373,7 @@ pci_vfio_disable_notifier(struct rte_pci_device *dev)
>  	ret = rte_intr_callback_unregister(&dev->vfio_req_intr_handle,
>  					   pci_vfio_req_handler,
>  					   (void *)&dev->device);
> -	if (ret) {
> +	if (ret < 0) {
>  		RTE_LOG(ERR, EAL,
>  			 "fail to unregister req notifier handler.\n");
>  		return -1;
>
  
Kevin Traynor Nov. 20, 2018, 11:05 a.m. UTC | #4
On 11/20/2018 11:04 AM, Kevin Traynor wrote:
> On 10/24/2018 11:11 AM, Darek Stojaczyk wrote:
>> This function is documented to return the number of unregistered
>> callbacks or negative numbers on error, but pci_vfio checks for
>> ret != 0 to detect failures. Not anymore.
>>
>> Fixes: c115fd000c32 ("vfio: handle hotplug request notifier")
>> Cc: jia.guo@intel.com
>> Cc: stable@dpdk.org
> 
> The Fixes: commit indicates this is not for stable, and the modified
> function was added in that commit. If there is some equivalent fix for
> stable, please send a backport for that.
> 

oh, I should have read the rest of the thread before replying. Ignore
previous mail, thanks.

>>
>> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
>> ---
>>  drivers/bus/pci/linux/pci_vfio.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
>> index b1f0683fe..305cc0601 100644
>> --- a/drivers/bus/pci/linux/pci_vfio.c
>> +++ b/drivers/bus/pci/linux/pci_vfio.c
>> @@ -341,7 +341,7 @@ pci_vfio_enable_notifier(struct rte_pci_device *dev, int vfio_dev_fd)
>>  		ret = rte_intr_callback_unregister(&dev->vfio_req_intr_handle,
>>  						 pci_vfio_req_handler,
>>  						 (void *)&dev->device);
>> -		if (ret)
>> +		if (ret < 0)
>>  			RTE_LOG(ERR, EAL,
>>  				"Fail to unregister req notifier handler.\n");
>>  		goto error;
>> @@ -373,7 +373,7 @@ pci_vfio_disable_notifier(struct rte_pci_device *dev)
>>  	ret = rte_intr_callback_unregister(&dev->vfio_req_intr_handle,
>>  					   pci_vfio_req_handler,
>>  					   (void *)&dev->device);
>> -	if (ret) {
>> +	if (ret < 0) {
>>  		RTE_LOG(ERR, EAL,
>>  			 "fail to unregister req notifier handler.\n");
>>  		return -1;
>>
>
  

Patch

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index b1f0683fe..305cc0601 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -341,7 +341,7 @@  pci_vfio_enable_notifier(struct rte_pci_device *dev, int vfio_dev_fd)
 		ret = rte_intr_callback_unregister(&dev->vfio_req_intr_handle,
 						 pci_vfio_req_handler,
 						 (void *)&dev->device);
-		if (ret)
+		if (ret < 0)
 			RTE_LOG(ERR, EAL,
 				"Fail to unregister req notifier handler.\n");
 		goto error;
@@ -373,7 +373,7 @@  pci_vfio_disable_notifier(struct rte_pci_device *dev)
 	ret = rte_intr_callback_unregister(&dev->vfio_req_intr_handle,
 					   pci_vfio_req_handler,
 					   (void *)&dev->device);
-	if (ret) {
+	if (ret < 0) {
 		RTE_LOG(ERR, EAL,
 			 "fail to unregister req notifier handler.\n");
 		return -1;