Skip to content

Conversation

@yamt
Copy link
Contributor

@yamt yamt commented Mar 27, 2020

No description provided.

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Mar 27, 2020

Nice patch!
How about we change the default stack size which is smaller than 2048 to DEFAULT_STACK_SIZE, but keep other big stack size as before:
examples/igmp/Makefile:STACKSIZE = 1024
examples/powerled/Makefile:STACKSIZE = 1024
examples/powermonitor/Makefile:STACKSIZE = 768
examples/relays/Makefile:STACKSIZE = 512
examples/smps/Makefile:STACKSIZE = 1024
graphics/screenshot/Makefile:STACKSIZE = 4096
system/flash_eraseall/Makefile:STACKSIZE = 1024
testing/cxxtest/Makefile:STACKSIZE = 4096
testing/smart_test/Makefile:STACKSIZE = 4096
Since the change is safe.

@yamt
Copy link
Contributor Author

yamt commented Mar 27, 2020

using more memory might or might not be safe for small systems.
for examples and testing, it might be fine though.

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Mar 27, 2020

using more memory might or might not be safe for small systems.
for examples and testing, it might be fine though.

Yes, but since:
1.The memory for each application increase is no more than 1.5K
2.We most likely don't run all this application at the same time
3.These application normally is launched by user manually
The suggestion in most case:
1.Don't increase the memory consumption after boot finish
2.Just increase 1.5KB when user launch these application
On the other hand:
1.Unify the whole code base
2.Make these applications can run on sim
So it's a good comprimise.

yamt added 6 commits March 27, 2020 15:33
For now, I left the following instances because it isn't
clear to me why they are using the different values.
Maybe they need one-by-one inspection.

    examples/igmp/Makefile:STACKSIZE = 1024
    examples/powerled/Makefile:STACKSIZE = 1024
    examples/powermonitor/Makefile:STACKSIZE = 768
    examples/relays/Makefile:STACKSIZE = 512
    examples/smps/Makefile:STACKSIZE = 1024
    graphics/screenshot/Makefile:STACKSIZE = 4096
    system/flash_eraseall/Makefile:STACKSIZE = 1024
    testing/cxxtest/Makefile:STACKSIZE = 4096
    testing/smart_test/Makefile:STACKSIZE = 4096
This commit changes only ones with the default 2048 and
leaves the others.
E.g. this leaves SYSTEM_RAMTEST_STACKSIZE, whose default is 1024.
I guess those need to be inspected one-by-one.
These had the larger default for the sim.
It's no longer necessary as DEFAULT_TASK_STACKSIZE
can have different default for each arch.

See also:
    commit b1d44a8
Where the hardcoded values are less than 2048.
Using a bit more memory for examples should not be a critical problem.
Where:
 * Under examples and testing
 * And the default value is less than 2048

Using a bit more memory for examples and tests should not
be a critical problem.
@yamt yamt force-pushed the default-stacksize branch from 29d8e73 to 44bd7f6 Compare March 27, 2020 06:34
@yamt
Copy link
Contributor Author

yamt commented Mar 27, 2020

@xiaoxiang781216 ok. done

@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 ok. done

Should we change SYSTEM_RAMTEST_STACKSIZE too?

…KSIZE

Given what this program does, it's probably ok to allocate a few more
kilo bytes of the stack.
@yamt
Copy link
Contributor Author

yamt commented Mar 27, 2020

i guess stack size is not too important for what ramtest does. done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants