On Fri, Oct 21, 2022 at 04:36:17PM +0000, James Houghton wrote:
> This is how it should have been to begin with. It would be very bad if
> we actually set PageUptodate with a UFFDIO_CONTINUE, as UFFDIO_CONTINUE
> doesn't actually set/update the contents of the page, so we would be
> exposing a non-zeroed page to the user.
>
> The reason this change is being made now is because UFFDIO_CONTINUEs on
> subpages definitely shouldn't set this page flag on the head page.
>
> Signed-off-by: James Houghton <jthoughton@xxxxxxxxxx>
> ---
> mm/hugetlb.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 1a7dc7b2e16c..650761cdd2f6 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6097,7 +6097,10 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm,
> * preceding stores to the page contents become visible before
> * the set_pte_at() write.
> */
> - __SetPageUptodate(page);
> + if (!is_continue)
> + __SetPageUptodate(page);
> + else
> + VM_WARN_ON_ONCE_PAGE(!PageUptodate(page), page);
Yeah the old code looks wrong, I'm just wondering whether we can 100%
guarantee this for hugetlb. E.g. for shmem that won't hold when we
uffd-continue on a not used page (e.g. by an over-sized fallocate()).
Another safer approach is simply fail the ioctl if !uptodate, but if you're
certain then WARN_ON_ONCE sounds all good too. At least I did have a quick
look on hugetlb fallocate() and pages will be uptodate immediately.
>
> /* Add shared, newly allocated pages to the page cache. */
> if (vm_shared && !is_continue) {
> --
> 2.38.0.135.g90850a2211-goog
>
>
--
Peter Xu
