add clean_up and clean_up_with_filter

[Freax13]

This pr adds support for deallocating unused page tables.

Closes #238

[Freax13]

I just noticed PageRange in the docs. I'll change my pr to use this later

[phil-opp]

Looks good to me overall, thanks a lot! Two things I noticed:

• Right now this is only implemented for the MappedPageTable type (and the derived OffsetPageTable), but not for the RecursivePageTable. If possible, I think it would be a good idea to make the method generic on the Mapper trait and implement it for the recursive page table type too.
• What are the use cases for clean_up_with_filter method? Its interface is quite complex with the filter function, so I wonder if a simpler interface based on an address range would be better (e.g. clean_up_addr_range). Or would it be too restricting? (Internally, we could of course still keep the filter-based function, so no change of the implementation would be necessary for this).

@josephlr Perhaps you could take a quick look at this too?

[Freax13]

Right now this is only implemented for the MappedPageTable type (and the derived OffsetPageTable), but not for the RecursivePageTable. If possible, I think it would be a good idea to make the method generic on the Mapper trait and implement it for the recursive page table type too.

I don't think it's possible to make this generic on Mapper alone because it's very high-level and doesn't support walking the page tables. It would be possible to do this with PageTableFrameMapping but there are no public implementations and RecursivePageTable doesn't implement it either.

I'm not entirely comfortable with recursive page tables just yet, so that's why I haven't implemented anything for them.

So I think there are three options:

1. just implement this for certain types like I already did
2. make it generic on PageTableFrameMapping, implement PageTableFrameMapping for RecursivePageTable and make those implementations public (don't implement them only on internal types)
3. Do something entirely different

What are the use cases for clean_up_with_filter method? Its interface is quite complex with the filter function, so I wonder if a simpler interface based on an address range would be better (e.g. clean_up_addr_range). Or would it be too restricting? (Internally, we could of course still keep the filter-based function, so no change of the implementation would be necessary for this).

In your comment on the initial issue (#238) you mention that the user should be able to integrate their own logic, so I tried to make it as customizable as possible. In hindsight I agree that this was perhaps needlessly over engineered. I like the idea of replacing clean_up_with_filter with clean_up_addr_range especially because it would also probably make the implementation much more readable. clean_up can also be implemented with clean_up_addr_range so there wouldn't be a need for an internal clean_up_with_filter.

[Freax13]

Right now this is only implemented for the MappedPageTable type (and the derived OffsetPageTable), but not for the RecursivePageTable. If possible, I think it would be a good idea to make the method generic on the Mapper trait and implement it for the recursive page table type too.

RecursivePageTables need their own implementation: They will never be completely empty because they always have an entry pointing to themselves.

EDIT:
I just started working on the implementation for RecursivePageTable and it turns out I misunderstood them before. I guess they still need some custom logic though because a clean up in a certain region should probably also clean up the corresponding recursive page table entries.

[Freax13]

I guess they still need some custom logic though because a clean up in a certain region should probably also clean up the corresponding recursive page table entries.

Or maybe not? Still struggling to wrap my head around this

[josephlr]

Added mostly minor nits, I'm not super familiar with the page-table code of this crate though.

[phil-opp]

I'm very sorry for the long delay on this, @Freax13! This is some great work and I should have reviewed it way sooner.

I added some more comments, otherwise this looks good to me. Given the long delay, I would understand if you don't want to work into this anymore. Please let me know in that case, then I'll try to fix the remaining things myself.

[Freax13]

I'm not sure why CI is failing, but I'm pretty sure it's unrelated to this pr.

[phil-opp]

Thanks a lot for the quick changes!

The CI failure was fixed on master already, so updating your PR to the latest master should fix it. Given that this PR is quite old already, I would recommend to just merge master into your branch instead of attempting a rebase.

[Freax13]

I saw that you no longer use the next branch, so I changed the base branch to master.

[phil-opp]

Yeah, I just noticed that as well. We normally only use it for PRs with breaking changes, other changes can be merged directly into master. I don't think that this contains any breaking changes, so I just wanted to propose changing the base branch. Thanks!

[phil-opp]

Thanks a lot!