Skip to content

gnrc_ipv6_nib: use recursive mutex instead of mutex for locking#12425

Merged
miri64 merged 4 commits intoRIOT-OS:masterfrom
miri64:gnrc_ipv6_nib/enh/mutex
Oct 21, 2019
Merged

gnrc_ipv6_nib: use recursive mutex instead of mutex for locking#12425
miri64 merged 4 commits intoRIOT-OS:masterfrom
miri64:gnrc_ipv6_nib/enh/mutex

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Oct 11, 2019

Contribution description

This should make the problem that popped up in #12408 easier to solve. There, since the source address detection accesses the NIB's prefix list interface now to determine the prefix lengths of given addresses. This deadlocks its internal process, however, since this function is also called by the neighbor solicitation send function from within the NIB. With a recursive mutex this can't happen.

I also made the mutex itself private and implemented two functions two lock/unlock it from the outside, which scraped of some 100 bytes of ROM on Cortex M0+.

Testing procedure

make -C tests/gnrc_ipv6_nib flash test

and

make -C tests/gnrc_ipv6_nib_6ln flash test

still pass, pinging between two gnrc_networking instances works.

Issues/PRs references

Preparation for a rework of #12408.

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 11, 2019
@miri64 miri64 added this to the Release 2020.01 milestone Oct 11, 2019
@miri64 miri64 requested a review from kaspar030 October 11, 2019 11:53
@miri64 miri64 mentioned this pull request Oct 11, 2019
Merged
@fjmolinas fjmolinas mentioned this pull request Oct 21, 2019
Closed
80 tasks
@miri64 miri64 requested a review from a team October 21, 2019 12:50
@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented Oct 21, 2019

The first thee commits might as well be one.
Unless there ever is a case where the IPv6 thread intentionally blocks on itself, this should not change the behavior of the current code.

As I'm not sure if this is the case (thread blocks on mutex_lock() to be woken by another thread) I'm not ACKing yet - but if you reassure me no such construct is used, this should be good.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Oct 21, 2019

The first thee commits might as well be one.
Unless there ever is a case where the IPv6 thread intentionally blocks on itself, this should not change the behavior of the current code.

That is the problem in #12408.

As I'm not sure if this is the case (thread blocks on mutex_lock() to be woken by another thread) I'm not ACKing yet - but if you reassure me no such construct is used, this should be good.

No, that is not the point of that mutex. It is just for access protection (since the NIB can also be accessed e.g. by the shell or the application).

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Oct 21, 2019

The first thee [sic] commits might as well be one.

Nah, this makes the change more comprehensible.

benpicco
benpicco approved these changes Oct 21, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

The mutex is only used for mutual exclusion and not for signalling, so replacing it with a recursive variant is safe.

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 21, 2019
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Oct 21, 2019

Restarting Murdock, as the last build was quite old.

@miri64 miri64 merged commit 1bdfbea into RIOT-OS:master Oct 21, 2019
@miri64 miri64 deleted the gnrc_ipv6_nib/enh/mutex branch October 21, 2019 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco approved these changes

@kaspar030 kaspar030 Awaiting requested review from kaspar030

Assignees

No one assigned

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2020.01

Development

Successfully merging this pull request may close these issues.

2 participants

@miri64 @benpicco