sys: Add a lockfree mpsc and spsc queues #63470

teburd · 2023-10-03T16:01:32Z

Moves the rtio_ prefixed lockfree queues to sys alongside existing
mpsc/spsc pbuf, ringbuf, and similar queue-like data structures.

teburd · 2023-10-04T15:06:28Z

@mgielda can you please take a look see, unclear why this test would take so much longer in renode (I'm only vaguely familiar with it) than qemu.

nashif · 2023-10-12T17:36:57Z

It will get confusing with the mpsc stuff and what we have already in include/zephyr/sys/mpsc_pbuf.h, probably need a better structure for all of this, did not look at content, just the file names :)

teburd · 2023-10-12T17:42:32Z

It will get confusing with the mpsc stuff and what we have already in include/zephyr/sys/mpsc_pbuf.h, probably need a better structure for all of this, did not look at content, just the file names :)

Possibly, this was actually asked for awhile ago by @nordic-krch who also authored mpsc_pbuf I believe #57436

nordic-krch · 2023-10-23T12:04:02Z

I'm not sure what to suggest but it would be good to organize it in a way that it is clear for the user. Currently we have:

mpsc - which is a fifo list of user data. It just organizes user data. more like slist but thread safe.
spsc - which is a container for storing fixed size data (up to n -elements). Zero copy approach (acquire, produce....)
spsc_pbuf - similar to spsc but hold variable length data (packets thus packet buffer pbuf). Zero copy approach.
mpsc_pbuf - more or less the same as spsc_pbuf but more complex since support multiproducer. Zero copy approach.
ring_buffer - also container but targeting raw data (no packets).
...
IMO, it would be more intuitive to divide it into lists (of pointers) and buffers (containers). From user perspective it's not important if it is lockfree or not but what can it do (organize my data vs store the data).

josuah · 2024-05-28T10:28:32Z

doc/services/rtio/index.rst

Removing these categories from RTIO pages help with understanding RTIO: MPSC and SPSC were looking like part of the API, but are only used internally rather than two variants of RTIO.

henrikbrixandersen · 2024-05-29T06:43:06Z

Please rebase on main and resolve any merge conflicts.

teburd · 2024-05-29T14:22:09Z

@henrikbrixandersen CI failing now on an unrelated doc build error again it seems

ubieda

CI failures are unrelated to this PR. I'm able to reproduce it on main. Hence I'm approving this PR.

I'll report it on a GH issue to keep track of it.

ubieda · 2024-06-01T17:21:03Z

Issue captured on #73606

Running lock free algorithms on renode seem to cause timeouts so exclude renode platforms. Signed-off-by: Tom Burdick <[email protected]>

Moves the rtio_ prefixed lockfree queues to sys alongside existing mpsc/spsc pbuf, ringbuf, and similar queue-like data structures. Signed-off-by: Tom Burdick <[email protected]>

The doc comment relating to mpsc atomics was worded poorly. Remove the poorly worded doc comment related to atomics and caches. Signed-off-by: Tom Burdick <[email protected]>

josuah

note: I am only contributor, not collaborator, not maintainer, not a TSC

Now it is clearly visible that RTIO does not have SPSC as a dependency, reducing the learning curve.

josuah · 2024-06-04T16:34:15Z

tests/lib/lockfree/src/test_mpsc.c

+#include <zephyr/sys/spsc_lockfree.h>
+#include <zephyr/sys/mpsc_lockfree.h>


The fact that they are both called _lockfree.h helps grouping them semantically, and suggest the kind of feature they provide.

josuah · 2024-06-04T16:38:48Z

include/zephyr/sys/mpsc_lockfree.h

