Skip to content

mbox: initial import of a thread-independent IPC#4552

Closed
miri64 wants to merge 2 commits intoRIOT-OS:masterfrom
miri64:mbox/feat/initial
Closed

mbox: initial import of a thread-independent IPC#4552
miri64 wants to merge 2 commits intoRIOT-OS:masterfrom
miri64:mbox/feat/initial

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Dec 27, 2015

Introduction

I know @kaspar030 is working on something like this too, but when I was working with lwIP again, I decided to port out the messaging layer I implemented for it, to better test it and to give it as an alternative to @kaspar030's work (and since it was already laying around in my repo it took me only half a day and some hours of boredom on Christmas to port it to msg_t).

mbox is thought to be an alternative - not a replacement - to standard-IPC. There are probably many ways to make it a little bit smaller in memory consumption, but I based it on the unix support for mbox of lwIP (with many adaptations to make it more RIOT-y) so it is quite stable.

Summary

mbox is a datatype with two base operations (of that there are some variations - more on that later):

  • mbox_put() and
  • mbox_get()

Blocking IPC

mbox_put() allows a user to put a msg_t in the mbox. If the mbox is full the thread will block and be signaled through the mbox::not_full semaphore, when there is space in the mbox again.

mbox_get() allows a user to get a msg_t from the mbox. If the mbox is empty the thread will block and be signaled through the mbox::not_empty semaphore, when another thread puts something in the mbox.

Non-blocking IPC

There is a non-blocking variant mbox_tryget() and mbox_tryput() for both operations, that just returns -EAGAIN when the normal operations would block (they don't require the semaphores, though they both post them, so the blocking variants are woken up).

mbox and time

Since the blocking mechanism is based on semaphores there is also a timeout-variant of mbox_get(): mbox_get_timed(). If we make this optional for semaphores we also can make it optional here.

Dependencies

Depends on #4551 and maybe #4374 (though I did not run into any problems during tests, regarding this bug).

@miri64 miri64 added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Dec 27, 2015
@miri64 miri64 added this to the Release 2016.03 milestone Dec 27, 2015
@miri64 miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 27, 2015
@OlegHahm
Copy link
Copy Markdown
Member

Sorry, I don't have time to review this.

@OlegHahm OlegHahm assigned kaspar030 and unassigned OlegHahm Dec 27, 2015
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 28, 2015

@Kijewski since you have some experience with semaphores also, you might want to have a look too.

@kaspar030
Copy link
Copy Markdown
Contributor

While I like the simplicity of the approach, it doesn't tackle some of the IMHO important aspects:

  • it cannot be used from within an ISR
  • it's kinda fat memory-wise (sizeof(mbox_t) = 44)
  • it's kinda slow (about a third of the speed of our current messaging)
  • it doesn't allow waiting on multiple queues
  • (it doesn't allow arbitrarily sized messages, but that's maybe not needed)

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 29, 2015

it cannot be used from within an ISR

Could be fixed easily IMHO

it's kinda fat memory-wise (sizeof(mbox_t) = 44)

As I mentioned in the description: there is room for optimization ;)

it's kinda slow (about a third of the speed of our current messaging)

Since semaphores are based on the current messaging this doesn't surprise me really

it doesn't allow waiting on multiple queues

Was that something we wanted? This should be adaptable easily, but it wouldn't make it stronger in the points you already gave.

(it doesn't allow arbitrarily sized messages, but that's maybe not needed)

What do you mean by that?

@kaspar030
Copy link
Copy Markdown
Contributor

@authmillenon Well, devising the API (mbox_put(), mbox_get(), ...) is simple, the other points make it hard ;)

(it doesn't allow arbitrarily sized messages, but that's maybe not needed)

What do you mean by that?

e.g.

mbox_init(mbox, num, size);
mbox_put(mbox, obj, size);

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 29, 2015

Except for the fixed size for the message box this is exactly how it was done in lwIP (with void pointers). I'm not sure however, how this could put to a better use, than a fixed msg_t already can.

@OlegHahm OlegHahm added Area: core Area: RIOT kernel. Handle PRs marked with this with care! Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR and removed State: waiting for other PR State: The PR requires another PR to be merged first Area: core Area: RIOT kernel. Handle PRs marked with this with care! labels Feb 27, 2016
@OlegHahm
Copy link
Copy Markdown
Member

Needs a rebase, but is no longer waiting on anything.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Feb 28, 2016

Rebased

SEMA_CREATE(1), SEMA_CREATE(0), 0 }

/**
* @brief Static initializer for message box.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Static?

@kaspar030 kaspar030 mentioned this pull request Feb 28, 2016
1 task
@kaspar030
Copy link
Copy Markdown
Contributor

I have #4919 lying around, please consider that.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Feb 29, 2016

I still need to test #4919, but I already know that I will prefer it => close.

@miri64 miri64 closed this Feb 29, 2016
@miri64 miri64 deleted the mbox/feat/initial branch February 29, 2016 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants