From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5BCF5C636D7 for ; Tue, 21 Feb 2023 16:00:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 91ADA6B0071; Tue, 21 Feb 2023 11:00:37 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8CB476B0072; Tue, 21 Feb 2023 11:00:37 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7939A6B0073; Tue, 21 Feb 2023 11:00:37 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 68ACB6B0071 for ; Tue, 21 Feb 2023 11:00:37 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 225081A0837 for ; Tue, 21 Feb 2023 16:00:37 +0000 (UTC) X-FDA: 80491761714.11.0F354FB Received: from mail-vs1-f49.google.com (mail-vs1-f49.google.com [209.85.217.49]) by imf17.hostedemail.com (Postfix) with ESMTP id 24DDF40034 for ; Tue, 21 Feb 2023 16:00:33 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=ZyCVXGpI; spf=pass (imf17.hostedemail.com: domain of jthoughton@google.com designates 209.85.217.49 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1676995234; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=gEGsGmTzHdzT9knfoHKHRGvhNUMsGTlXEeNMDOHsjGc=; b=O6NrXHCFrP+IqUyMlPVS4qUSJ2r4GEvc3k45Lfq8wcb4l7K1T+U+t1Y+ynKWWTUBvrVf7r P1EigDK7Csowy4RyFIGWTpLp39u414l15KxIBJzcj98UjM+eUhNkewucR1AnRkjgCqEMCo UcLduITe1Pi8lvtVgD1ebVsA9zKAZ14= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=ZyCVXGpI; spf=pass (imf17.hostedemail.com: domain of jthoughton@google.com designates 209.85.217.49 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1676995234; a=rsa-sha256; cv=none; b=VZVSerQdb+MqmQ+cuU333pbPG6bXKJukzcPmwAV3tZkH84tGjIMYPY790i2Cioch0tNYTa ahVPFiuxkyQGyS+Kf7xY7VTED+OWvPIsdInJLFTiw10STBjIZKXkymGEvlSO1BInQDeQey keROz/st+VcaYjYMs+IghYnUC2ogfRc= Received: by mail-vs1-f49.google.com with SMTP id m10so3091723vso.4 for ; Tue, 21 Feb 2023 08:00:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=gEGsGmTzHdzT9knfoHKHRGvhNUMsGTlXEeNMDOHsjGc=; b=ZyCVXGpI7+v5rkLMYZIW9t2A0sb5StXS66ZGe3cubmw41jnMmgqMg5nK+9qNgVZnKl g9hcY039uN6jzrvDD3h2cbUjzryCjNsf6ZFjEtJ09Q3q3NcCYk2XGB7GJBmsasszuleF 3LZSHl6OlnOZTrThG6VVIYsiO6NAfHzyuDYq1VdjvoQUeDMicr/kIph7+bqAfZ5yZOVT KKUj1SrL5QbW9/GGRhEeTRWX608JPKslol+S7IpoZ9YW98Z/FrHO7C8g5PsRBUcKfuQz lcAVpgHzYKzk/+EuFxLuL5cgEg/Lg+4SBv5zCC7XiSaoE+ne4YM+O8wltoxS+xCWYHCR 5oNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gEGsGmTzHdzT9knfoHKHRGvhNUMsGTlXEeNMDOHsjGc=; b=YB4Sc0W4eJHssQ6DFp1oNay9cGlb2wgaSTYl1wC1E0b18bXP+zdo0yDPzl09Mz3flM GNNVwU4OQu5ZWt3efljkTn0A9SV+riwUguVfLw8CevAWbBHAAGFfcXnpOX6JS9h0EMrG lRCUKRO/rOFJLzVJn0RsNspiZWhq++gXojnkDOVLAeObvktx6y0xeW5pRNW1BoUeDgRg TbO0iFAn4yiQtUcDupYAfePEGkKIzpI1pJBpjmNpiRcL7NRyXpMQzw45Ij29Vn++IFR7 yhbHlJA0AL4PTYlu6PNnKK/qAFqb21atllPhlH8EZgg7dySeEUE3J1d68vxmlcnLjyuQ spFw== X-Gm-Message-State: AO0yUKWSHjxXKxar9mU14EiM0ZH31i5Z/coz50Qw/JU/HB9pgGZQU+ZX ho3/iMdzKC2ssKQvGmaCtXQ3PH8huvJf7ZNfF9i9vQ== X-Google-Smtp-Source: AK7set96mNcnHjcj9pZjuGPPdOQ+l30zoQUNRCrKVvvkxSlowK5hHK5inugxItL4+VkeVb740A1gSax4vCrdZPv0fRo= X-Received: by 2002:a67:dc81:0:b0:41b:ed91:4d51 with SMTP id g1-20020a67dc81000000b0041bed914d51mr1364819vsk.84.1676995233019; Tue, 21 Feb 2023 08:00:33 -0800 (PST) MIME-Version: 1.0 References: <20230218002819.1486479-1-jthoughton@google.com> <20230218002819.1486479-2-jthoughton@google.com> In-Reply-To: From: James Houghton Date: Tue, 21 Feb 2023 07:59:56 -0800 Message-ID: Subject: Re: [PATCH v2 01/46] hugetlb: don't set PageUptodate for UFFDIO_CONTINUE To: Mina Almasry Cc: Mike Kravetz , Muchun Song , Peter Xu , Andrew Morton , David Hildenbrand , David Rientjes , Axel Rasmussen , "Zach O'Keefe" , Manish Mishra , Naoya Horiguchi , "Dr . David Alan Gilbert" , "Matthew Wilcox (Oracle)" , Vlastimil Babka , Baolin Wang , Miaohe Lin , Yang Shi , Frank van der Linden , Jiaqi Yan , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: 6xazx5qqf153wr77a91t8j3ksu5k1fad X-Rspamd-Queue-Id: 24DDF40034 X-HE-Tag: 1676995233-740724 X-HE-Meta: U2FsdGVkX1+O9UfS7l0+jQP2eD4jZjJdQ1cVpoiLXJFT7ySue6L+RuZiFLPvfUHiM/J4p10AsrVFzZhavKJyTRBRyPzlayJMWVL0OcQq/wK/jCfFDD8L0MZD09JtwGLQOAhijXdLXUiequhnns57/3tjVaDj5vwG7VZaC5E5FzJ7UOD2BbdU8495OLIK6umuWNM83gwW8fsLQjMC8wH7P3DwpzF4KIXlVqAF3pibGkaYTpW/fvuDLNaigRmiWVwQmqiymAMSgdRB9BQ3lCezEISYoqvNbD4uM0GGpFlU3/RLHHIaybnEqSjCelLMT8wO1GUZjH46Dh2uKg9mHCDJ3xoL37E4y/pTfniUsYP5ssFpSMaA/6MAaPy6QwgO/Q6o4GYIwScPgMMNfs9zNYyMruh33Iyf8MbEKpBlH/o5/i941G9v/9CxwNXPfFiyHzK8DEm7gLKUZcGtq5hDrh6nBuceO9kenayWrFaB6pWQIU20+My4i4CbQzliRK51SWlVgQwnXtiw9zzYgKNy5f8B8MabR5y4MyFqH26TOKuI0hOvbqoeBNddd+jaiHnS6sILb2Lya+dZ47hNsqolQP3OJW91z/mP+jfQG9aVeL4iyjdnSsAX98H2tW00Sf4lS+EM+gRipQ7dCKvnT+kbl6Poj8hqveTEszscW5knq7p5rvwD0JlwkUx6e47DWTlZrgxWlpQiijJYiOFw00yFty+oddWKvLZInI1BSF5za1ius1zsmPHNdemvMO2aESDtcoo4II3Gvt+KFuaLw9QJR0FQbCBblLfkW4e4ukBBmRV9xzW/UQyBaYPplVqFc6gP7fifNWK6IAQmDIKHNWX68p9nhmPoQtnjOWNgXs5Ecuqs8+mMn0hExLQJ5Vav0Nch/OQvPJugP3fNUF0iMawypO2J3R1KCOuDreL8xadGrwFpaWTg+LrGTkqOJFBYrPh2U9zPGF50H3ydFEjMz9eZGrk swhtRuSG gY0nwdkP4nD16vJNHCfszLWHh7iTw2ygkCASV33e7/X35JU09koc/YiSU8twsqYYRz5q5vKlt78LbHz5FGxru1laCDgvXEk/9oi7O+fMS693Qu5dhczOAkySP8Jk+xBjzCjYP7PZgsvEBHw21djzEJ4yiLnEnMfCqeReShMsV/VaKCP3ck8ZZ0Ci1Sw/5Y0usHoFp87Ktxs4Q1x4jnpCEcueGrPYRWjNn3EVrSyrkVYKVhZ4E2aJ8m4EGrHERP3fCWdGafAds8XM1HBjBY7jF7UCw9x+iDGWo6BINaq4oxtjkfxnlTpg3jBPc/dfL386bTzAH5uriPQsga57KB34pjenAgDBPEt3y8Mk9X6OHKvtLA1K9Rtp0HkNW/Q== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Feb 17, 2023 at 4:42 PM Mina Almasry wrote= : > > On Fri, Feb 17, 2023 at 4:28=E2=80=AFPM James Houghton 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 > > > > 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 *d= st_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 U= ptodate > > + * 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. There was some discussion about it here[1]. Without even thinking about how the pages become uptodate, I think this patch is justified like this: UFFDIO_CONTINUE =3D> we aren't actually changing the contents of the page, so we shouldn't be changing the uptodate-ness of the page. HugeTLB pages in the page cache are always uptodate: 1. fallocate -- the page is allocated, zeroed, marked as uptodate, and then placed in the page cache. 2. hugetlb_no_page -- same as above. So uptodate <=3D> "the page has been zeroed", so it would be very bad if we gave a !uptodate page to userspace via UFFDIO_CONTINUE. I'll update the comment to something like: "HugeTLB pages are always Uptodate as soon as they are added to the page cache. Given that we aren't changing the contents of the page, we shouldn't be updating the Uptodate-ness of the page." [1]: https://lore.kernel.org/linux-mm/Y5JrS4o5Detzid9V@monkey/ Thanks, Mina. :) - James