On 05.12.23 13:04, Ryan Roberts wrote:
On 04/12/2023 14:21, David Hildenbrand wrote:
Let's get rid of the compound parameter and instead define implicitly
which mappings we're adding. That is more future proof, easier to read
and harder to mess up.
Use an enum to express the granularity internally. Make the compiler
always special-case on the granularity by using __always_inline.
Add plenty of sanity checks with CONFIG_DEBUG_VM. Replace the
folio_test_pmd_mappable() check by a config check in the caller and
sanity checks. Convert the single user of folio_add_file_rmap_range().
This function design can later easily be extended to PUDs and to batch
PMDs. Note that for now we don't support anything bigger than
PMD-sized folios (as we cleanly separated hugetlb handling). Sanity checks
Is that definitely true? Don't we support PUD-mapping file-backed DAX memory?
They are not handled via the rmap. Otherwise, all the PMD accounting
(e.g., FilePmdMapped) in RMAP code would already be wrong for them.
And it's easy to verify by looking at zap_huge_pud() that doesn't call
any rmap code.
[...]
+
+static inline void __folio_rmap_sanity_checks(struct folio *folio,
+ struct page *page, unsigned int nr_pages, enum rmap_mode mode)
+{
+ /* hugetlb folios are handled separately. */
+ VM_WARN_ON_FOLIO(folio_test_hugetlb(folio), folio);
+ VM_WARN_ON_FOLIO(folio_test_large(folio) &&
+ !folio_test_large_rmappable(folio), folio);
+
+ VM_WARN_ON_ONCE(!nr_pages || nr_pages > folio_nr_pages(folio));
nit: I don't think you technically need the second half of this - its covered by
the test below...
My thinking was that if nr_pages would be "-1", one could end up with
weird wraparounds.
But yeah, I thought about this as well and might just remove it.
+ VM_WARN_ON_FOLIO(page_folio(page) != folio, folio);
+ VM_WARN_ON_FOLIO(page_folio(page + nr_pages - 1) != folio, folio);
...this one.
Thanks!
--
Cheers,
David / dhildenb
