Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 8 Dec 2022 14:55:07 -0800
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     James Houghton <jthoughton@google.com>
Cc:     Peter Xu <peterx@redhat.com>,
        Muchun Song <songmuchun@bytedance.com>,
        David Hildenbrand <david@redhat.com>,
        David Rientjes <rientjes@google.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        Mina Almasry <almasrymina@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>,
        Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v2 01/47] hugetlb: don't set PageUptodate for
 UFFDIO_CONTINUE
Message-ID: <Y5JrS4o5Detzid9V@monkey>
References: <20221021163703.3218176-1-jthoughton@google.com>
 <20221021163703.3218176-2-jthoughton@google.com>
 <Y3UQCmlnQXfKhbyE@x1n>
 <CADrL8HXXT+7BBMj3kgR8g=z4RZv0q-AF5ykkg1vXF9aYCWg9MA@mail.gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CADrL8HXXT+7BBMj3kgR8g=z4RZv0q-AF5ykkg1vXF9aYCWg9MA@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?VAWjxM59SXlqTJsgU9OxwMoYa/Mbsv6u8o8Woh2ExoY9l7t/nfKH9Q230BnK?=
 =?us-ascii?Q?u/IjA/RXkdHlZFZhbuhbBxQ9BgRqUO36627JJl0zCZnLTmzKkUoY+HCVQIww?=
 =?us-ascii?Q?U8VVqzGqz+UBisfffCe6LVCF/hVrSqGcw8PqZijRdBYO5WUgI88eDV0opXYm?=
 =?us-ascii?Q?yIebt2h9SOFv7OvZUuQnXXoxdgSXNalM+lNi96oxJUegzMob0utqwcDXo/OI?=
 =?us-ascii?Q?HPswGMJ/OOfx8Mc2NfGFPvDH8fpdL46dncMq0HTeQA5wvupft6WN51VWtD0Z?=
 =?us-ascii?Q?uzc9xPphnIacD0Mqb4iCyJpOsxH2sLgcw7pczwMloeo18eAF4mP4PTsmkZ5U?=
 =?us-ascii?Q?XwCl/aZOoKa+KIk9sl7aEbmHpNkmMG5XNZ9cKM6B+q55gfe4W6cg23l37YjI?=
 =?us-ascii?Q?6khSHiWgukG123z5unWLm4h4sqoa6HMT+UGqrdvhgKJUB5zntdK/fR9v6BDT?=
 =?us-ascii?Q?wjeMTYywEnL020Bvbd8tcDqEICxH6qYkO2fV6CBtcOgCWdUPxeXjZPU6rInY?=
 =?us-ascii?Q?qO65HxLHtvFC3SZV5HLHyp3+sX3Pr2tzEwE7C6SQG4pll7HIeUn1FZ7LFzRB?=
 =?us-ascii?Q?WgCL3sY5ohMGiQYXciPvH6sdtnvMa7Ou1k/kUpyajEPgBSnExyIr0L5EeIt4?=
 =?us-ascii?Q?+V+R5ecfUhUTK0UZw5Bd+8cmEEyTRMEicPZmVyQSwhLysKyjlMgespZf4Rsr?=
 =?us-ascii?Q?2FqBUNwXmYskF4UxM4mag7NX3hPGcq8yKdgL28eFbsr55va76G/z6b71BVYM?=
 =?us-ascii?Q?FEaVyRRc9xN3ur092DnCGCYZB51a3fOnTNb3/c4ROBj0bqIyOh0YCnKYPjp7?=
 =?us-ascii?Q?50YvStvEyPCoxMtoZv8CV5fysXpV06b1AxVD71gQE2Gku8zU/glpDl8gEqbX?=
 =?us-ascii?Q?CES/5QaVu6j395d2eI4mPtebNXcYk58D34a6LgAoF1Mdf+s/LCNVIJrPZLVc?=
 =?us-ascii?Q?tfUyqE2W9z6lKjlf+kJ0UJZ15wkHwpGkECFIbr+qFDu36lO+UToq1Su8izW6?=
 =?us-ascii?Q?jKMIu6JTvO0gyvr44XyK5cWKPlME3IPj/72Gq8RJkYki0ajpzUAB6EP3BT08?=
 =?us-ascii?Q?4tesdxgb8HJIhl+WWqOHwADd93JpMmTpEtellim16vRtmvb3LEWAneLOB0Fh?=
 =?us-ascii?Q?kD9dgzugMDIqCuKgDASpDHRrNcHbiz+3h4XbFFxJk8sJLQ1JiEWJGj30ajdC?=
 =?us-ascii?Q?HBMDKT3MbaXzqKqy0Kinl/pGINsHUCDU4fCFPvXF7QzyXBopiE0cS2nZZ50k?=
 =?us-ascii?Q?9m0CwRBIZ8eGZVAM3LhO798WMtOqlrIGju17hNnDU1QvgIlvZU2U39zHA2Cb?=
 =?us-ascii?Q?b0Js9knHDbdFSDzg9gZZHMDqhDqLhdJ3b0sB5s593oQJkJvKa0G93RJ4XzFr?=
 =?us-ascii?Q?ElwZxJHEUoF5gIFJ8XJbKt2UHvctMZWy9OcTC2gKP3lMA3hEalJJ/38MMl9t?=
 =?us-ascii?Q?ZIGVrivNvQIONpQso7Bad8ircZ68EOEMEbJx7NkqCKYB7DUNSTFgwjaT+zE3?=
 =?us-ascii?Q?HMVTo4KU0u3QNgHT/ufSnvaJ7aN/qC5sFsuq96Kj53gpsubM4i+mski/haKQ?=
 =?us-ascii?Q?L98abVDdX0fA2c6aTF4fme2rWwrNNuOMRZUDfp3xltq/+6U1O7N5wtpYGzkZ?=
 =?us-ascii?Q?qA=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ca325d12-f3b5-4864-e365-08dad96f42bb
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Dec 2022 22:55:11.2428
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ANbahW3JqfcEFAeYNqMmauYsc9z+uyhfpy+I229Ta6qa6QabVM9dE36ozSQyzG+sc1nHmokdj0Bkjei+pejT5g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR10MB7277
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-12-08_12,2022-12-08_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=944 bulkscore=0
 suspectscore=0 phishscore=0 malwarescore=0 spamscore=0 mlxscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2210170000 definitions=main-2212080189
X-Proofpoint-GUID: b2iAiw9KxLEmz-nphlaTBPcaVFhxHP6R
X-Proofpoint-ORIG-GUID: b2iAiw9KxLEmz-nphlaTBPcaVFhxHP6R
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/21/22 10:33, James Houghton wrote:
> On Wed, Nov 16, 2022 at 8:30 AM Peter Xu <peterx@redhat.com> wrote:
> >
> > 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@google.com>
> > > ---
> > >  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.
> 
> Failing the ioctl sounds better than only WARNing. I'll do that and
> drop the WARN_ON_ONCE for v1. Thanks!
> 

Sorry for the VERY late reply ...

After checking all the code paths, I do not think it is possible for a
!PageUptodate to be in the cache (target of continue).

ACK to failing the ioctl if not set, although I don't think it is possible
in current code.
-- 
Mike Kravetz