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

refactor struct device to reduce RAM demands #26072

Closed
pabigot opened this issue Jun 9, 2020 · 2 comments
Closed

refactor struct device to reduce RAM demands #26072

pabigot opened this issue Jun 9, 2020 · 2 comments
Labels
Enhancement Changes/Updates/Additions to existing features
Milestone

Comments

@pabigot
Copy link
Collaborator

pabigot commented Jun 9, 2020

#22941 proposes a suite of upgrades to the device infrastructure. An initial step made in that process was to move values from a separate config_info structure into struct device directly. This was driven by a goal of making struct device instances const qualified and so storable in ROM, based on the observation that the only required non-const portion of that structure is a flag that indicates whether the device successfully initiated (flag currently implemented as nulling the API pointer).

#25208 evaluated the practicality of this change concluding that it's not feasible, in part because too much API expects to represent struct device * in void *, and casting const away opens the possibility of writing through a pointer-to-const which produces undefined behavior. Dev-review also raised the possibility of runtime instantiation of devices.

We must make a decision whether to support const struct device instances. If not, we should reverse the course initiated by #23407 and instead move the immutable portions of struct device back to a secondary structure that is const-qualified.

This affects progress on #22545 which requires adding more immutable state to struct device to represent dependency information.

@pabigot pabigot added the RFC Request For Comments: want input from the community label Jun 9, 2020
@pabigot pabigot added this to the v2.4.0 milestone Jun 9, 2020
@pabigot pabigot removed the RFC Request For Comments: want input from the community label Jun 11, 2020
@pabigot pabigot changed the title RFC: refactor struct device to reduce RAM demands refactor struct device to reduce RAM demands Jun 11, 2020
@MaureenHelm MaureenHelm added the Enhancement Changes/Updates/Additions to existing features label Jun 12, 2020
@pabigot
Copy link
Collaborator Author

pabigot commented Jun 16, 2020

Some data on options:

#25208 makes struct device const. It touches 1335 files including 541 changes to 157 samples. It will force all users to rework their code to use const struct device* to conform to the changes in every device API.

For samples/boards/reel_board/mesh_badge on reel_board: net -225 RAM +312 Flash
   text	   data	    bss	    dec	    hex	filename
 246626	   7413	  52236	 306275	  4ac63	build/zephyr/zephyr.elf   # RAM
 246938	   7173	  52251	 306362	  4acba	build/zephyr/zephyr.elf   # Flash
    312    -240      15

#26127 moves immutable data to dev->fixed. It touches 553 files including 33 changes to 28 samples. Impact on users will be minimal. Impact on drivers will be addition of a pointer dereference in many driver implementation functions, which I suspect is small but we don't have any clear criteria for assessing it.

For samples/boards/reel_board/mesh_badge on reel_board: net -120 RAM +594 Flash
   text	   data	    bss	    dec	    hex	filename
 246190	   7509	  52256	 305955	  4ab23	build/zephyr/zephyr.elf   # RAM
 246784	   7389	  52256	 306429	  4acfd	build/zephyr/zephyr.elf   # Flash
    594    -120       0

Note that you cannot compare between the two PRs because they have different bases. I'd have to rebase 25208 on the same master commit as 26127 for a fair comparison.

Also note that the difference due to master changes between the base versions is comparable to the difference from applying either approach. We're looking at memory usage changes that are on the order of one or two percent of real application RAM usage.

At this point absent consensus it may be best to just keep going with the current architecture until there's a plan for device rework that's based on validated requirements and supported by committed resources.

@pabigot
Copy link
Collaborator Author

pabigot commented Dec 28, 2020

struct device is now const. We don't yet have generic device RAM support, but that's not this issue.

@pabigot pabigot closed this as completed Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

2 participants