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

measure_stack_free_internal(): don't try to align end of stack #20723

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Jun 4, 2024

Contribution description

The end-of stack alignment causes the function to fail measuring the stack size.

Testing procedure

A new test was added to verify the stack measurement.

ps also shows the stack usage again

2024-06-04 16:56:22,086 # ps
2024-06-04 16:56:22,089 # 	pid | name                 | state    Q | pri | stack  ( used) ( free) | base addr  | current     
2024-06-04 16:56:22,091 # 	  - | isr_stack            | -        - |   - |   8192 (   -1) ( 8193) |  0x8085880 |  0x8085880
2024-06-04 16:56:22,093 # 	  1 | idle                 | pending  Q |  15 |   8192 (  436) ( 7756) |  0x8080180 |  0x8081fdc 
2024-06-04 16:56:22,095 # 	  2 | main                 | running  Q |   7 |  12288 ( 2792) ( 9496) |  0x8082180 |  0x8084fdc 
2024-06-04 16:56:22,097 # 	  3 | pktdump              | bl rx    _ |   6 |  12224 (  896) (11328) |  0x808da40 |  0x809085c 
2024-06-04 16:56:22,098 # 	  4 | ipv6                 | bl rx    _ |   4 |   8128 ( 1464) ( 6664) |  0x8087c80 |  0x8089a9c 
2024-06-04 16:56:22,100 # 	  5 | udp                  | bl rx    _ |   5 |   4032 (  960) ( 3072) |  0x8092ce0 |  0x8093afc 
2024-06-04 16:56:22,102 # 	  6 | gnrc_netdev_tap      | bl anyfl _ |   2 |   8064 ( 2216) ( 5848) |  0x808a220 |  0x808bffc 
2024-06-04 16:56:22,105 # 	  7 | RPL                  | bl rx    _ |   5 |   8192 (  896) ( 7296) |  0x8090c80 |  0x8092adc 
2024-06-04 16:56:22,106 # 	    | SUM                  |            |     |  69312 ( 9660) (59652)

Issues/PRs references

fixes the issue from #18942 (comment)

@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Jun 4, 2024
@benpicco benpicco requested review from miri64 and maribu June 4, 2024 14:48
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 4, 2024
@benpicco benpicco requested a review from kaspar030 as a code owner June 4, 2024 14:54
@github-actions github-actions bot added the Area: core Area: RIOT kernel. Handle PRs marked with this with care! label Jun 4, 2024
@benpicco benpicco changed the title tests/core/thread_stack_alignment: also test for stack usage measure_stack_free_internal(): don't try to align end of stack Jun 4, 2024
@benpicco benpicco force-pushed the tests/thread_stack_alignment-usage branch from 58e62c9 to 5b73294 Compare June 4, 2024 14:57
@benpicco benpicco added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Jun 4, 2024
@benpicco benpicco requested a review from chrysn June 4, 2024 14:58
@benpicco benpicco changed the title measure_stack_free_internal(): don't try to align end of stack tests/core/thread_stack_alignment: also test for stack usage Jun 4, 2024
@benpicco
Copy link
Contributor Author

benpicco commented Jun 4, 2024

superseded by #20724

@benpicco benpicco closed this Jun 4, 2024
@benpicco benpicco deleted the tests/thread_stack_alignment-usage branch June 4, 2024 15:21
@maribu
Copy link
Member

maribu commented Jun 4, 2024

Let's go with this. The stack size is sanitized before any arch code is called, I don't think there is value in keeping the extra alignment code:

RIOT/core/thread.c

Lines 220 to 221 in 2576649

/* round down the stacksize to a multiple of thread_t alignments (usually 16/32bit) */
stacksize -= stacksize % alignof(thread_t);

@benpicco benpicco restored the tests/thread_stack_alignment-usage branch June 4, 2024 16:23
@benpicco benpicco reopened this Jun 4, 2024
@benpicco benpicco changed the title tests/core/thread_stack_alignment: also test for stack usage measure_stack_free_internal(): don't try to align end of stack Jun 4, 2024
@riot-ci
Copy link

riot-ci commented Jun 4, 2024

Murdock results

✔️ PASSED

5b73294 tests/core/thread_stack_alignment: also test for stack usage

Success Failures Total Runtime
10160 0 10161 17m:17s

Artifacts

Merged via the queue into RIOT-OS:master with commit ec71eba Jun 4, 2024
51 checks passed
@benpicco benpicco deleted the tests/thread_stack_alignment-usage branch June 4, 2024 22:11
@mguetschow mguetschow added this to the Release 2024.07 milestone Jun 11, 2024
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! Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants