DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jeff Guo <jia.guo@intel.com>
To: Matan Azrad <matan@mellanox.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 2/3] vfio: fix to add handler lock for hot-unplug
Date: Thu, 8 Nov 2018 16:30:57 +0800
Message-ID: <f90895b5-d3e7-4478-a9fd-8c58274a0e00@intel.com> (raw)
In-Reply-To: <AM0PR0502MB4019202DC43D1F977410CC10D2C50@AM0PR0502MB4019.eurprd05.prod.outlook.com>


On 11/8/2018 3:20 PM, Matan Azrad wrote:
>
> From: Jeff Guo
>> On 11/6/2018 2:23 PM, Matan Azrad wrote:
>>> Hi Jeff
>>>
>>> Can you detail more in the commit log that we can understand the
>> synchronization problematic scenario. And how does this commit fix it?
>> Please check my reply in the 1/3 mail. And explain more here is that, when
>> device be hot-unplugged in vfio, the req notifier will invoked, then user
>> space could release device resource in user space side,
>>
>> then vfio check that the device be released out from the device group, it will
>> take the device control again and trigger the device kernel release
>> processing, at the mean time it will sent remove uevent to
>>
>> user space. Here although the req handler seems will always process before
>> uevent handler, but even for fast path and slow path protection of device
>> accessing when device is removing , it should also be need.
>>
>> what do you think about that?
> Just don't understanf hoe the fast\control-path can access to pci_vfio_req_handler...


The pci_vfio_req_handler is register in eal interrupt thread when pci 
probe driver, so it should be access in the control path.

Since the sigbus handler will be enable when enable hotplug , if origin 
sigbus occur the sigbus handler be invoked,  in the handler it will 
process the bus and device.  So i think protection of the bus and device 
so that it will not cause race condition that should be need.


>>
>>>> -----Original Message-----
>>>> From: Jeff Guo <jia.guo@intel.com>
>>>> Sent: Tuesday, November 6, 2018 8:07 AM
>>>> To: konstantin.ananyev@intel.com; anatoly.burakov@intel.com; Thomas
>>>> Monjalon <thomas@monjalon.net>; bernard.iremonger@intel.com;
>>>> jingjing.wu@intel.com; wenzhuo.lu@intel.com
>>>> Cc: ferruh.yigit@intel.com; dev@dpdk.org; jia.guo@intel.com;
>>>> helin.zhang@intel.com; Matan Azrad <matan@mellanox.com>;
>>>> shaopeng.he@intel.com
>>>> Subject: [PATCH 2/3] vfio: fix to add handler lock for hot-unplug
>>>>
>>>> This patch add hot-unplug handler lock and unlock in device request
>>>> handler when process bus and device resource, in order to avoid the
>>>> synchronization issue when device be hot-unplugged.
>>>>
>>>> Fixes: c115fd000c32 ("vfio: handle hotplug request notifier")
>>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>>>> ---
>>>>    drivers/bus/pci/linux/pci_vfio.c | 14 +++++++++++++-
>>>>    1 file changed, 13 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/bus/pci/linux/pci_vfio.c
>>>> b/drivers/bus/pci/linux/pci_vfio.c
>>>> index 305cc06..d2c8410 100644
>>>> --- a/drivers/bus/pci/linux/pci_vfio.c
>>>> +++ b/drivers/bus/pci/linux/pci_vfio.c
>>>> @@ -19,6 +19,7 @@
>>>>    #include <rte_vfio.h>
>>>>    #include <rte_eal.h>
>>>>    #include <rte_bus.h>
>>>> +#include <rte_spinlock.h>
>>>>
>>>>    #include "eal_filesystem.h"
>>>>
>>>> @@ -35,6 +36,14 @@
>>>>     * This file is only compiled if CONFIG_RTE_EAL_VFIO is set to "y".
>>>>     */
>>>>
>>>> +/*
>>>> + * spinlock for device hot-unplug failure handling. If it try to
>>>> +access bus or
>>>> + * device, such as handle sigbus on bus or handle memory failure for
>>>> +device
>>>> + * just need to use this lock. It could protect the bus and the
>>>> +device to avoid
>>>> + * race condition.
>>>> + */
>>>> +static rte_spinlock_t failure_handle_lock =
>>>> +RTE_SPINLOCK_INITIALIZER;
>>>> +
>>>>    #ifdef VFIO_PRESENT
>>>>
>>>>    #ifndef PAGE_SIZE
>>>> @@ -289,11 +298,12 @@ pci_vfio_req_handler(void *param)
>>>>    	int ret;
>>>>    	struct rte_device *device = (struct rte_device *)param;
>>>>
>>>> +	rte_spinlock_lock(&failure_handle_lock);
>>>>    	bus = rte_bus_find_by_device(device);
>>>>    	if (bus == NULL) {
>>>>    		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n",
>>>>    			device->name);
>>>> -		return;
>>>> +		goto handle_end;
>>>>    	}
>>>>
>>>>    	/*
>>>> @@ -306,6 +316,8 @@ pci_vfio_req_handler(void *param)
>>>>    		RTE_LOG(ERR, EAL,
>>>>    			"Can not handle hot-unplug for device (%s)\n",
>>>>    			device->name);
>>>> +handle_end:
>>>> +	rte_spinlock_unlock(&failure_handle_lock);
>>>>    }
>>>>
>>>>    /* enable notifier (only enable req now) */
>>>> --
>>>> 2.7.4

  reply	other threads:[~2018-11-08  8:31 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 [this message]
2018-11-06  6:07 ` [dpdk-dev] [PATCH 3/3] app/testpmd: fix callback issue " Jeff Guo
2018-11-06  6:36   ` Matan Azrad
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=f90895b5-d3e7-4478-a9fd-8c58274a0e00@intel.com \
    --to=jia.guo@intel.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=jingjing.wu@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=matan@mellanox.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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git