Skip to content

rtio: MPSC queues for iodevs #51772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 3, 2023
Merged

rtio: MPSC queues for iodevs #51772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a79f0b4
rtio: Remove unused spsc initializer params
teburd Feb 22, 2023
377ec9f
rtio: Add lock free MPSC queue for iodevs and more
teburd Oct 25, 2022
97acb6f
rtio: Shareable lock-free iodevs
teburd Oct 28, 2022
f375265
rtio: Exclude failing renode platform
teburd Feb 22, 2023
ad0631e
rtio: Drop stray printks in API test
teburd Feb 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 39 additions & 64 deletions include/zephyr/rtio/rtio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#define ZEPHYR_INCLUDE_RTIO_RTIO_H_

#include <zephyr/rtio/rtio_spsc.h>
#include <zephyr/rtio/rtio_mpsc.h>
#include <zephyr/sys/__assert.h>
#include <zephyr/sys/atomic.h>
#include <zephyr/device.h>
Expand Down Expand Up @@ -164,6 +165,7 @@ struct rtio_cq {
};

struct rtio;
struct rtio_iodev_sqe;

struct rtio_executor_api {
/**
Expand All @@ -179,12 +181,12 @@ struct rtio_executor_api {
/**
* @brief SQE completes successfully
*/
void (*ok)(struct rtio *r, const struct rtio_sqe *sqe, int result);
void (*ok)(struct rtio_iodev_sqe *iodev_sqe, int result);

/**
* @brief SQE fails to complete
* @brief SQE fails to complete successfully
*/
void (*err)(struct rtio *r, const struct rtio_sqe *sqe, int result);
void (*err)(struct rtio_iodev_sqe *iodev_sqe, int result);
};

/**
Expand Down Expand Up @@ -257,47 +259,32 @@ struct rtio {
struct rtio_cq *cq;
};


/**
* @brief API that an RTIO IO device should implement
* @brief IO device submission queue entry
*/
struct rtio_iodev_api {
/**
* @brief Submission function for a request to the iodev
*
* The iodev is responsible for doing the operation described
* as a submission queue entry and reporting results using using
* `rtio_sqe_ok` or `rtio_sqe_err` once done.
*/
void (*submit)(const struct rtio_sqe *sqe,
struct rtio *r);

/**
* TODO some form of transactional piece is missing here
* where we wish to "transact" on an iodev with multiple requests
* over a chain.
*
* Only once explicitly released or the chain fails do we want
* to release. Once released any pending iodevs in the queue
* should be done.
*
* Something akin to a lock/unlock pair.
*/
};

/* IO device submission queue entry */
struct rtio_iodev_sqe {
struct rtio_mpsc_node q;
const struct rtio_sqe *sqe;
struct rtio *r;
};

