Skip to content

core: mutexes memorize holding thread id #4555

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

core: mutexes memorize holding thread id #4555

wants to merge 4 commits into from

Conversation

Kijewski
Copy link
Contributor

Rationale: see #4494. Closes #4494.

examples/default:

text    data    bss     dec     BOARD/BINDIRBASE

-12     0       0       -12     airfy-beacon
22912   164     10316   33392   old
22900   164     10316   33380   new

-8      0       0       -8      arduino-due
12244   168     2648    15060   old
12236   168     2648    15052   new

-32     0       0       -32     arduino-mega2560
11200   1566    719     13485   old
11168   1566    719     13453   new

-32     0       0       -32     avsextrem
60588   2356    95945   158889  old
60556   2356    95945   158857  new

-8      0       0       -8      cc2538dk
12344   164     2620    15128   old
12336   164     2620    15120   new

0       0       0       0       chronos
14838   106     1042    15986   old
14838   106     1042    15986   new

-8      0       0       -8      ek-lm4f120xl
12572   164     2620    15356   old
12564   164     2620    15348   new

-16     0       0       -16     f4vi1
14296   168     2760    17224   old
14280   168     2760    17208   new

-4      0       0       -4      fox
34364   164     11684   46212   old
34360   164     11684   46208   new

-8      0       0       -8      frdm-k64f
19808   1268    2684    23760   old
19800   1268    2684    23752   new

-4      0       0       -4      iotlab-m3
34844   164     11772   46780   old
34840   164     11772   46776   new

-8      0       0       -8      limifrog-v1
13908   164     2756    16828   old
13900   164     2756    16820   new

-8      0       0       -8      mbed_lpc1768
12336   164     2628    15128   old
12328   164     2628    15120   new

-32     0       0       -32     msb-430
20114   34      1082    21230   old
20082   34      1082    21198   new

-42     0       0       -42     msb-430h
10550   34      1056    11640   old
10508   34      1056    11598   new

-16     0       0       -16     msba2
73532   2356    93961   169849  old
73516   2356    93961   169833  new

-12     0       0       -12     msbiot
14608   168     2792    17568   old
14596   168     2792    17556   new

-4      0       0       -4      mulle
41908   1304    11896   55108   old
41904   1304    11896   55104   new

-280    0       0       -280    native
77825   612     82712   161149  old
77545   612     82712   160869  new

-12     0       0       -12     nrf51dongle
22940   164     10316   33420   old
22928   164     10316   33408   new

-12     0       0       -12     nrf6310
22912   164     10316   33392   old
22900   164     10316   33380   new

-16     0       0       -16     nucleo-f091
15624   164     2636    18424   old
15608   164     2636    18408   new

-8      0       0       -8      nucleo-f103
12468   164     2764    15396   old
12460   164     2764    15388   new

-8      0       0       -8      nucleo-f303
12676   164     2636    15476   old
12668   164     2636    15468   new

-8      0       0       -8      nucleo-f334
12288   164     2620    15072   old
12280   164     2620    15064   new

-16     0       0       -16     nucleo-f401
14304   168     2760    17232   old
14288   168     2760    17216   new

-8      0       0       -8      nucleo-l1
13780   164     2748    16692   old
13772   164     2748    16684   new

-8      0       0       -8      openmote
12252   164     2620    15036   old
12244   164     2620    15028   new

4       0       0       4       pba-d-01-kw2x
37752   1268    11948   50968   old
37756   1268    11948   50972   new

-12     0       0       -12     pca10000
22940   164     10316   33420   old
22928   164     10316   33408   new

-12     0       0       -12     pca10005
22916   164     10316   33396   old
22904   164     10316   33384   new

-32     0       0       -32     pttu
60796   2356    95945   159097  old
60764   2356    95945   159065  new

0       0       0       0       qemu-i386
132413  2340    68232   202985  old
132413  2340    68232   202985  new

-8      0       0       -8      remote
12908   164     2620    15692   old
12900   164     2620    15684   new

-12     0       0       -12     saml21-xpro
23164   164     10444   33772   old
23152   164     10444   33760   new

-12     0       0       -12     samr21-xpro
34144   168     11608   45920   old
34132   168     11608   45908   new

-8      0       0       -8      slwstk6220a
12200   164     2748    15112   old
12192   164     2748    15104   new

