RE: [PATCH v2 11/20] net/virtio: annotate lock for guest announce

["Xia, Chenbo" <chenbo.xia@intel.com>]

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, February 24, 2023 11:12 PM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; Maxime Coquelin <maxime.coquelin@redhat.com>; Xia,
> Chenbo <chenbo.xia@intel.com>
> Subject: [PATCH v2 11/20] net/virtio: annotate lock for guest announce
> 
> Expose requirements for helpers dealing with the
> VIRTIO_DEV_TO_HW(dev)->state_lock lock.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 8 ++++----
>  drivers/net/virtio/virtio_ethdev.h | 7 +++++--
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index 0103d95920..a3de44958c 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1149,11 +1149,11 @@ virtio_dev_pause(struct rte_eth_dev *dev)
>  {
>  	struct virtio_hw *hw = dev->data->dev_private;
> 
> -	rte_spinlock_lock(&hw->state_lock);
> +	rte_spinlock_lock(&VIRTIO_DEV_TO_HW(dev)->state_lock);
> 
>  	if (hw->started == 0) {
>  		/* Device is just stopped. */
> -		rte_spinlock_unlock(&hw->state_lock);
> +		rte_spinlock_unlock(&VIRTIO_DEV_TO_HW(dev)->state_lock);
>  		return -1;
>  	}
>  	hw->started = 0;
> @@ -1174,7 +1174,7 @@ virtio_dev_resume(struct rte_eth_dev *dev)
>  	struct virtio_hw *hw = dev->data->dev_private;
> 
>  	hw->started = 1;
> -	rte_spinlock_unlock(&hw->state_lock);
> +	rte_spinlock_unlock(&VIRTIO_DEV_TO_HW(dev)->state_lock);
>  }
> 
>  /*
> @@ -1217,7 +1217,7 @@ virtio_notify_peers(struct rte_eth_dev *dev)
>  	}
> 
>  	/* If virtio port just stopped, no need to send RARP */
> -	if (virtio_dev_pause(dev) < 0) {
> +	if (virtio_dev_pause(dev) != 0) {
>  		rte_pktmbuf_free(rarp_mbuf);
>  		return;
>  	}
> diff --git a/drivers/net/virtio/virtio_ethdev.h
> b/drivers/net/virtio/virtio_ethdev.h
> index c08f382791..ece0130603 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -112,8 +112,11 @@ int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
> 
>  void virtio_interrupt_handler(void *param);
> 
> -int virtio_dev_pause(struct rte_eth_dev *dev);
> -void virtio_dev_resume(struct rte_eth_dev *dev);
> +#define VIRTIO_DEV_TO_HW(dev) ((struct virtio_hw *)(dev)->data-
> >dev_private)
> +int virtio_dev_pause(struct rte_eth_dev *dev)
> +	__rte_exclusive_trylock_function(0, &VIRTIO_DEV_TO_HW(dev)-
> >state_lock);

Just curious, why this is trylock instead of lock?

Thanks,
Chenbo

> +void virtio_dev_resume(struct rte_eth_dev *dev)
> +	__rte_unlock_function(&VIRTIO_DEV_TO_HW(dev)->state_lock);
>  int virtio_dev_stop(struct rte_eth_dev *dev);
>  int virtio_dev_close(struct rte_eth_dev *dev);
>  int virtio_inject_pkts(struct rte_eth_dev *dev, struct rte_mbuf **tx_pkts,
> --
> 2.39.2