From: David Hildenbrand
> Sent: 09 August 2022 21:57
...
These two functions seem to contain a lot of the same tests.
They also seem a bit large for 'inline'.
> -static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
> +/* FOLL_FORCE can write to even unwritable PTEs in COW mappings. */
> +static inline bool can_follow_write_pte(pte_t pte, struct page *page,
> + struct vm_area_struct *vma,
> + unsigned int flags)
> {
> - return pte_write(pte) ||
> - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
> + /* If the pte is writable, we can write to the page. */
> + if (pte_write(pte))
> + return true;
> +
> + /* Maybe FOLL_FORCE is set to override it? */
> + if (!(flags & FOLL_FORCE))
> + return false;
> +
> + /* But FOLL_FORCE has no effect on shared mappings */
> + if (vma->vm_flags & (VM_MAYSHARE | VM_SHARED))
> + return false;
> +
> + /* ... or read-only private ones */
> + if (!(vma->vm_flags & VM_MAYWRITE))
> + return false;
> +
> + /* ... or already writable ones that just need to take a write fault */
> + if (vma->vm_flags & VM_WRITE)
> + return false;
> +
> + /*
> + * See can_change_pte_writable(): we broke COW and could map the page
> + * writable if we have an exclusive anonymous page ...
> + */
> + if (!page || !PageAnon(page) || !PageAnonExclusive(page))
> + return false;
> +
> + /* ... and a write-fault isn't required for other reasons. */
> + if (vma_soft_dirty_enabled(vma) && !pte_soft_dirty(pte))
> + return false;
> + return !userfaultfd_pte_wp(vma, pte);
> }
...
> -static inline bool can_follow_write_pmd(pmd_t pmd, unsigned int flags)
> +/* FOLL_FORCE can write to even unwritable PMDs in COW mappings. */
> +static inline bool can_follow_write_pmd(pmd_t pmd, struct page *page,
> + struct vm_area_struct *vma,
> + unsigned int flags)
> {
> - return pmd_write(pmd) ||
> - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pmd_dirty(pmd));
> + /* If the pmd is writable, we can write to the page. */
> + if (pmd_write(pmd))
> + return true;
> +
> + /* Maybe FOLL_FORCE is set to override it? */
> + if (!(flags & FOLL_FORCE))
> + return false;
> +
> + /* But FOLL_FORCE has no effect on shared mappings */
> + if (vma->vm_flags & (VM_MAYSHARE | VM_SHARED))
> + return false;
> +
> + /* ... or read-only private ones */
> + if (!(vma->vm_flags & VM_MAYWRITE))
> + return false;
> +
> + /* ... or already writable ones that just need to take a write fault */
> + if (vma->vm_flags & VM_WRITE)
> + return false;
> +
> + /*
> + * See can_change_pte_writable(): we broke COW and could map the page
> + * writable if we have an exclusive anonymous page ...
> + */
> + if (!page || !PageAnon(page) || !PageAnonExclusive(page))
> + return false;
> +
> + /* ... and a write-fault isn't required for other reasons. */
> + if (vma_soft_dirty_enabled(vma) && !pmd_soft_dirty(pmd))
> + return false;
> + return !userfaultfd_huge_pmd_wp(vma, pmd);
> }
Perhaps only the initial call (common success path?) should
be inlined?
With the flags and vma tests being moved to an inline helper.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
