Hi Matthew,
I have a somewhat different patch for this. Let me know what you think. It depends on a few other patches in my tree, so probably won't compile for you.
I don't have extensive experience with folios or anything but on the whole it looks good to me. I like the use of `mapping` to dodge the compound_head() checks. Beyond that, only a few things caught my eye.
- if (PageKsm(page)) + if (mapping & PAGE_MAPPING_KSM) u |= 1 << KPF_KSM;
This might need an #ifdef? Say mapping is movable and anon -- then (mapping & PAGE_MAPPING_KSM) is true. Before, we called PageKsm, which falls through to a PG_ksm check. If !CONFIG_KSM then that flag is always false. But now, we're liable to report KPF_KSM even if !CONFIG_KSM.
/* * compound pages: export both head/tail info * they together define a compound page's start/end pos and order */ - if (PageHead(page)) - u |= 1 << KPF_COMPOUND_HEAD; - if (PageTail(page)) + if (page == &folio->page) + u |= kpf_copy_bit(k, KPF_COMPOUND_HEAD, PG_head); + else u |= 1 << KPF_COMPOUND_TAIL;
This makes sense but it'd require changes to the documentation. I ran a python3 memhog to see if anonymous pages are currently reported as COMPOUND_HEAD or COMPOUND_TAIL and it seems to be a no on both. But with this, I think every pfn will have one of the two set. Unless you can have a page outside of a folio -- not sure. Also, in
- if (page_is_idle(page)) +#if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT) + u |= kpf_copy_bit(k, KPF_IDLE, PG_idle); +#else + if (folio_test_idle(folio)) u |= 1 << KPF_IDLE; +#endif
and
- if (PageSwapCache(page)) +#define SWAPCACHE ((1 << PG_swapbacked) | (1 << PG_swapcache)) + if ((k & SWAPCACHE) == SWAPCACHE) u |= 1 << KPF_SWAPCACHE; u |= kpf_copy_bit(k, KPF_SWAPBACKED, PG_swapbacked);
it seems to me like the #ifdef/#define could be supplanted by folio_test_idle and folio_test_swapcache. But I guess those would require extra folio_flags queries and an #include <page_idle.h>. So if this is more performant, I can understand the design. Best, Svetly
