[...]
VM_BUG_ON(haddr & ~HPAGE_PMD_MASK);
VM_BUG_ON_VMA(vma->vm_start > haddr, vma);
VM_BUG_ON_VMA(vma->vm_end < haddr + HPAGE_PMD_SIZE, vma);
- VM_BUG_ON(!is_pmd_migration_entry(*pmd) && !pmd_trans_huge(*pmd));
+
+ VM_WARN_ON(!is_pmd_migration_entry(*pmd) && !pmd_trans_huge(*pmd) &&
+ !is_pmd_device_private_entry(*pmd));
Indentation. But I do wonder if we want a helper to do a more
efficient
The indentation at my end is fine, do you mean you want to see the conditions
aligned?
If that's the case then all good (sometimes the added +/-/" " in the
diff mess it up and confuse me)
is_pmd_migration_entry() || is_pmd_device_private_entry()
If only I could come up with a good name ... any ideas?
is_non_present_folio_entry()
maybe?
There is is_pfn_swap_entry(), but that includes hwpoison entries as well.
Yes, we could just use that I guess. Or alternatively add a
is_non_present_folio_entry() that does not include hwpoison (because
they are special).
Well, there is device-exclusive .... but that would not be reachable on these
paths yet, ever.
count_vm_event(THP_SPLIT_PMD);
@@ -2937,18 +2940,43 @@ static void __split_huge_pmd_locked(struct
vm_area_struct *vma, pmd_t *pmd,
return __split_huge_zero_page_pmd(vma, haddr, pmd);
}
- pmd_migration = is_pmd_migration_entry(*pmd);
- if (unlikely(pmd_migration)) {
- swp_entry_t entry;
+ present = pmd_present(*pmd);
+ if (unlikely(!present)) {
I hate this whole function. But maybe in this case it's better
to just have here
if (is_pmd_migration_entry(old_pmd)) {
} else if (is_pmd_device_private_entry(old_pmd)) {
There is not much shared code and the helps reduce the indentation level.
We can definitely try this
+ swp_entry = pmd_to_swp_entry(*pmd);
old_pmd = *pmd;
- entry = pmd_to_swp_entry(old_pmd);
- page = pfn_swap_entry_to_page(entry);
- write = is_writable_migration_entry(entry);
- if (PageAnon(page))
- anon_exclusive = is_readable_exclusive_migration_entry(entry);
- young = is_migration_entry_young(entry);
- dirty = is_migration_entry_dirty(entry);
+
+ folio = pfn_swap_entry_folio(swp_entry);
+ VM_WARN_ON(!is_migration_entry(swp_entry) &&
+ !is_device_private_entry(swp_entry));
Indentation.
Same as above, checkpatch.pl does not seem to complain
Make sure that both "!" are equally indented. checkpatch does not catch
what we usually do in MM (if you read other code).
[...]
soft_dirty = pmd_swp_soft_dirty(old_pmd);
uffd_wp = pmd_swp_uffd_wp(old_pmd);
} else {
@@ -3034,30 +3062,49 @@ static void __split_huge_pmd_locked(struct
vm_area_struct *vma, pmd_t *pmd,
* Note that NUMA hinting access restrictions are not transferred to
* avoid any possibility of altering permissions across VMAs.
*/
- if (freeze || pmd_migration) {
+ if (freeze || !present) {
Here too, I wonder if we should just handle device-private completely
separately for now.
For all practical purposes they are inside the if statement. freeze and
is_migration need to go together.
Right, but there is only the loop part share between
freeze || is_migration_entry(swp_entry)
and device_private entries.
So we can just avoid that churn and handle device_private entries
separately, right?
--
Cheers
David / dhildenb
