[v3,4/6] net/virtio-user: lock-protect status updates

Message ID 20201026163930.94032-5-amorenoz@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series net/virtio-user: fix server mode |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Adrian Moreno Oct. 26, 2020, 4:39 p.m. UTC
  In order to safely set and get the device status from different
threads (e.g: interrupt handlers).

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 14 +++++++++++---
 drivers/net/virtio/virtio_user/virtio_user_dev.h |  4 ++--
 drivers/net/virtio/virtio_user_ethdev.c          |  6 +++---
 3 files changed, 16 insertions(+), 8 deletions(-)
  

Comments

Maxime Coquelin Oct. 28, 2020, 10:35 a.m. UTC | #1
On 10/26/20 5:39 PM, Adrian Moreno wrote:
> In order to safely set and get the device status from different
> threads (e.g: interrupt handlers).
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 14 +++++++++++---
>  drivers/net/virtio/virtio_user/virtio_user_dev.h |  4 ++--
>  drivers/net/virtio/virtio_user_ethdev.c          |  6 +++---
>  3 files changed, 16 insertions(+), 8 deletions(-)

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
  

Patch

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 5a1e76006..36e5619df 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -806,11 +806,13 @@  virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx)
 }
 
 int
-virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
+virtio_user_dev_set_status(struct virtio_user_dev *dev, uint8_t status)
 {
 	int ret;
 	uint64_t arg = status;
 
+	pthread_mutex_lock(&dev->mutex);
+	dev->status = status;
 	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
 		ret = dev->ops->send_request(dev,
 				VHOST_USER_SET_STATUS, &arg);
@@ -824,22 +826,26 @@  virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
 		PMD_INIT_LOG(ERR, "VHOST_USER_SET_STATUS failed (%d): %s", ret,
 			     strerror(errno));
 	}
+
+	pthread_mutex_unlock(&dev->mutex);
 	return ret;
 }
 
 int
-virtio_user_update_status(struct virtio_user_dev *dev)
+virtio_user_dev_update_status(struct virtio_user_dev *dev)
 {
 	uint64_t ret;
 	uint8_t status;
 	int err;
 
+	pthread_mutex_lock(&dev->mutex);
 	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {
 		err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret);
 		if (!err && ret > UINT8_MAX) {
 			PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS "
 					"response 0x%" PRIx64 "\n", ret);
-			return -1;
+			err = -1;
+			goto error;
 		}
 
 		status = ret;
@@ -873,5 +879,7 @@  virtio_user_update_status(struct virtio_user_dev *dev)
 			     strerror(errno));
 	}
 
+error:
+	pthread_mutex_unlock(&dev->mutex);
 	return err;
 }
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 998986875..e053897d8 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -81,7 +81,7 @@  void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
 void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
 				  uint16_t queue_idx);
 uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
-int virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status);
-int virtio_user_update_status(struct virtio_user_dev *dev);
+int virtio_user_dev_set_status(struct virtio_user_dev *dev, uint8_t status);
+int virtio_user_dev_update_status(struct virtio_user_dev *dev);
 extern const char * const virtio_user_backend_strings[];
 #endif
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index e870fb2ff..97ddc5651 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -278,8 +278,8 @@  virtio_user_set_status(struct virtio_hw *hw, uint8_t status)
 		virtio_user_start_device(dev);
 	else if (status == VIRTIO_CONFIG_STATUS_RESET)
 		virtio_user_reset(hw);
-	dev->status = status;
-	virtio_user_send_status_update(dev, status);
+
+	virtio_user_dev_set_status(dev, status);
 }
 
 static uint8_t
@@ -287,7 +287,7 @@  virtio_user_get_status(struct virtio_hw *hw)
 {
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
-	virtio_user_update_status(dev);
+	virtio_user_dev_update_status(dev);
 
 	return dev->status;
 }