-4      0       0       -4      spark-core
22800   164     10444   33408   old
22796   164     10444   33404   new

-16     0       0       -16     stm32f0discovery
12536   164     2628    15328   old
12520   164     2628    15312   new

-8      0       0       -8      stm32f3discovery
12684   164     2636    15484   old
12676   164     2636    15476   new

-12     0       0       -12     stm32f4discovery
14544   168     2776    17488   old
14532   168     2776    17476   new

-42     0       0       -42     telosb
10616   30      1056    11702   old
10574   30      1056    11660   new

-8      0       0       -8      udoo
12244   168     2648    15060   old
12236   168     2648    15052   new

-16     0       0       -16     weio
12352   164     2620    15136   old
12336   164     2620    15120   new

-42     0       0       -42     wsn430-v1_3b
10624   34      1056    11714   old
10582   34      1056    11672   new

-42     0       0       -42     wsn430-v1_4
10624   34      1056    11714   old
10582   34      1056    11672   new

-12     0       0       -12     yunjia-nrf51822
22900   164     10316   33380   old
22888   164     10316   33368   new

-38     0       0       -38     z1
10160   30      1056    11246   old
10122   30      1056    11208   new

@Kijewski
core: fix mutex debug messages
558f211
@Kijewski
core: unify mutex_lock and mutex_trylock
684fa8b 
The code is much clearer IMO. atomic_set_to_one() uses disableIRQ() for
most platforms, so using to causes an overhead.
@Kijewski
core: no needless pointer-casting in mutex queue
e06ee95
@Kijewski
core: mutexes memorize holding thread id
abc3429 
Closes #4494
@Kijewski Kijewski added Area: core Area: RIOT kernel. Handle PRs marked with this with care! Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 28, 2015
@Kijewski Kijewski added this to the Release 2016.03 milestone Dec 28, 2015
Kijewski
Kijewski reviewed Dec 28, 2015
View reviewed changes
core/include/mutex.h
void mutex_lock(mutex_t *mutex);
static inline void mutex_lock(mutex_t *mutex)
{
_mutex_lock(mutex, 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I return the information whether the caller had to wait to get hold of the mutex? The API change would not affect any existing code, but I cannot think of a case when you would need this information.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same here... It would (slightly) hurt performance in all cases, and can maybe be emulated by calling first mutex_trylock, then mutex_lock.

@kaspar030
Copy link
Contributor

Nice. I would have thought that always disabling IRQs would hurt non-locked case, but it turns out that it's a lot faster this way (330k vs 480k locks/second on sam r21).

@kaspar030
Copy link
Contributor

But IMHO the PR title is misleading, not using atomics is not a trivial change, and has nothing to do with storing the holder's PID. Not storing the PID (using this PR's code but using 0 and 1 instead of PID) saves another 16 bytes code (on cortex M0).

edit
Using 0/1 instead of PID makes the unlocked case slightly faster (~7%).

@Kijewski Would you mind seperating those changes?

@Kijewski
Copy link
Contributor Author

Please test https://github.com/Kijewski/RIOT/commit/849e7f85fda53bcce62148dd17ff9a2a6ec2ccfd.

@kaspar030 kaspar030 mentioned this pull request Dec 29, 2015
Merged
@kaspar030
Copy link
Contributor

Please test Kijewski@849e7f8.

Hangs somewhere for me.

Could you check out #4557? I had a branch lying around which drops mutex' dependency on priority_queue. I've integrated your ideas from this PR. Result is 4% more speed, 20 bytes code saved and sizeof(mutex) == sizeof(void*).

@OlegHahm
Copy link
Member

IMO any non-trivial change to the core shouldn't be labeled as "minor".

@Kijewski Kijewski removed the Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer label Dec 29, 2015
@Kijewski
Copy link
Contributor Author

Closed in favor of #4557.

@Kijewski Kijewski closed this Dec 29, 2015
@Kijewski Kijewski deleted the issue-4494 branch December 29, 2015 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kaspar030 kaspar030

Labels
Area: core Area: RIOT kernel. Handle PRs marked with this with care! CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Milestone
Release 2016.04
Development

Successfully merging this pull request may close these issues.
3 participants
@Kijewski @kaspar030 @OlegHahm