Skip to content
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.

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

core: merge msg into mbox #10399

Closed
miri64 opened this issue Nov 15, 2018 · 4 comments
Closed

core: merge msg into mbox #10399

miri64 opened this issue Nov 15, 2018 · 4 comments
Assignees
Labels
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 State: stale State: The issue / PR has no activity for >185 days Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Comments

@miri64
Copy link
Member

miri64 commented Nov 15, 2018

From my understanding there is some shared functionality between the asynchronous behavior of core_msg and core_mbox: messages are put into a queue by n threads and gotten out n threads. In case of core_msg n is however one.

On one hand side we might get some code size benefits in merging the two, on the other we would put some more complexity into the asynchronous behavior of core_msg, since it isn't required to handle more than one reception thread.

A good compromise in my opinion would be to provide an new mbox implementation that assumes only one reader and base the old mbox and msg implementation on it. I started something into that direction in this branch. But I

  1. did not port msg yet
  2. am not sure if this

    RIOT/core/mbox_base.c

    Lines 63 to 66 in 1631d29

    if (blocking) {
    _mbox_base_wait(&mbox->writers, irqstate);
    irqstate = irq_disable();
    }
    and that

    RIOT/core/mbox_base.c

    Lines 92 to 96 in 1631d29

    if (next) {
    thread_t *thread = container_of((clist_node_t*)next, thread_t,
    rq_entry);
    _mbox_base_notify(thread, irqstate);
    }
    is still possible, as both _mbox_base_put() and mbox_base_get() would be called by the wrapper implementations of mbox and msg, both might have their own irqstate stored (and thus it might not properly return to the right IRQ state in both _mbox_base_wait() and _mbox_base_notify().
@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation 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 labels Nov 15, 2018
@kaspar030
Copy link
Contributor

I had an msg implementation that used mbox below. I didn't PR it because plain msg was simply faster. Whatever you come up with, try bench_msg() on real hardware.

@miri64
Copy link
Member Author

miri64 commented Nov 15, 2018

@kaspar030 any thoughts on 2.?

@miri64
Copy link
Member Author

miri64 commented Dec 18, 2018

Ping @kaspar030?

@stale
Copy link

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019
@stale stale bot closed this as completed Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 State: stale State: The issue / PR has no activity for >185 days Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

No branches or pull requests

2 participants