+#define MPSC_INIT(symbol)                                                                          \
 	{                                                                                          \
-		.head = (struct rtio_mpsc_node *)&symbol.stub,                                     \
-		.tail = (struct rtio_mpsc_node *)&symbol.stub,                                     \
+		.head = (struct mpsc_node *)&symbol.stub,                                          \
+		.tail = (struct mpsc_node *)&symbol.stub,                                          \
 		.stub = {                                                                          \
 			.next = NULL,                                                              \
 		},                                                                                 \


In case consistency with SPSC and other few Zephyr APIs was aimed, MPSC_INITIALIZER() is also possible

https://github.com/zephyrproject-rtos/zephyr/pull/63470/files#diff-690e89323e13a9e22d9b6744504481620011234df581bbab923629217e0c529dR82

But it was already like that in RTIO, and seems like mostly for internal use anyway.

May I ask why there is no MPSC_DEFINE()?

The use-case might have been limited to these infocation, and never be defined alone outside of RTIO context. Now someone might want to define a global MPSC queue.

https://github.com/zephyrproject-rtos/zephyr/pull/63470/files#diff-a0e010388d2114551593a77dfc3d6a51c2714c375411f3206af13cf6fee084f2R800-R801

Though, not hard to call struct mpsc my_mpsc_queue = MSPC_INIT(my_mpsc_queue); over MPSC_DEFINE(my_mpsc_queue);. Just one fewer repetition of the symbol name...

I think it'd be a nice addition in a follow up, having rebased this and reworked it so many times now, I'd really like to see @nordic-krch re-approve and move it in before another mega rebase needs to happen again (causing lots of work).

josuah · 2024-06-04T16:40:52Z

include/zephyr/sys/spsc_lockfree.h

@@ -1,12 +1,11 @@
 /*
- * Copyright (c) 2022 Intel Corporation
+ * Copyright (c) 2023 Intel Corporation


2024? Time flies!

Sad but true

teburd requested review from henrikbrixandersen, nordic-krch and yperess October 3, 2023 16:07

teburd force-pushed the lockfree_queues branch 2 times, most recently from 28fd29b to 1ae34c1 Compare October 4, 2023 02:19

teburd force-pushed the lockfree_queues branch from 6cd6709 to 89f36a4 Compare October 12, 2023 16:54

teburd marked this pull request as ready for review October 13, 2023 16:25

teburd requested review from MaureenHelm, carlescufi, galak, nashif and tbursztyka as code owners October 13, 2023 16:25

teburd added this to the v3.6.0 milestone Oct 13, 2023

zephyrbot added area: SPI SPI bus area: C++ area: I2C area: RTIO platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) labels Oct 13, 2023

zephyrbot requested review from alexanderwachter, attie-argentum, cfriedt, mnkp, nandojve, pdgendt and stephanosio October 13, 2023 16:26

zephyrbot assigned teburd Oct 13, 2023

nordic-krch previously approved these changes May 28, 2024

View reviewed changes

josuah reviewed May 28, 2024

View reviewed changes

ubieda previously approved these changes May 28, 2024

View reviewed changes

teburd dismissed stale reviews from ubieda and nordic-krch via ff8d099 May 29, 2024 13:20

teburd force-pushed the lockfree_queues branch from 29daab6 to ff8d099 Compare May 29, 2024 13:20

ubieda previously approved these changes May 29, 2024

View reviewed changes

teburd dismissed ubieda’s stale review via c0c51b1 May 29, 2024 14:56

teburd force-pushed the lockfree_queues branch from ff8d099 to c0c51b1 Compare May 29, 2024 14:56

teburd added this to the v3.7.0 milestone May 30, 2024

teburd force-pushed the lockfree_queues branch from c0c51b1 to d59c3cd Compare May 30, 2024 14:33

zephyrbot added the area: Kernel label May 30, 2024

teburd force-pushed the lockfree_queues branch 3 times, most recently from 5c32393 to 475d913 Compare June 1, 2024 15:00

ubieda approved these changes Jun 1, 2024

View reviewed changes

teburd force-pushed the lockfree_queues branch from 475d913 to 72fe378 Compare June 3, 2024 20:18

teburd added 3 commits June 3, 2024 15:18

rtio: Exclude platforms from CI testing

e8bc3c5

Running lock free algorithms on renode seem to cause timeouts so exclude renode platforms. Signed-off-by: Tom Burdick <[email protected]>

sys: Add a lockfree mpsc and spsc queues

6fbb316

Moves the rtio_ prefixed lockfree queues to sys alongside existing mpsc/spsc pbuf, ringbuf, and similar queue-like data structures. Signed-off-by: Tom Burdick <[email protected]>

sys: MPSC doc comment cleanup

72fe378

The doc comment relating to mpsc atomics was worded poorly. Remove the poorly worded doc comment related to atomics and caches. Signed-off-by: Tom Burdick <[email protected]>

teburd requested review from josuah and nordic-krch June 4, 2024 13:56

josuah approved these changes Jun 4, 2024

View reviewed changes

ubieda mentioned this pull request Jun 5, 2024

Contributor Nomination (josuah) #73815

Closed

nordic-krch approved these changes Jun 6, 2024

View reviewed changes

carlescufi merged commit c5e591b into zephyrproject-rtos:main Jun 6, 2024

teburd deleted the lockfree_queues branch June 7, 2024 13:27

		#include <zephyr/sys/spsc_lockfree.h>
		#include <zephyr/sys/mpsc_lockfree.h>

sys: Add a lockfree mpsc and spsc queues #63470

sys: Add a lockfree mpsc and spsc queues #63470

Uh oh!

Conversation

teburd commented Oct 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teburd commented Oct 4, 2023

Uh oh!

nashif commented Oct 12, 2023

Uh oh!

teburd commented Oct 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nordic-krch commented Oct 23, 2023

Uh oh!

josuah May 28, 2024

Choose a reason for hiding this comment

Uh oh!

henrikbrixandersen commented May 29, 2024

Uh oh!

teburd commented May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ubieda left a comment

Choose a reason for hiding this comment

Uh oh!

ubieda commented Jun 1, 2024

Uh oh!

josuah left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josuah Jun 4, 2024

Choose a reason for hiding this comment

Uh oh!

josuah Jun 4, 2024

Choose a reason for hiding this comment

Uh oh!

josuah Jun 4, 2024

Choose a reason for hiding this comment

Uh oh!

teburd Jun 5, 2024

Choose a reason for hiding this comment

Uh oh!

josuah Jun 4, 2024

Choose a reason for hiding this comment

Uh oh!

teburd Jun 4, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

teburd commented Oct 3, 2023 •

edited

Loading

teburd commented Oct 12, 2023 •

edited

Loading

teburd commented May 29, 2024 •

edited

Loading

josuah left a comment •

edited

Loading