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 866B7C636D7 for ; Tue, 21 Feb 2023 19:58:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 227756B0075; Tue, 21 Feb 2023 14:58:59 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1B16E6B007B; Tue, 21 Feb 2023 14:58:59 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 02A946B007D; Tue, 21 Feb 2023 14:58:58 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id DEAFE6B0075 for ; Tue, 21 Feb 2023 14:58:58 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id B8F5C1C5C94 for ; Tue, 21 Feb 2023 19:58:58 +0000 (UTC) X-FDA: 80492362356.14.F61616A Received: from mail-vs1-f42.google.com (mail-vs1-f42.google.com [209.85.217.42]) by imf19.hostedemail.com (Postfix) with ESMTP id 04C121A000C for ; Tue, 21 Feb 2023 19:58:56 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=O7FR1DNz; spf=pass (imf19.hostedemail.com: domain of jthoughton@google.com designates 209.85.217.42 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=1677009537; 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=DF0KNJghjUV7wJh512cQZNMEw1nvR5xMFp/83eAe6+M=; b=m5zGJIn3djzac8U1xPjmaNND7CeOR1gIDsirG3RV3Hj5Ns3tLNpu37klA5e+aWIdUT8jrT q7/5h078G6pJwfa3IttTQOPk5Dv697a/L5AEiJUjUIMvEB5LFemRxVm6jZL3pe8F4xPDQm 6NFRCDMwlj5gU5GgeOzBv0pavcW+MBs= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=O7FR1DNz; spf=pass (imf19.hostedemail.com: domain of jthoughton@google.com designates 209.85.217.42 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=1677009537; a=rsa-sha256; cv=none; b=WW/sJcjnxjZnvldMTKK+PhjHyoiL/7GIKOdVUOUwBFuNskVkdkDkLed9o+L3eHEan0hczM eb9FLT8PLdwe9PQz0GbZCRBLXQqkkcbnB9lB0LBVZPbJoE7e9jOz/1U9N7gpLgx/BUmCcc yotw+vawlww+aGOhezmkUGck84wLqYg= Received: by mail-vs1-f42.google.com with SMTP id 6so6093564vsv.7 for ; Tue, 21 Feb 2023 11:58:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1677009536; 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=DF0KNJghjUV7wJh512cQZNMEw1nvR5xMFp/83eAe6+M=; b=O7FR1DNzUbas3YRCPSnDIgVMvpW4evNW+U818zCom4X2UeJi0dgPlpatukFFI2jHgl OLA0ICVxN68LNdowqQZRw6YHeqBtycyj86+YWkW+5KpqEnPFJt3igdN1lLkWng/LQsWW 0BOp77eVuiaBlAdmzWVcN2/3QK2Xzc5aJcA97ywETPAbbzdXmNdDIyvPYqMo66Exdsw+ 5FQJdLLddxwcyJTQHWAU6PrsXeYpIZDtGdRow0xNfF47aCF4UvTARQb7JmKfrXpPIXtd BbMClX41M3PoQ5RIR/L+eneSMsgJLXAbsESqIUDraACr5YEtlUBpSBXTCaUdbVOXyawZ ze6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677009536; 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=DF0KNJghjUV7wJh512cQZNMEw1nvR5xMFp/83eAe6+M=; b=xue2vFj58dAPNqAS9MlKJrC9wIJdCpajog/W2LLQhaCGhi2tasgFf43zKJkaNvbSpt /2Is74w3D/LxqfbTEyQ8NcXfep6DimZb1+2BtJkBwknnp6H/pTDnT3XdyZjQ+dZQHUdF wwVEOMPz4uJ9L3yNiD7zjA/GXmph5ew6uEs3sZjjDXwm071EZ8fn1i1DuClvS/YFe6z8 72CduBX191k0pzVkIb7/H9iOKjnUC38WCIi3BGw6WGIZxi3abmL161Y+1pWRU2qKPC6N XYEJMswmna8/1h9IIQ5jS+QxXvk2OL+4oP1GVE8csFQe6/wbpGstG4PZvwNmh8fuGRTY 98eg== X-Gm-Message-State: AO0yUKXhOQVdEBMnFzNnvtKn2OV506vjwS5iIBTXgcXsCu4zvV+d625U 2BgOQFRdS8Tb/AAZuQT6YpkAFX48a9iMHmqwNv0aWQ== X-Google-Smtp-Source: AK7set/OBfSTihP3rfzNxKV6mPiXYOTPbFTsjj4Q9W0m/pL2cgeM0fB1vkcpq/SRM9VoGG1lEOcmqmbxlnCbND7aURk= X-Received: by 2002:a67:e081:0:b0:41e:910f:10ed with SMTP id f1-20020a67e081000000b0041e910f10edmr803070vsl.11.1677009535939; Tue, 21 Feb 2023 11:58:55 -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 11:58:19 -0800 Message-ID: Subject: Re: [PATCH v2 01/46] hugetlb: don't set PageUptodate for UFFDIO_CONTINUE To: Mike Kravetz Cc: Mina Almasry , 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-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 04C121A000C X-Stat-Signature: 5apydpyxcdutxg5aokxp47aimj7dwd7x X-Rspam-User: X-HE-Tag: 1677009536-282781 X-HE-Meta: U2FsdGVkX192/E4q4lO8FaisLaLSi9wCrSZ5rxJ0g94EFIgmD5FbO/MhLKz9zpild/Id5HQg11axfBpPqD86dBiyTGcx6FkJmfRwJu17nBLIeQi2eXjOHJ9MtJntoGNO88SmSuLfc0ymOy5kwuZB8u34khHDh3lhU+dAxYRMj/twCwEErqkwwQWbbXyd/kunxteVKoPRuxxIu18TJ4yMM9gAL9tU7AdSrbFKMp1ekGc1TnfSs++q5VQ5+Dm2rp3TcSswDFhH5GEMYM2sNaKO1EEYm0mB2zwE5xe+cLz7qG1S0s8+0Id8wR14xpzSz1q7PylpFvmVPVNGH7Dmwu5xqSkbBAIDh74fdBFv4JOAimtHaVmr737d78auFmZAazzh4afIvjBkqvs4tLyl2ITPiTliKhRsCoYJMP/NNt813lq2GFksI/inaMmNX8V7GJRn+kNBLJU29aszfetNrib2fibPYjDIgwWCQcppM/AAztAFhyrP6bg4r9Z5tnDJ4W45auOx/XLWVWMTHm9SxkAUCcyrjcmREzpeDoTxRwMxXj0xUmXjalB04m8kn/oo4AbH/buLgtGFVb9YNs4FbBBjq064md9ythZMA3s2+pGZ5VFCU66Ien31y/K8TCu7DCXe9r9eddrIiKizr18PrPkGol5p1LoNsn+1QMxURtddZqJFWyV4g4GeISPtcXf9t8A7W4lhg4YdeAlhqVVx+amOp8/vuphC6EyMSwpHvYxYA5H3EUi7vIIGSnPPzc9ouAbJijWR1xPVwyRvhMJXm5IMEIsOrgJBGCVFOj7UYKoCmmJts8TGHyKcLadE8ownZbp0wSMBBV++BGWYY/MWIUKMHQmKRg/dZUOvAWY71rMEZUdfBHj0HDP7OhEq1FpkdeKpR6X1WbM11fDcu+ICFf7/8DoaOWuy4QLDWj1aroZgc1XWe5h2+3F1PzEL2B94b0xb9kTwgz7zh8656IF9/hS H6R2ICtb fgP87YyZYZQ0McwAW4rqfhnOSMoIkrYG1kUsPeqbYl5JY+nBhQ/0ZfbKePGGmiHDAcmP83CtbaAoRwK8nHvZhNtCYciGP9AL/XT2nO0Lm3R9ky2kI3T2mbJ2QoMJVZS/yI/sLMgoy7LLKfcc2cXdDCYAs7KWoiBnVxwsQVAr+xeJWehP4agidy8hj/YV/VRe5DUnMuII5s+h4qvQc1LCxYXP+RXRH/HrrQ34I7LPaE22rOnF5Y+gQOGcZBXuGheB8cPhQguNrhZbTOvFNL6PX/HkwQbJYx1stK0WsvJj6Ua9RcBBfVbN5L8ftpi+IjNt7F7+pNUZguCkVekyq3ZKECKYfjQ== 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 Tue, Feb 21, 2023 at 11:34 AM Mike Kravetz wro= te: > > On 02/21/23 07:59, James Houghton wrote: > > On Fri, Feb 17, 2023 at 4:42 PM Mina Almasry w= rote: > > > > > > 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_CONTINU= E; > > > > 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_CONTINUE= s 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_struc= t *dst_mm, > > > > * preceding stores to the page contents become visible bef= ore > > > > * 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 alwa= ys Uptodate > > > > + * 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. > > Agree! > > > 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 i= f > > 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." > > Perhaps a better way of saying is that hugetlb pages are marked uptodate > shortly after allocation when their contents are initialized. Initialize= d > data could be zero, or it could be contents copied from another location > (such as in the UFFDIO_COPY case also handled in this routine). I'll write something like this. Thank you! > > Saying "PageUptodate indicates that the page has been zeroed" as in the > commit message is technically not correct. And I'll make sure to update the commit description as well. > > Ack to the patch. Thanks, Mike!