DPDK patches and discussions
 help / color / mirror / Atom feed
From: Matan Azrad <matan@mellanox.com>
To: Jeff Guo <jia.guo@intel.com>,
	"konstantin.ananyev@intel.com" <konstantin.ananyev@intel.com>,
	"anatoly.burakov@intel.com" <anatoly.burakov@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	"bernard.iremonger@intel.com" <bernard.iremonger@intel.com>,
	"jingjing.wu@intel.com" <jingjing.wu@intel.com>,
	"wenzhuo.lu@intel.com" <wenzhuo.lu@intel.com>
Cc: "ferruh.yigit@intel.com" <ferruh.yigit@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"helin.zhang@intel.com" <helin.zhang@intel.com>,
	"shaopeng.he@intel.com" <shaopeng.he@intel.com>
Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: fix callback issue for hot-unplug
Date: Tue, 6 Nov 2018 06:36:03 +0000	[thread overview]
Message-ID: <AM0PR0502MB401977B7D4AEEF569F86942CD2CB0@AM0PR0502MB4019.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <1541484436-91320-4-git-send-email-jia.guo@intel.com>

Hi Jeff

 From: Jeff Guo <jia.guo@intel.com>
> Before detach device when device be hot-unplugged, the failure process in
> user space and kernel space both need to be finished, such as eal interrupt
> callback need to be inactive before the callback be unregistered when device
> is being cleaned. This patch add rte alarm for device detaching, with that it
> could finish interrupt callback soon and give time to let the failure process
> done before detaching.
> 
> Fixes: 2049c5113fe8 ("app/testpmd: use hotplug failure handler")
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
>  app/test-pmd/testpmd.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 9c0edca..9c673cf 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2620,7 +2620,18 @@ eth_dev_event_callback(const char
> *device_name, enum rte_dev_event_type type,
>  				device_name);
>  			return;
>  		}
> -		rmv_event_callback((void *)(intptr_t)port_id);
> +		/*
> +		 * Before detach device, the hot-unplug failure process in
> +		 * user space and kernel space both need to be finished,
> +		 * such as eal interrupt callback need to be inactive before
> +		 * the callback be unregistered when device is being cleaned.
> +		 * So finished interrupt callback soon here and give time to
> +		 * let the work done before detaching.
> +		 */
> +		if (rte_eal_alarm_set(100000,
> +				rmv_event_callback, (void
> *)(intptr_t)port_id))
> +			RTE_LOG(ERR, EAL,
> +				"Could not set up deferred device


It looks me strange to use callback and alarm to remove a device:
Why not to use callback and that is it?

I think that it's better to let the EAL to detach the device after all the callbacks were done and not to do it by the user callback.
So the application\callback owners just need to clean its resources with understanding that after the callback the device(and the callback itself) will be detached by the EAL. 


> removal\n");
>  		break;
>  	case RTE_DEV_EVENT_ADD:
>  		RTE_LOG(ERR, EAL, "The device: %s has been added!\n",
> --
> 2.7.4

  reply	other threads:[~2018-11-06  6:36 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-06  6:07 [dpdk-dev] [PATCH 0/3] fix vfio hot-unplug issue Jeff Guo
2018-11-06  6:07 ` [dpdk-dev] [PATCH 1/3] eal: fix lock issue for hot-unplug Jeff Guo
2018-11-06  6:22   ` Matan Azrad
2018-11-07  5:49     ` Jeff Guo
2018-11-08  7:08       ` Matan Azrad
2018-11-06  6:07 ` [dpdk-dev] [PATCH 2/3] vfio: fix to add handler lock " Jeff Guo
2018-11-06  6:23   ` Matan Azrad
2018-11-07  6:15     ` Jeff Guo
2018-11-08  7:20       ` Matan Azrad
2018-11-08  8:30         ` Jeff Guo
2018-11-06  6:07 ` [dpdk-dev] [PATCH 3/3] app/testpmd: fix callback issue " Jeff Guo
2018-11-06  6:36   ` Matan Azrad [this message]
2018-11-07  7:30     ` Jeff Guo
2018-11-07 11:05       ` Ananyev, Konstantin
2018-11-08  7:28         ` Matan Azrad
2018-11-08  8:49           ` Jeff Guo
2018-11-08  9:35             ` Matan Azrad
2018-11-09  3:55               ` Jeff Guo
2018-11-09  5:24                 ` Matan Azrad
2018-11-09  6:17                   ` Jeff Guo
     [not found]                     ` <AM0PR0502MB401938411A7E2BA9E76576A2D2C00@AM0PR0502MB4019.eurprd05.prod.outlook.com>
2018-11-12  1:35                       ` Thomas Monjalon
2018-11-14  9:32                         ` Jeff Guo
2018-11-15  9:18 ` [dpdk-dev] [PATCH V2 0/3] fix vfio hot-unplug issue Jeff Guo
2018-11-15  9:18   ` Jeff Guo
2018-11-18 16:19     ` Thomas Monjalon
2018-11-15  9:18   ` [dpdk-dev] [PATCH V2 1/3] eal: fix lock issue for hot-unplug Jeff Guo
2018-11-15  9:18   ` [dpdk-dev] [PATCH V2 2/3] vfio: fix to add handler lock " Jeff Guo
2018-11-15  9:18   ` [dpdk-dev] [PATCH V2 3/3] app/testpmd: fix callback issue " Jeff Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM0PR0502MB401977B7D4AEEF569F86942CD2CB0@AM0PR0502MB4019.eurprd05.prod.outlook.com \
    --to=matan@mellanox.com \
    --cc=anatoly.burakov@intel.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=helin.zhang@intel.com \
    --cc=jia.guo@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=shaopeng.he@intel.com \
    --cc=thomas@monjalon.net \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).