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.

Sign up for GitHub

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

Jump to bottom

kernel/mm: shuffle header files #65149

Merged
merged 3 commits into from
Nov 20, 2023
Merged

kernel/mm: shuffle header files #65149

merged 3 commits into from
Nov 20, 2023

Conversation

dcpleung
Copy link
Member

  • Moved kernel MM APIs (k_mem_*) under include/zephyr/kernel/.
  • Separated out z_* ones into internal header directory.
  • Separated demand paging APIs into its own header file.
  • Some doxygen work so MM APIs are actually included in the API doc.

@dcpleung dcpleung force-pushed the kernel/mm_apis_reshuffle branch from 383aae4 to 6017692 Compare November 14, 2023 20:34
@dcpleung
kernel: mm: move kernel mm functions under kernel includes
5c8eaed 
This moves the k_* memory management functions from sys/ into
kernel/ includes, as there are kernel public APIs. The z_*
functions are further separated into the kernel internal
header directory.

Also made a quick change to doxygen to group sys_mem_* into
the OS Memory Management group so they will appear in doc.

Signed-off-by: Daniel Leung <[email protected]>
@dcpleung
kernel: mm: separate demand paging headers into its own file
7ab6172 
This separates demand paging related headers into its own file
instead of being stuffed inside the main kernel memory
management header file.

Signed-off-by: Daniel Leung <[email protected]>
@dcpleung dcpleung force-pushed the kernel/mm_apis_reshuffle branch from 6017692 to bc167e3 Compare November 15, 2023 20:45
@dcpleung
doc: kernel/mm: some doxygen works
bc167e3 
() Some kernel memory management functions were previously
   not in any group. So put them under the kernel memory
   management group, and now they appear in API doc.
() Group things together when appropriate.
() Add doc if none exists before.

Signed-off-by: Daniel Leung <[email protected]>
@dcpleung dcpleung marked this pull request as ready for review November 16, 2023 01:35
@kartben kartben self-requested a review November 16, 2023 04:44
@carlescufi
Copy link
Member

@jhedberg can you take a look?

kartben
kartben requested changes Nov 16, 2023
View reviewed changes
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

Unless I'm missing something, the fact that users of the API would need to update their includes calls for an entry in the migration guide.
Looks 👍🏼 otherwise, thank you!

@jhedberg
Copy link
Member

@jhedberg can you take a look?

Looks fine, but I'll wait for the comment from @kartben to be addressed first.

@dcpleung
Copy link
Member Author

Unless I'm missing something, the fact that users of the API would need to update their includes calls for an entry in the migration guide. Looks 👍🏼 otherwise, thank you!

Not writing this in the migration guide is intentional. Out of tree repo users and app developers should continue using sys/mem_manage.h as it always include the kernel memory management headers. On the other hand, IMHO, in tree users should be a little more discipline on what to include.

Note that the main intention of this PR is to move k_* symbols under kernel/. The concept of sys -> memory management has not changed.

If you insist, I can add this to the migration guide (though I think it creates unnecessary work for others). Or I can remove the #include changes in this PR.

@kartben
Copy link
Collaborator

kartben commented Nov 18, 2023

Unless I'm missing something, the fact that users of the API would need to update their includes calls for an entry in the migration guide. Looks 👍🏼 otherwise, thank you!

Not writing this in the migration guide is intentional. Out of tree repo users and app developers should continue using sys/mem_manage.h as it always include the kernel memory management headers. On the other hand, IMHO, in tree users should be a little more discipline on what to include.

Note that the main intention of this PR is to move k_* symbols under kernel/. The concept of sys -> memory management has not changed.

If you insist, I can add this to the migration guide (though I think it creates unnecessary work for others). Or I can remove the #include changes in this PR.

So I was missing something, thanks!

@nashif nashif assigned dcpleung and unassigned jhedberg Nov 18, 2023
@carlescufi carlescufi merged commit f52f76f into zephyrproject-rtos:main Nov 20, 2023
34 checks passed
@dcpleung dcpleung deleted the kernel/mm_apis_reshuffle branch November 20, 2023 16:20
@henrikbrixandersen
Copy link
Member

This broke CI on main: #65506

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

@peter-mitsis peter-mitsis peter-mitsis approved these changes

@jhedberg jhedberg jhedberg approved these changes

@cfriedt cfriedt cfriedt approved these changes

@carlocaione carlocaione carlocaione approved these changes

@andyross andyross andyross approved these changes

@nashif nashif nashif approved these changes

@kartben kartben kartben approved these changes

@carlescufi carlescufi Awaiting requested review from carlescufi

@galak galak Awaiting requested review from galak

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@npitre npitre Awaiting requested review from npitre

@ioannisg ioannisg Awaiting requested review from ioannisg

@ceolin ceolin Awaiting requested review from ceolin

@laurenmurphyx64 laurenmurphyx64 Awaiting requested review from laurenmurphyx64

@stephanosio stephanosio Awaiting requested review from stephanosio

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

Assignees

@dcpleung dcpleung

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@dcpleung @carlescufi @jhedberg @kartben @henrikbrixandersen @nashif @andyross @carlocaione @cfriedt @peter-mitsis