DPDK patches and discussions
 help / color / mirror / Atom feed
From: Adrian Moreno <amorenoz@redhat.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>,
	matan@mellanox.com, xiao.w.wang@intel.com,
	zhihong.wang@intel.com, chenbo.xia@intel.com,
	david.marchand@redhat.com, viacheslavo@mellanox.com,
	hemant.agrawal@nxp.com, sachin.saxena@nxp.com, grive@u256.net,
	dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v3 09/14] vhost: use linked-list for vDPA devices
Date: Fri, 26 Jun 2020 16:46:24 +0200	[thread overview]
Message-ID: <a674e2d1-7f6f-c765-28a1-5a7533091ced@redhat.com> (raw)
In-Reply-To: <20200626140441.1441763-10-maxime.coquelin@redhat.com>



On 6/26/20 4:04 PM, Maxime Coquelin wrote:
> There is no more notion of device ID outside of vdpa.c.
> We can now move from array to linked-list model for keeping
> track of the vDPA devices.
> 
> There is no point in using array here, as all vDPA API are
> used from the control path, so no performance concerns.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/rte_vdpa.h |   1 +
>  lib/librte_vhost/vdpa.c     | 134 +++++++++++++++++++-----------------
>  2 files changed, 70 insertions(+), 65 deletions(-)
> 
> diff --git a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h
> index da8ee33ab1..dbdc273702 100644
> --- a/lib/librte_vhost/rte_vdpa.h
> +++ b/lib/librte_vhost/rte_vdpa.h
> @@ -105,6 +105,7 @@ struct rte_vdpa_dev_ops {
>   * vdpa device structure includes device address and device operations.
>   */
>  struct rte_vdpa_device {
> +	TAILQ_ENTRY(rte_vdpa_device) next;
>  	/** Generic device information */
>  	struct rte_device *device;
>  	/** vdpa device operations */
> diff --git a/lib/librte_vhost/vdpa.c b/lib/librte_vhost/vdpa.c
> index bed20a3103..72561552c3 100644
> --- a/lib/librte_vhost/vdpa.c
> +++ b/lib/librte_vhost/vdpa.c
> @@ -9,35 +9,54 @@
>   */
>  
>  #include <stdbool.h>
> +#include <sys/queue.h>
>  
>  #include <rte_class.h>
>  #include <rte_malloc.h>
> +#include <rte_spinlock.h>
> +#include <rte_tailq.h>
> +
>  #include "rte_vdpa.h"
>  #include "vhost.h"
>  
> -static struct rte_vdpa_device vdpa_devices[MAX_VHOST_DEVICE];
> +/** Double linked list of vDPA devices. */
> +TAILQ_HEAD(vdpa_device_list, rte_vdpa_device);
> +
> +static struct vdpa_device_list vdpa_device_list =
> +		TAILQ_HEAD_INITIALIZER(vdpa_device_list);
> +static rte_spinlock_t vdpa_device_list_lock = RTE_SPINLOCK_INITIALIZER;
>  static uint32_t vdpa_device_num;
>  
>  
> -struct rte_vdpa_device *
> -rte_vdpa_find_device_by_name(const char *name)
> +/* Unsafe, needs to be called with vdpa_device_list_lock held */
> +static struct rte_vdpa_device *
> +__vdpa_find_device_by_name(const char *name)
>  {
> -	struct rte_vdpa_device *dev;
> -	int i;
> +	struct rte_vdpa_device *dev, *ret = NULL;
>  
>  	if (name == NULL)
>  		return NULL;
>  
> -	for (i = 0; i < MAX_VHOST_DEVICE; ++i) {
> -		dev = &vdpa_devices[i];
> -		if (dev->ops == NULL)
> -			continue;
> -
> -		if (strncmp(dev->device->name, name, RTE_DEV_NAME_MAX_LEN) == 0)
> -			return dev;
> +	TAILQ_FOREACH(dev, &vdpa_device_list, next) {
> +		if (!strncmp(dev->device->name, name, RTE_DEV_NAME_MAX_LEN)) {
> +			ret = dev;
> +			break;
> +		}
>  	}
>  
> -	return NULL;
> +	return ret;
> +}
> +
> +struct rte_vdpa_device *
> +rte_vdpa_find_device_by_name(const char *name)
> +{
> +	struct rte_vdpa_device *dev;
> +
> +	rte_spinlock_lock(&vdpa_device_list_lock);
> +	dev = __vdpa_find_device_by_name(name);
> +	rte_spinlock_unlock(&vdpa_device_list_lock);
> +
> +	return dev;
>  }
>  
>  struct rte_device *
> @@ -54,52 +73,52 @@ rte_vdpa_register_device(struct rte_device *rte_dev,
>  		struct rte_vdpa_dev_ops *ops)
>  {
>  	struct rte_vdpa_device *dev;
> -	int i;
>  
> -	if (vdpa_device_num >= MAX_VHOST_DEVICE || ops == NULL)
> +	if (ops == NULL)
>  		return NULL;
>  
> -	for (i = 0; i < MAX_VHOST_DEVICE; i++) {
> -		dev = &vdpa_devices[i];
> -		if (dev->ops == NULL)
> -			continue;
> -
> -		if (dev->device == rte_dev)
> -			return NULL;
> -	}
> -
> -	for (i = 0; i < MAX_VHOST_DEVICE; i++) {
> -		if (vdpa_devices[i].ops == NULL)
> -			break;
> +	rte_spinlock_lock(&vdpa_device_list_lock);
> +	/* Check the device hasn't been register already */
> +	dev = __vdpa_find_device_by_name(rte_dev->name);
> +	if (dev) {
> +		dev = NULL;
> +		goto out_unlock;
>  	}
>  
> -	if (i == MAX_VHOST_DEVICE)
> -		return NULL;
> +	dev = rte_zmalloc(NULL, sizeof(*dev), 0);
> +	if (!dev)
> +		goto out_unlock;
>  
> -	dev = &vdpa_devices[i];
>  	dev->device = rte_dev;
>  	dev->ops = ops;
> +	TAILQ_INSERT_TAIL(&vdpa_device_list, dev, next);
>  	vdpa_device_num++;
> +out_unlock:
> +	rte_spinlock_unlock(&vdpa_device_list_lock);
>  
>  	return dev;
>  }
>  
>  int
> -rte_vdpa_unregister_device(struct rte_vdpa_device *vdev)
> +rte_vdpa_unregister_device(struct rte_vdpa_device *dev)
>  {
> -	int i;
> +	struct rte_vdpa_device *cur_dev, *tmp_dev;
> +	int ret = -1;
>  
> -	for (i = 0; i < MAX_VHOST_DEVICE; i++) {
> -		if (vdev != &vdpa_devices[i])
> +	rte_spinlock_lock(&vdpa_device_list_lock);
> +	TAILQ_FOREACH_SAFE(cur_dev, &vdpa_device_list, next, tmp_dev) {
> +		if (dev != cur_dev)
>  			continue;
>  
> -		memset(vdev, 0, sizeof(struct rte_vdpa_device));
> +		TAILQ_REMOVE(&vdpa_device_list, dev, next);
> +		rte_free(dev);
>  		vdpa_device_num--;
> -
> -		return 0;
> +		ret = 0;
> +		break;
>  	}
> +	rte_spinlock_unlock(&vdpa_device_list_lock);
>  
> -	return -1;
> +	return ret;
>  }
>  
>  int
> @@ -246,19 +265,6 @@ rte_vdpa_reset_stats(struct rte_vdpa_device *dev, uint16_t qid)
>  	return dev->ops->reset_stats(dev, qid);
>  }
>  
> -static uint16_t
> -vdpa_dev_to_id(const struct rte_vdpa_device *dev)
> -{
> -	if (dev == NULL)
> -		return MAX_VHOST_DEVICE;
> -
> -	if (dev < &vdpa_devices[0] ||
> -			dev >= &vdpa_devices[MAX_VHOST_DEVICE])
> -		return MAX_VHOST_DEVICE;
> -
> -	return (uint16_t)(dev - vdpa_devices);
> -}
> -
>  static int
>  vdpa_dev_match(struct rte_vdpa_device *dev,
>  	      const struct rte_device *rte_dev)
> @@ -278,24 +284,22 @@ vdpa_find_device(const struct rte_vdpa_device *start, rte_vdpa_cmp_t cmp,
>  		struct rte_device *rte_dev)
>  {
>  	struct rte_vdpa_device *dev;
> -	uint16_t idx;
>  
> -	if (start != NULL)
> -		idx = vdpa_dev_to_id(start) + 1;
> +	rte_spinlock_lock(&vdpa_device_list_lock);
> +	if (start == NULL)
> +		dev = TAILQ_FIRST(&vdpa_device_list);
>  	else
> -		idx = 0;
> -	for (; idx < MAX_VHOST_DEVICE; idx++) {
> -		dev = &vdpa_devices[idx];
> -		/*
> -		 * ToDo: Certainly better to introduce a state field,
> -		 * but rely on ops being set for now.
> -		 */
> -		if (dev->ops == NULL)
> -			continue;
> +		dev = TAILQ_NEXT(start, next);
> +
> +	while (dev != NULL) {
>  		if (cmp(dev, rte_dev) == 0)
> -			return dev;
> +			break;
> +
> +		dev = TAILQ_NEXT(dev, next);
>  	}
> -	return NULL;
> +	rte_spinlock_unlock(&vdpa_device_list_lock);
> +
> +	return dev;
>  }
>  
>  static void *
> 
Acked-by: Adrián Moreno <amorenoz@redhat.com>
-- 
Adrián Moreno


  reply	other threads:[~2020-06-26 14:46 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-26 14:04 [dpdk-dev] [PATCH v3 00/14] vDPA API and framework rework Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 01/14] bus/dpaa: fix null pointer dereference Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 02/14] bus/fslmc: " Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 03/14] vhost: introduce vDPA devices class Maxime Coquelin
2020-06-29 14:48   ` Ferruh Yigit
2020-06-29 16:23     ` Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 04/14] vhost: make vDPA framework bus agnostic Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 05/14] vhost: replace device ID in vDPA ops Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 06/14] vhost: replace vDPA device ID in Vhost Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 07/14] vhost: replace device ID in applications Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 08/14] vhost: remove useless vDPA API Maxime Coquelin
2020-06-29 14:48   ` Ferruh Yigit
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 09/14] vhost: use linked-list for vDPA devices Maxime Coquelin
2020-06-26 14:46   ` Adrian Moreno [this message]
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 10/14] vhost: introduce wrappers for some vDPA ops Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 11/14] examples/vdpa: use new wrappers instead of ops Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 12/14] examples/vdpa: remove useless device count Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 13/14] vhost: remove vDPA device count API Maxime Coquelin
2020-06-26 14:04 ` [dpdk-dev] [PATCH v3 14/14] vhost: split vDPA header file Maxime Coquelin
2020-06-26 14:47 ` [dpdk-dev] [PATCH v3 00/14] vDPA API and framework rework Adrian Moreno
2020-06-29  9:02   ` Maxime Coquelin
  -- strict thread matches above, loose matches on Subject: below --
2020-06-26 13:26 Maxime Coquelin
2020-06-26 13:27 ` [dpdk-dev] [PATCH v3 09/14] vhost: use linked-list for vDPA devices Maxime Coquelin
2020-06-26 13:56   ` Maxime Coquelin

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=a674e2d1-7f6f-c765-28a1-5a7533091ced@redhat.com \
    --to=amorenoz@redhat.com \
    --cc=chenbo.xia@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=grive@u256.net \
    --cc=hemant.agrawal@nxp.com \
    --cc=matan@mellanox.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=sachin.saxena@nxp.com \
    --cc=viacheslavo@mellanox.com \
    --cc=xiao.w.wang@intel.com \
    --cc=zhihong.wang@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).