/**
* @brief IO device submission queue
*
* This is used for reifying the member of the rtio_iodev struct
* @brief API that an RTIO IO device should implement
*/
struct rtio_iodev_sq {
struct rtio_spsc _spsc;
struct rtio_iodev_sqe buffer[];
struct rtio_iodev_api {
/**
* @brief Submit to the iodev an entry to work on
*
* This call should be short in duration and most likely
* either enqueue or kick off an entry with the hardware.
*
* If polling is required the iodev should add itself to the execution
* context (@see rtio_add_pollable())
*
* @param iodev_sqe Submission queue entry
*/
void (*submit)(struct rtio_iodev_sqe *iodev_sqe);
};

/**
Expand All @@ -308,7 +295,7 @@ struct rtio_iodev {
const struct rtio_iodev_api *api;

/* Queue of RTIO contexts with requests */
struct rtio_iodev_sq *iodev_sq;
struct rtio_mpsc iodev_sq;

/* Data associated with this iodev */
void *data;
Expand Down Expand Up @@ -389,29 +376,17 @@ static inline void rtio_sqe_prep_write(struct rtio_sqe *sqe,
#define RTIO_CQ_DEFINE(name, len) \
RTIO_SPSC_DEFINE(name, struct rtio_cqe, len)


/**
* @brief Statically define and initialize a fixed length iodev submission queue
*
* @param name Name of the queue.
* @param len Queue length, power of 2 required
*/
#define RTIO_IODEV_SQ_DEFINE(name, len) \
RTIO_SPSC_DEFINE(name, struct rtio_iodev_sqe, len)

/**
* @brief Statically define and initialize an RTIO IODev
*
* @param name Name of the iodev
* @param iodev_api Pointer to struct rtio_iodev_api
* @param qsize Size of the submission queue, must be power of 2
* @param iodev_data Data pointer
*/
#define RTIO_IODEV_DEFINE(name, iodev_api, qsize, iodev_data) \
static RTIO_IODEV_SQ_DEFINE(_iodev_sq_##name, qsize); \
const STRUCT_SECTION_ITERABLE(rtio_iodev, name) = { \
#define RTIO_IODEV_DEFINE(name, iodev_api, iodev_data) \
STRUCT_SECTION_ITERABLE(rtio_iodev, name) = { \
.api = (iodev_api), \
.iodev_sq = (struct rtio_iodev_sq *const)&_iodev_sq_##name, \
.iodev_sq = RTIO_MPSC_INIT((name.iodev_sq)), \
.data = (iodev_data), \
}

Expand Down Expand Up @@ -449,14 +424,16 @@ static inline void rtio_set_executor(struct rtio *r, struct rtio_executor *exc)
}

/**
* @brief Perform a submitted operation with an iodev
* @brief Submit to an iodev a submission to work on
*
* @param sqe Submission to work on
* @param r RTIO context
* Should be called by the executor when it wishes to submit work
* to an iodev.
*
* @param iodev_sqe Submission to work on
*/
static inline void rtio_iodev_submit(const struct rtio_sqe *sqe, struct rtio *r)
static inline void rtio_iodev_submit(struct rtio_iodev_sqe *iodev_sqe)
{
sqe->iodev->api->submit(sqe, r);
iodev_sqe->sqe->iodev->api->submit(iodev_sqe);
}

/**
Expand Down Expand Up @@ -571,27 +548,25 @@ static inline void rtio_cqe_release_all(struct rtio *r)
*
* This may start the next asynchronous request if one is available.
*
* @param r RTIO context
* @param sqe Submission that has succeeded
* @param iodev_sqe IODev Submission that has succeeded
* @param result Result of the request
*/
static inline void rtio_sqe_ok(struct rtio *r, const struct rtio_sqe *sqe, int result)
static inline void rtio_iodev_sqe_ok(struct rtio_iodev_sqe *iodev_sqe, int result)
{
r->executor->api->ok(r, sqe, result);
iodev_sqe->r->executor->api->ok(iodev_sqe, result);
}

/**
* @brief Inform the executor of a submissions completion with error
*
* This SHALL fail the remaining submissions in the chain.
*
* @param r RTIO context
* @param sqe Submission that has failed
* @param iodev_sqe Submission that has failed
* @param result Result of the request
*/
static inline void rtio_sqe_err(struct rtio *r, const struct rtio_sqe *sqe, int result)
static inline void rtio_iodev_sqe_err(struct rtio_iodev_sqe *iodev_sqe, int result)
{
r->executor->api->err(r, sqe, result);
iodev_sqe->r->executor->api->err(iodev_sqe, result);
}

/**
Expand Down
16 changes: 7 additions & 9 deletions include/zephyr/rtio/rtio_executor_concurrent.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,18 @@ int rtio_concurrent_submit(struct rtio *r);
/**
* @brief Report a SQE has completed successfully
*
* @param r RTIO context to use
* @param sqe RTIO SQE to report success
* @param sqe RTIO IODev SQE to report success
* @param result Result of the SQE
*/
void rtio_concurrent_ok(struct rtio *r, const struct rtio_sqe *sqe, int result);
void rtio_concurrent_ok(struct rtio_iodev_sqe *sqe, int result);

/**
* @brief Report a SQE has completed with error
*
* @param r RTIO context to use
* @param sqe RTIO SQE to report success
* @param sqe RTIO IODev SQE to report success
* @param result Result of the SQE
*/
void rtio_concurrent_err(struct rtio *r, const struct rtio_sqe *sqe, int result);
void rtio_concurrent_err(struct rtio_iodev_sqe *sqe, int result);

/**
* @brief Concurrent Executor
Expand All @@ -76,8 +74,8 @@ struct rtio_concurrent_executor {
/* Array of task statuses */
uint8_t *task_status;

/* Array of struct rtio_sqe *'s one per task' */
struct rtio_sqe **task_cur;
/* Array of struct rtio_iodev_sqe *'s one per task' */
struct rtio_iodev_sqe *task_cur;
};

/**
Expand All @@ -101,7 +99,7 @@ static const struct rtio_executor_api z_rtio_concurrent_api = {
* @param concurrency Allowed concurrency (number of concurrent tasks).
*/
#define RTIO_EXECUTOR_CONCURRENT_DEFINE(name, concurrency) \
static struct rtio_sqe *_task_cur_##name[(concurrency)]; \
static struct rtio_iodev_sqe _task_cur_##name[(concurrency)]; \
uint8_t _task_status_##name[(concurrency)]; \
static struct rtio_concurrent_executor name = { \
.ctx = { .api = &z_rtio_concurrent_api }, \
Expand Down
11 changes: 5 additions & 6 deletions include/zephyr/rtio/rtio_executor_simple.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,25 @@ int rtio_simple_submit(struct rtio *r);
/**
* @brief Report a SQE has completed successfully
*
* @param r RTIO context to use
* @param sqe RTIO SQE to report success
* @param iodev_sqe RTIO IODEV SQE to report success
* @param result Result of the SQE
*/
void rtio_simple_ok(struct rtio *r, const struct rtio_sqe *sqe, int result);
void rtio_simple_ok(struct rtio_iodev_sqe *iodev_sqe, int result);

/**
* @brief Report a SQE has completed with error
*
* @param r RTIO context to use
* @param sqe RTIO SQE to report success
* @param iodev_sqe RTIO IODEV SQE to report success
* @param result Result of the SQE
*/
void rtio_simple_err(struct rtio *r, const struct rtio_sqe *sqe, int result);
void rtio_simple_err(struct rtio_iodev_sqe *iodev_sqe, int result);

/**
* @brief Simple Executor
*/
struct rtio_simple_executor {
struct rtio_executor ctx;
struct rtio_iodev_sqe task;
};

/**
Copy link
Contributor

@nordic-krch nordic-krch Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment to unchanged content but leaving it anyway.
Imo, this header should only have

struct rtio_simple_executor {
...
};
extern const struct rtio_execture_api z_rtio_simple_api;

And maybe initializer macro that would set proper API.
API variable should be created in c file as currently anyone including this header will get own copy. This way functions can be private in c file.

Copy link
Contributor Author

@teburd teburd Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point, thanks! Will create a PR that fixes this.

Expand Down
Loading