Skip to content

Conversation

@dcpleung
Copy link
Member

This adds a new kconfig for eviction algorithm which needs page tracking. When enabled, k_mem_paging_eviction_add()/_remove() and k_mem_paging_eviction_accessed() must be implemented. If an algorithm does not do page tracking, there is no need to implement these functions, and no need for the kernel MMU code to call into empty functions. This should save a few function calls and some CPU cycles.

Note that arm64 unconditionally calls those functions so forces CONFIG_EVICTION_TRACKING to be enabled there.

Copy link

@npitre npitre left a comment

Choose a reason for hiding this comment

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

OK for the Kconfig symbol, however it might be nicer to define
empty static inline stubs when disabled rather than adding more #ifdef
in kernel/mmu.c.

@dcpleung
Copy link
Member Author

Being static inline does not necessarily eliminate the related code when no optimization is used. These inlined functions are converted into non-inlined functions and will be called. Even changing the static inline to static ALWAYS_INLINE will simply replace the assembly call into nop. So they are not totally eliminated. One could argue that this is expected from using no optimizations, but it would be great if the code is actually removed when kconfig is disabled, such that there are fewer surprised when debugging.

@npitre
Copy link

npitre commented Nov 15, 2024 via email

@dcpleung
Copy link
Member Author

Because lots of time I do debug with no optimizations.

@npitre
Copy link

npitre commented Nov 15, 2024 via email

peter-mitsis
peter-mitsis previously approved these changes Nov 15, 2024
Copy link
Contributor

@peter-mitsis peter-mitsis left a comment

Choose a reason for hiding this comment

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

I'm OK with the #ifdef stuff, but if it is blocking issue, would defining the affected routines as empty macros be a satisfactory compromise? It is an approach with precedence and should hopefully work well with the no optimization scenario.

@dcpleung
Copy link
Member Author

Because lots of time I do debug with no optimizations.
Then you must be used to skip over all those empty inline stubs already.

Well... doesn't mean that I want to make it worse.

Besides, there is now code being run which should not have been there, for example the for loop in z_mem_manage_init() to add page frames in queue. Instead of skipping over the whole loop, there would be actual looping to call into a function that does nothing (but there is still a call in assembly and all the stack manipulation). Semantically speaking, it would not be the same code. Therefore if possible, I would like those part of code to not appear in compiled code, regardless of optimization level.

This adds a new kconfig for eviction algorithm which needs page
tracking. When enabled, k_mem_paging_eviction_add()/_remove()
and k_mem_paging_eviction_accessed() must be implemented.
If an algorithm does not do page tracking, there is no need to
implement these functions, and no need for the kernel MMU code
to call into empty functions. This should save a few function
calls and some CPU cycles.

Note that arm64 unconditionally calls those functions so
forces CONFIG_EVICTION_TRACKING to be enabled there.

Signed-off-by: Daniel Leung <[email protected]>
@dcpleung
Copy link
Member Author

I guess a comprise can be done via IS_ENABLED().

@nashif nashif merged commit 01f8e0f into zephyrproject-rtos:main Nov 18, 2024
30 checks passed
@dcpleung dcpleung deleted the demand_paging/eviction_tracking branch November 18, 2024 18:51
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.

6 participants