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

malloc test: fix out of memory problem for NUCLEO_F070RB #5338

Merged
merged 1 commit into from
Nov 2, 2017

Conversation

maciejbocianski
Copy link
Contributor

@maciejbocianski maciejbocianski commented Oct 18, 2017

Description

This test cause out of memory error ("Operator new[] out of memory") when run on NUCLEO_F070RB with GCC_ARM compiler

Changes was introduced to reduce heap allocations
Now test threads uses static memory as a stack memory

Status

READY

Migrations

NO

@jeromecoutant
Copy link
Collaborator

Hi
What do you think about #4890 ?

@jeromecoutant
Copy link
Collaborator

@c1728p9

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 18, 2017

@jeromecoutant Thanks for the reference.
@maciejbocianski Have you reviewed the referenced PR ? I believe both of these patches fixing the same problem.

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 18, 2017

This patch is also related #5285 (how much space should a target have available by default).

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 18, 2017

cc @bulislaw @c1728p9

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 19, 2017

@jeromecoutant I believe with this patch, both devices you encoutered problems earlier (NUCLEO_F070RB, NUCLEO_F072RB) should pass

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 19, 2017

/morph build

@maciejbocianski
Copy link
Contributor Author

maciejbocianski commented Oct 19, 2017

tested only on NUCLEO_F070RB
but currently none of them are tested on CI

@mbed-ci
Copy link

mbed-ci commented Oct 19, 2017

Build : SUCCESS

Build number : 255
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5338/

Skipping test trigger, missing label 'NEED CI'

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 19, 2017

@jeromecoutant I believe with this patch, both devices you encoutered problems earlier (NUCLEO_F070RB, NUCLEO_F072RB) should pass

Are you happy with this patch, please review

@jeromecoutant
Copy link
Collaborator

Hi

Yes, this makes the malloc test passed. Thx!

| NUCLEO_F070RB-GCC_ARM | NUCLEO_F070RB | tests-mbedmicro-rtos-mbed-malloc | OK | 28.62 | default |

Maybe we can apply the same patch with other failed rtos tests ?

| NUCLEO_F070RB-GCC_ARM | tests-mbedmicro-rtos-mbed-basic | TIMEOUT
| NUCLEO_F070RB-GCC_ARM | tests-mbedmicro-rtos-mbed-mail | TIMEOUT
| NUCLEO_F070RB-GCC_ARM | tests-mbedmicro-rtos-mbed-semaphore | TIMEOUT
| NUCLEO_F070RB-GCC_ARM | tests-mbedmicro-rtos-mbed-threads | TIMEOUT

@adbridge
Copy link
Contributor

@ @bulislaw @c1728p9 bump

@bulislaw
Copy link
Member

@maciejbocianski are you able to do the same for other failing tests as suggested in the previous comment?

@bulislaw
Copy link
Member

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 26, 2017

Build : SUCCESS

Build number : 353
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5338/

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci
Copy link

mbed-ci commented Oct 27, 2017

@maciejbocianski
Copy link
Contributor Author

problem fixed - there was lack of thread stack alignment

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 27, 2017

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 27, 2017

Build : SUCCESS

Build number : 356
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5338/

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci
Copy link

mbed-ci commented Oct 27, 2017

@jeromecoutant
Copy link
Collaborator

Hi

Quick comment to add information:

| NUCLEO_F070RB-GCC_ARM | tests-mbedmicro-rtos-mbed-mail | TIMEOUT

=> #5376

| NUCLEO_F070RB-GCC_ARM | tests-mbedmicro-rtos-mbed-semaphore | TIMEOUT

=> #5377

| NUCLEO_F070RB-GCC_ARM | tests-mbedmicro-rtos-mbed-threads | TIMEOUT

=> #5360

Thx!

@adbridge
Copy link
Contributor

15:59:20 [K64F] [1509119959.83][HTST][INF] {{result;sync_failed}}
15:59:20 [K64F] mbedgt: checking for GCOV data...
15:59:20 [K64F] mbedgt: mbed-host-test-runner: stopped and returned 'SYNC_FAILED'
15:59:20 [K64F] mbedgt: test on hardware with target id: DUMMY
15:59:20 [K64F] mbedgt: test suite 'autogen-tests-uvisor-k64f_gcc_arm_r_1_1_2' ....................................... SYNC_FAILED in 34.69 sec
15:59:20 [K64F] test case: 'box_id_namespace_ok' ............................................................. OK in 0.00 sec
15:59:20 [K64F] test case: 'box_id_namespace_swapped_out_buffer' ............................................. ERROR in 0.00 sec
15:59:20 [K64F] test case: 'box_id_namespace_unowned_buffer' ................................................. OK in 0.00 sec
15:59:20 [K64F] test case: 'ipc_test_1/.r./s.a/.../' ......................................................... FAIL in 0.00 sec

@orenc17a Any idea why this is failing ?

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 2, 2017

/morph uvisor-test

@@ -54,17 +54,22 @@ void task_using_malloc(void)

int main()
{
// static stack for threads to reduce heap usage on devices with small RAM
// and eliminate run out of heap memory problem
MBED_ALIGN(8) uint8_t stack[THREAD_STACK_SIZE * NUM_THREADS];
Copy link
Contributor

Choose a reason for hiding this comment

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

The alignment isn't necessary with #5405.

If you make this a two dimensional array then the loop below will be clearer.

Declaration:

uint8_t stack[NUM_THREADS][THREAD_STACK_SIZE];

Loop:

thread_list[i] = new Thread(osPriorityNormal, THREAD_STACK_SIZE, stack[i]);

@theotherjimmy
Copy link
Contributor

@c1728p9 Are you requesting changes? We were about to merge this.

@c1728p9
Copy link
Contributor

c1728p9 commented Nov 2, 2017

I don't want to block anything so lets merge it. @maciejbocianski can you address the comments in a separate PR?

theotherjimmy added a commit that referenced this pull request Nov 2, 2017
malloc test: fix out of memory problem for NUCLEO_F070RB
@theotherjimmy theotherjimmy merged commit d1c65c9 into ARMmbed:master Nov 2, 2017
@maciejbocianski
Copy link
Contributor Author

maciejbocianski commented Nov 3, 2017

@c1728p9 sure, I will address it in separate PR (#5430)

maciejbocianski added a commit to maciejbocianski/mbed-os that referenced this pull request Nov 3, 2017
@mbed-ci
Copy link

mbed-ci commented Nov 3, 2017

Build : SUCCESS

Build number : 422
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5338/

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci
Copy link

mbed-ci commented Nov 3, 2017

Build : SUCCESS

Build number : 424
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5338/

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci
Copy link

mbed-ci commented Nov 3, 2017

@mbed-ci
Copy link

mbed-ci commented Nov 3, 2017

adbridge pushed a commit that referenced this pull request Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants