MIME-Version: 1.0
References: <20230218002819.1486479-1-jthoughton@google.com> <20230218002819.1486479-2-jthoughton@google.com>
In-Reply-To: <20230218002819.1486479-2-jthoughton@google.com>
From:   Mina Almasry <almasrymina@google.com>
Date:   Fri, 17 Feb 2023 16:41:57 -0800
Message-ID: <CAHS8izM5Es+DMiOyPt93o6Xw+SeuOR45r+MMoeagiYN594NiyQ@mail.gmail.com>
Subject: Re: [PATCH v2 01/46] hugetlb: don't set PageUptodate for UFFDIO_CONTINUE
To:     James Houghton <jthoughton@google.com>
Cc:     Mike Kravetz <mike.kravetz@oracle.com>,
        Muchun Song <songmuchun@bytedance.com>,
        Peter Xu <peterx@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        David Hildenbrand <david@redhat.com>,
        David Rientjes <rientjes@google.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        "Zach O'Keefe" <zokeefe@google.com>,
        Manish Mishra <manish.mishra@nutanix.com>,
        Naoya Horiguchi <naoya.horiguchi@nec.com>,
        "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
        "Matthew Wilcox (Oracle)" <willy@infradead.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Baolin Wang <baolin.wang@linux.alibaba.com>,
        Miaohe Lin <linmiaohe@huawei.com>,
        Yang Shi <shy828301@gmail.com>,
        Frank van der Linden <fvdl@google.com>,
        Jiaqi Yan <jiaqiyan@google.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Feb 17, 2023 at 4:28=E2=80=AFPM James Houghton <jthoughton@google.c=
om> wrote:
>
> If would be bad if we actually set PageUptodate with UFFDIO_CONTINUE;
> PageUptodate indicates that the page has been zeroed, and we don't want
> to give 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@google.com>
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 07abcb6eb203..792cb2e67ce5 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6256,7 +6256,16 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst=
_mm,
>          * preceding stores to the page contents become visible before
>          * the set_pte_at() write.
>          */
> -       __folio_mark_uptodate(folio);
> +       if (!is_continue)
> +               __folio_mark_uptodate(folio);
> +       else if (!folio_test_uptodate(folio)) {
> +               /*
> +                * This should never happen; HugeTLB pages are always Upt=
odate
> +                * as soon as they are allocated.
> +                */

if (is_continue) then we grab a page from the page cache, no? Are
pages in page caches always uptodate? Why? I guess that means they're
mapped hence uptodate?

Also this comment should explain why pages in the page cache are
always uptodate, no? Because this error branch is hit if (is_continue
&& !folio_test_uptodate()), not when pages are freshly allocated.

> +               ret =3D -EFAULT;
> +               goto out_release_nounlock;
> +       }
>
>         /* Add shared, newly allocated pages to the page cache. */
>         if (vm_shared && !is_continue) {
> --
> 2.39.2.637.g21b0678d19-goog
>