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

Increase HEAP size for UBLOX_EVK_ODIN_W2 and NUCLEO_F429ZI #3871

Merged
merged 1 commit into from
Mar 3, 2017

Conversation

mazimkhan
Copy link

@mazimkhan mazimkhan commented Mar 2, 2017

Notes:

  • Pull requests will not be accepted until the submitter has agreed to the contributer agreement.
  • This is just a template, so feel free to use/remove the unnecessary things

Description

mbed-os-example-tls/tls-client example fails on UBLOX_EVK_ODIN_W2 and NUCLEO_F429ZI for IAR due to out of memory issue.
Both the boards have 191K RAM. But heap size is set to 0xC000 (48K). This is fine with GCC_ARM and ARMCC compilers as heap can grow. But IAR has fixed heap. Since tls-client example runs fine on K64F x IAR. The heap size should atleast be 0x10000 (64K). However, since these boards have good size RAM this PR sets fixed heap size to ~1/2 of RAM that is 0x18000 (96K).

Status

READY/IN DEVELOPMENT/HOLD

Migrations

If this PR changes any APIs or behaviors, give a short description of what API users should do when this PR is merged.

NO

Related PRs

List related PRs against other branches:

branch PR
other_pr_production link
other_pr_master link

Todos

  • Tests
  • Documentation

Deploy notes

After merging this PR mbed-os.lib(s) in mbed-os-example-tls should be updated.

Steps to test or reproduce

To reproduce run example mbed-os-example-tls/tls-client it should fail on UBLOX_EVK_ODIN_W2 x IAR and NUCLEO_F429ZI x IAR
To test use this PR version of mbed-os and the example should pass.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 2, 2017

cc @andreaslarssonublox @bcostm @adustm @LMESTM @jeromecoutant - please review

@adustm
Copy link
Member

adustm commented Mar 2, 2017

Hello,
It's ok for us. We may consider decreasing to 64K if this causes an issue for some reason to some other users.
Kind regards

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 2, 2017

It's ok for us. We may consider decreasing to 64K if this causes an issue for some reason to some other users.

@mazimkhan What do you think?

@mazimkhan
Copy link
Author

@adustm Do we have a use case where it can cause issue. Actually mbed-os-example-tls takes between 48K to 64K. Problem is IAR has a fixed heap. It is easy to find a specific value for an app. But mbed-os is a lib and future tests/examples (relevant to this board) may again break 64K barrier.

@adustm
Copy link
Member

adustm commented Mar 2, 2017

Hello,
I don't personally, but I have no idea how the end customers are using the memory.
I can't remember of someone asking for a heap size decrease. It should be fine.
Cheers

@JanneKiiskila
Copy link
Contributor

I think focus should be on having the
#1 - examples working (all of them)
#2 - adding information on how to spot if you're out of mem and what to do in that case
-- for example modify this linker file to achieve what you want

@andreaslarssonublox
Copy link

@JanneKiiskila I agree. Should this be described in a document like the below since it's the same for all targets or are there better places?
https://docs.mbed.com/docs/mbed-os-handbook/en/latest/concepts/memory_model/

@JanneKiiskila
Copy link
Contributor

@andreaslarssonublox - yes, it should be a in a common document, this is not UBLOX specific issue - it's common. All IoT boards will run out of memory rather easily, so we should give common guidance.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 3, 2017

/morph test

@mbed-bot
Copy link

mbed-bot commented Mar 3, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1621

All builds and test passed!

@SeppoTakalo
Copy link
Contributor

After this change, our CI jobs have started to fail builds on IAR to platforms NUCLEO_F429ZI and Ublox EVK ODIN W2.

See: https://jenkins-internal.mbed.com/job/ARMmbed/job/mbed-os-example-mesh-minimal/job/master/

However, on my local machine all seems to work. What IAR version are we supposed to use? I have V7.70.2

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 10, 2017

However, on my local machine all seems to work. What IAR version are we supposed to use? I have V7.70.2

7.8

After this change, our CI jobs have started to fail builds on IAR to platforms NUCLEO_F429ZI and Ublox EVK ODIN W2.

why continuous-integration/jenkins/pr-head did not fail ? We might want to revert this change

@SeppoTakalo
Copy link
Contributor

I think this failure is only on applications using enough memory.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 10, 2017

I think this failure is only on applications using enough memory.

What we can do is just either revert it back to the value, or decrease it by some margin that would enable our apps to work and any further changes are disallowed. After this, any failure in apps is a known-issue that is limitation of IAR. Playing ping-pong with HEAP size value is not a solution, we cant find a value that serves all apps. @mazimkhan will send a patch where we can discuss this further

I think this failure is only on applications using enough memory.

they could be in CI?

@mazimkhan
Copy link
Author

mazimkhan commented Mar 10, 2017

mbed-os-examples-tls/tls-client example has a minimum heap requirement of 0x10000 that was not there in the original code. This PR increases heap may be too much (0x18000) reducing data space below what is required by mbed-client.
Hence I think we should set it to the minimum size required by tls-client example. That should fix both.

However, for mbed-os to support different platforms and apps there should be a strategy on use of global/static variable and heap to avoid fiddling with heap sizes. Large variations between data and heap size requirements across apps may cause this type situation.
Please see my comment here ARMmbed/mbed-os-example-client#194 (comment)

mazimkhan pushed a commit to mazimkhan/mbed-os that referenced this pull request Mar 15, 2017
Targets NUCLEO_F429ZI and UBLOX_EVK_ODIN_W2 have 192K RAM.
Heap size in PR ARMmbed#3871 was increased from 48K to 96K as tls-client
example failed with 48K heap. But this resulted in compilation failures
in mbed-client that requires 71K for global/static data.
Hence this PR reduces heap to 64K that minimum required by tls-client
to work. This also meets mbed-client data segment requirements.
adbridge pushed a commit that referenced this pull request Mar 24, 2017
Targets NUCLEO_F429ZI and UBLOX_EVK_ODIN_W2 have 192K RAM.
Heap size in PR #3871 was increased from 48K to 96K as tls-client
example failed with 48K heap. But this resulted in compilation failures
in mbed-client that requires 71K for global/static data.
Hence this PR reduces heap to 64K that minimum required by tls-client
to work. This also meets mbed-client data segment requirements.
adbridge pushed a commit that referenced this pull request Mar 24, 2017
Targets NUCLEO_F429ZI and UBLOX_EVK_ODIN_W2 have 192K RAM.
Heap size in PR #3871 was increased from 48K to 96K as tls-client
example failed with 48K heap. But this resulted in compilation failures
in mbed-client that requires 71K for global/static data.
Hence this PR reduces heap to 64K that minimum required by tls-client
to work. This also meets mbed-client data segment requirements.
@Rajkumar181
Copy link

Hi Team,

I am getting same issue. Can you help me on this.

This is my Logs

..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:3161: handshake message: msglen = 5021, type = 11, hslen = 5021
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:3846: <= read record
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:4180: => send alert message
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:4181: send alert level=2 message=80
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:2764: => write record
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:2910: output record: msgtype = 21, version = [3:3], msglen = 2
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:2913: dumping 'output record sent to network' (7 bytes)
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:2913: 0000: 15 03 03 00 02 02 50 ......P
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:2471: => flush output
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:2490: message length: 7, out_left: 7
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:2496: ssl->f_send() returned 7 (-0xfffffff9)
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:2523: <= flush output
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:2922: <= write record
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:4193: <= send alert message
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:4573: mbedtls_x509_crt_parse_der() returned -10368 (-0x2880)
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:6764: <= handshake
failed
! mbedtls_ssl_handshake returned -0x2880

Last error was: -10368 - X509 - Allocation of memory failed

..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:7542: => free
..\Middlewares\Third_Party\mbedTLS\library\ssl_tls.c:7607: <= free

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