On the other hand, with a pagefault_disabled-like approach, there is no
way to instruct call {3} to fully exit lazy_mmu regardless of the
nesting level.
Sure there is, with a better API. See below. :)
I meant while keeping the existing shape of the API but yes fair enough!
Time to do it properly I guess :)
[...]
Assume we store in the task_struct
uint8_t lazy_mmu_enabled_count;
bool lazy_mmu_paused;
I didn't think of that approach! I can't immediately see any problem
with it, assuming we're fine with storing arch-specific context in
thread_struct (which seems to be the case as things stand).
Right, just to complete the picture:
a) We will have some CONFIG_ARCH_LAZY_MMU
b) Without that config, all lazy_mmu_*() functions are a nop and no
lazy_mmu_state is stored in task_struct
struct lazy_mmu_state {
uint8_t enabled_count;
bool paused;
}
c) With that config, common-code lazy_mmu_*() functions implement the
updating of the lazy_mmu_state in task_struct and call into arch code
on the transition from 0->1, 1->0 etc.
Maybe that can be done through exiting
arch_enter_lazy_mmu_mode()/arch_leave_lazy_mmu_mode() callbacks, maybe
we need more. I feel like
we might be able to implement that through the existing helpers.
We can do things like
a) Sanity check that while we are paused that we get no more
enable/disable requests
b) Sanity check that while we are paused that we get no more pause
requests.
These are good points - and this is only possible with such global
state. (Similarly we can check that the counter never underflows.)
Exactly.
[..]
Overall what you're proposing seems sensible to me, the additional
fields in task_struct don't take much space and we can keep the API
unchanged in most cases. It is also good to have the option to check
that the API is used correctly. I'll reply to the cover letter to let
anyone who didn't follow this thread chip in, before I go ahead and try
out that new approach.
And on top of the proposal above we will have some
struct arch_lazy_mmu_state;
define by the architecture (could be an empty struct on most).
We can store that inside "struct lazy_mmu_state;" or if we ever have to,
start returning only that from the enable/disable etc. functions.
For now, I'd say just store it in the task struct in the lazy_mmu_state.
But we can always adjust later if required.
In the first (this) series we probably don't even have to introduce
arch_lazy_mmu_state.
--
Cheers
David / dhildenb
