+int break_ksm_pud_entry(pud_t *pud, unsigned long addr, unsigned long next,
struct mm_walk *walk)+{
- /* We only care about page tables to walk to a single base page. */
- if (pud_leaf(*pud) || !pud_present(*pud))
return 1;- return 0;
+}
Is this needed? I thought the pgtable walker handlers this already.
[...]
Most probably yes. I was trying to avoid about PUD splits, but I guess we simply should not care in VMAs that are considered by KSM (MERGABLE). Most probably never ever happens.
static int break_ksm(struct vm_area_struct *vma, unsigned long addr) {
- struct page *page; vm_fault_t ret = 0;
- if (WARN_ON_ONCE(!IS_ALIGNED(addr, PAGE_SIZE)))
return -EINVAL;- do { bool ksm_page = false;
cond_resched();
page = follow_page(vma, addr,FOLL_GET | FOLL_MIGRATION | FOLL_REMOTE);if (IS_ERR_OR_NULL(page))break;if (PageKsm(page))ksm_page = true;put_page(page);
ret = walk_page_range_vma(vma, addr, addr + PAGE_SIZE,&break_ksm_ops, &ksm_page);if (WARN_ON_ONCE(ret < 0))return ret;I'm not sure this would be worth it, especially with a 4% degrade. The next patch will be able to bring 50- LOC, but this patch does 60+ anyway, based on another new helper just introduced...
I just don't see whether there's strong enough reason to do so to drop FOLL_MIGRATE. It's different to the previous VM_FAULT_WRITE refactor because of the unshare approach was much of a good reasoning to me.
Perhaps I missed something?
My main motivation is to remove most of that GUP hackery here, which is 1) Getting a reference on a page and waiting for migration to finish even though both is unnecessary. 2) As we don't have sufficient control, we added FOLL_MIGRATION hacks to MM core to work around limitations in the GUP-based approacj. 3) We rely on legacy follow_page() interface that we should really get rid of in the long term.
All we want to do is walk the page tables and make a decision if something we care about is mapped. Instead of leaking these details via hacks into GUP code and making that code harder to grasp/maintain, this patch moves that logic to the actual user, while reusing generic page walking code.
Yes, we have to extend page walking code, but it's just the natural, non-hacky way of doing it.
Regarding the 4% performance degradation (if I wouldn't have added the benchmarks, nobody would know and probably care ;) ), I am not quite sure why that is the case. We're just walking page tables after all in both cases. Maybe the callback-based implementation of pagewalk code is less efficient, but we might be able to improve that implementation if we really care about performance here. Maybe removing break_ksm_pud_entry() already improves the numbers slightly.
Thanks!