[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCH 6/6] lib/ukblkdev: Stop and release an Unikraft block device
On 14.06.19 14:17, Simon Kuenzer wrote:
On 29.05.19 10:34, Roxana Nicolescu wrote:
A device can be stopped, which will not permit further requests.
Also, for closing the device, the device must pe stopped,
each queue must be release and all the data allocated must be freed.
Signed-off-by: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
---
lib/ukblkdev/blkdev.c | 85
+++++++++++++++++++++++++++++++++
lib/ukblkdev/exportsyms.uk | 4 ++
lib/ukblkdev/include/uk/blkdev.h | 31 ++++++++++++
lib/ukblkdev/include/uk/blkdev_core.h | 13 +++++
lib/ukblkdev/include/uk/blkdev_driver.h | 11 +++++
5 files changed, 144 insertions(+)
diff --git a/lib/ukblkdev/blkdev.c b/lib/ukblkdev/blkdev.c
index 534a9ae1..d06b5250 100644
--- a/lib/ukblkdev/blkdev.c
+++ b/lib/ukblkdev/blkdev.c
@@ -467,3 +467,88 @@ int uk_blkdev_sync_io(struct uk_blkdev *dev,
return req->result_status;
}
#endif
+
+int uk_blkdev_stop(struct uk_blkdev *dev)
+{
+ int rc = 0;
+
+ UK_ASSERT(dev);
+ UK_ASSERT(dev->_data);
+ UK_ASSERT(dev->dev_ops);
+ UK_ASSERT(dev->dev_ops->dev_stop);
+ UK_ASSERT(dev->_data->state == UK_BLKDEV_RUNNING);
+
+ rc = dev->dev_ops->dev_stop(dev);
+ if (rc)
+ uk_pr_err("Failed to stop blkdev%"PRIu16" device %d\n",
+ dev->_data->id, rc);
+ else {
+ uk_pr_info("Stopped blkdev%"PRIu16" device\n",
+ dev->_data->id);
+ dev->_data->state = UK_BLKDEV_CONFIGURED;
+ }
+
+ return rc;
+}
+
+void uk_blkdev_queue_release(struct uk_blkdev *dev, uint16_t queue_id)
+{
+ int rc = 0;
+
+ UK_ASSERT(dev != NULL);
+ UK_ASSERT(dev->_data);
+ UK_ASSERT(dev->dev_ops);
+ UK_ASSERT(dev->dev_ops->queue_release);
+ UK_ASSERT(queue_id < CONFIG_LIBUKBLKDEV_MAXNBQUEUES);
+ UK_ASSERT(dev->_data->state != UK_BLKDEV_RUNNING);
+
+#if CONFIG_LIBUKBLKDEV_DISPATCHERTHREADS
+ if (dev->_data->queue_handler[queue_id].callback)
+ _destroy_event_handler(&dev->_data->queue_handler[queue_id]);
Hum not sure if there is a condition where this could crash. Maybe you
should get the queue thread into an defined state and destroy it there.
I am just thinking what happens when it is getting destroyed while
executing the registered event callback. Maybe we need a condition
variable next to the event semaphore which is checked if the thread
should exit by itself.
At least we should put a /* TODO */ comment here because we may run into
problems especially with preemptive scheduling. It is okay to solve this
problem later.
+#endif
+
+ rc = dev->dev_ops->queue_release(dev, queue_id);
Use `struct uk_blkdev_queue *` also here as commented in patch 4/6.
+ if (rc)
+ uk_pr_err("Failed to release blkdev%"PRIu16"-q%"PRIu16": %d\n",
+ dev->_data->id, queue_id, rc);
+ else {
+ uk_pr_info("Released blkdev%"PRIu16"-q%"PRIu16"\n",
+ dev->_data->id, queue_id);
+
+ dev->_queue[queue_id] = NULL;
+ }
+}
+
+void uk_blkdev_drv_unregister(struct uk_blkdev *dev, struct uk_alloc *a)
+{
+ uint16_t id;
+
+ UK_ASSERT(dev != NULL);
+ UK_ASSERT(dev->_data);
+ UK_ASSERT(dev->_data->state != UK_BLKDEV_RUNNING);
+
+ id = dev->_data->id;
+
+ uk_free(a, dev->_data);
Maybe it is better to store `uk_alloc *a` on `_data` while allocating.
Then we would not need the allocator function argument anymore.
+ UK_TAILQ_REMOVE(&uk_blkdev_list, dev, _list);
+ blkdev_count--;
+
+ uk_pr_info("Unregistered blkdev%"PRIu16": %p\n",
+ id, dev);
+}
+
+void uk_blkdev_close(struct uk_blkdev *dev)
+{
+ uint16_t id;
+
+ UK_ASSERT(dev);
+ UK_ASSERT(dev->_data);
+ UK_ASSERT(dev->dev_ops);
+ UK_ASSERT(dev->dev_ops->dev_close);
+ UK_ASSERT(dev->_data->state != UK_BLKDEV_RUNNING);
+
+ id = dev->_data->id;
+ dev->dev_ops->dev_close(dev
Hum, maybe calling this function uk_blkdev_unconfigure() would be a
better fit (and unconfigure for the callback).
+
+ uk_pr_info("Closed blkdev%"PRIu16"\n", id);
+}
diff --git a/lib/ukblkdev/exportsyms.uk b/lib/ukblkdev/exportsyms.uk
index 0274dae8..0abd8e10 100644
--- a/lib/ukblkdev/exportsyms.uk
+++ b/lib/ukblkdev/exportsyms.uk
@@ -22,3 +22,7 @@ uk_blkdev_queue_finish_reqs
uk_blkdev_sync_io
uk_blkdev_read
uk_blkdev_write
+uk_blkdev_stop
+uk_blkdev_queue_release
+uk_blkdev_drv_unregister
+uk_blkdev_close
diff --git a/lib/ukblkdev/include/uk/blkdev.h
b/lib/ukblkdev/include/uk/blkdev.h
index ba4c2784..00789198 100644
--- a/lib/ukblkdev/include/uk/blkdev.h
+++ b/lib/ukblkdev/include/uk/blkdev.h
@@ -468,6 +468,37 @@ int uk_blkdev_sync_io(struct uk_blkdev *dev,
#endif
+/**
+ * Stop an Unikraft block device, and set its state to
UK_BLKDEV_CONFIGURED
+ * state.
+ * The device can be restarted with a call to uk_blkdev_start().
+ *
+ * @param dev
+ * The Unikraft Block Device.
+ */
+int uk_blkdev_stop(struct uk_blkdev *dev);
+
+/**
+ * Free a queue and its descriptors for an Unikraft block device.
+ * @param dev
+ * The Unikraft Block Device.
+ * @param queue_id
+ * The index of the queue to set up.
+ * The value must be in range [0, nb_queue -1] previously supplied
+ * to uk_blkdev_configure()
+ */
+void uk_blkdev_queue_release(struct uk_blkdev *dev, uint16_t queue_id);
Use `struct uk_blkdev_queue *` also here.
+
+/**
+ * Close a stopped Unikraft block device.
+ * The function frees all resources except for
+ * the ones needed by the UK_BLKDEV_UNCONFIGURED state.
+ *
+ * @param dev
+ * The Unikraft Block Device.
+ */
+void uk_blkdev_close(struct uk_blkdev *dev);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/ukblkdev/include/uk/blkdev_core.h
b/lib/ukblkdev/include/uk/blkdev_core.h
index 66831f90..71fdd34b 100644
--- a/lib/ukblkdev/include/uk/blkdev_core.h
+++ b/lib/ukblkdev/include/uk/blkdev_core.h
@@ -204,6 +204,16 @@ typedef int
(*uk_blkdev_queue_submit_one_t)(struct uk_blkdev *dev,
typedef int (*uk_blkdev_queue_finish_reqs_t)(struct uk_blkdev *dev,
uint16_t queue_id);
+/** Driver callback type to stop an Unikraft block device. */
+typedef int (*uk_blkdev_stop_t)(struct uk_blkdev *dev);
+
+/** Driver callback type to release a queue of an Unikraft block
device. */
+typedef int (*uk_blkdev_queue_release_t)(struct uk_blkdev *dev,
+ uint16_t queue_id);
Use `struct uk_blkdev_queue *` also here as commented in patch 4/6.
+
+/** Driver callback type to close an Unikraft block device. */
+typedef void (*uk_blkdev_close_t)(struct uk_blkdev *dev);
+
struct uk_blkdev_ops {
uk_blkdev_get_info_t get_info;
uk_blkdev_configure_t dev_configure;
@@ -211,7 +221,10 @@ struct uk_blkdev_ops {
uk_blkdev_queue_configure_t queue_setup;
uk_blkdev_queue_intr_enable_t queue_intr_enable;
uk_blkdev_start_t dev_start;
+ uk_blkdev_stop_t dev_stop;
uk_blkdev_queue_intr_disable_t queue_intr_disable;
+ uk_blkdev_queue_release_t queue_release;
+ uk_blkdev_close_t dev_close;
};
/**
diff --git a/lib/ukblkdev/include/uk/blkdev_driver.h
b/lib/ukblkdev/include/uk/blkdev_driver.h
index 37b5a3e8..f361e8fb 100644
--- a/lib/ukblkdev/include/uk/blkdev_driver.h
+++ b/lib/ukblkdev/include/uk/blkdev_driver.h
@@ -98,6 +98,17 @@ static inline void uk_blkdev_drv_queue_event(struct
uk_blkdev *dev,
#endif
}
+/**
+ * Frees the data allocated for the Unikraft Block Device.
+ * Removes the block device from the list.
+ *
+ * @param dev
+ * Unikraft block device
+ * @param a
+ * Allocator used for libukblkdev private data (dev->_data).
+ */
+void uk_blkdev_drv_unregister(struct uk_blkdev *dev, struct uk_alloc
*a);
+
#ifdef __cplusplus
}
#endif
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
