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 AD529C369B2 for ; Thu, 17 Apr 2025 11:22:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CC1F028001A; Thu, 17 Apr 2025 07:22:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BAE0D280027; Thu, 17 Apr 2025 07:22:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9AFDB28001A; Thu, 17 Apr 2025 07:22:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 65A7E280027 for ; Thu, 17 Apr 2025 07:22:08 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 80EB116160B for ; Thu, 17 Apr 2025 11:22:08 +0000 (UTC) X-FDA: 83343296736.17.1BCE7FE Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) by imf30.hostedemail.com (Postfix) with ESMTP id 7BC8B8000D for ; Thu, 17 Apr 2025 11:22:06 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=kmuzlFgl; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf30.hostedemail.com: domain of gavinguo@igalia.com designates 178.60.130.6 as permitted sender) smtp.mailfrom=gavinguo@igalia.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744888926; 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=4l3H5XoP1qoOtPaanLPOXIJj8oTfFM6gBeFk4wZOUOQ=; b=pEyon42qM0o+PPX/4p+X5FWbgVROEX6fkhsR42qBFJ+KP1WruocAdLezz54/e0zaLt02O6 pYaSzi0VLtPTVnu6QeYmSraUeg3zQwoZDpaGhf2WOcFf8Sdk5MuAg0NzwJO65BSgKKAUKs wKEPztRRrcmXsCVui3fi/2YZU65Yq90= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=kmuzlFgl; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf30.hostedemail.com: domain of gavinguo@igalia.com designates 178.60.130.6 as permitted sender) smtp.mailfrom=gavinguo@igalia.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744888927; a=rsa-sha256; cv=none; b=oZnILm52vCgDx/LG/eazcY3u++ZkKqH9YrxtAHINFRSk+WE/U6f40fppJdssPeZo3J5OIv ClCS1WpKL6jkj3APpTWE++OeBdX/nZvD4kSYQBlGRILaasCyZoelfflGJzfFnIj/Z5U9MB y5X1gMuLvcz7QFZz/73sryDeXUCL0FU= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=4l3H5XoP1qoOtPaanLPOXIJj8oTfFM6gBeFk4wZOUOQ=; b=kmuzlFglRmkvwwjFCnM+UN+/0i AMXAhQnC+MpXEUzEWy7310zMVAtdM17ezjvMq6vrbkafMxqoIWLa8TGSk55xx2RFoDGlT3fgVpmDL gR0ULX8zekNaFw48xR4Oy8VRf200hG48vd2/K5u7i6mqzMkagmNXifeVmZzB7iTUyDgxHhsjE2q/U yrQS1087fYAIDtMxCopRT+9CVysIAzON+joB5ljxDXl2mO4qHA+3gUt3k/NKtGpRhMmYjtt7GTLf0 +FjM2c61Lxwe+h2wgTEFL0Xk7uwnr0FsS1cTiXszfwV4+C+nVsDN4jGiXoHc/pfDA/AnUBnA5Erqw wlzSZTzA==; Received: from 39-14-49-133.adsl.fetnet.net ([39.14.49.133] helo=[192.168.220.43]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1u5NJb-000ma4-V0; Thu, 17 Apr 2025 13:21:56 +0200 Message-ID: Date: Thu, 17 Apr 2025 19:21:48 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm/huge_memory: fix dereferencing invalid pmd migration entry To: David Hildenbrand , Hugh Dickins Cc: linux-mm@kvack.org, akpm@linux-foundation.org, willy@infradead.org, ziy@nvidia.com, linmiaohe@huawei.com, revest@google.com, kernel-dev@igalia.com, linux-kernel@vger.kernel.org References: <20250414072737.1698513-1-gavinguo@igalia.com> <27d13454-280f-4966-b694-d7e58d991547@redhat.com> <6787d0ea-a1b9-08cf-1f48-e361058eec20@google.com> <83f17b85-c9fa-43a0-bec1-22c8565b67ad@redhat.com> <98d1d195-7821-4627-b518-83103ade56c0@redhat.com> <7d0ef7b5-043b-beca-72a9-6ae98b0d55fb@google.com> <05a7d51e-f065-445a-af0e-481f3461a76e@redhat.com> Content-Language: en-US From: Gavin Guo In-Reply-To: <05a7d51e-f065-445a-af0e-481f3461a76e@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Stat-Signature: u1p5yy5dffcf6swr6cq4ne567hzky44a X-Rspamd-Queue-Id: 7BC8B8000D X-Rspam-User: X-Rspamd-Server: rspam05 X-HE-Tag: 1744888926-763947 X-HE-Meta: U2FsdGVkX1+RxQ/Spuq9mX82+sD/j6CyWpr/bZEyMGKUwAvCUYvfy9RmXfFmlf6QZpSAUUN72c62GyBBx+wSEE64KnObfoVF167QsYfPUdqKSsyOgcawxs+6yBnXS8i4iMh2B9AA5tbsPjdzOK+IfmwhYedMdsOKufc3qRWELhBMrDuN1U5ACPECq3hu3vuZcKZXUMnAiozcSTHqicc1FFqL+iTXb4Dg6XFIP3mtnLfR9/1EGNOEKDfb2+JYwkCqkhiqt7lz23CouzXdfm3UU+pVqXTrSN0VTKyqIOL48xXnSNbjE4+zl6FG2ccXtc3wAP5eIjhyQq85O4dvZZiqmOuSOt17lR7o9Ww5UjEr7yI/qDpUrjdlHsQDMFIEUEPfljlBhjtUlyfgQKp1yFosef8HhgNAbQphF7MyrCDw3uKHckZVVkJe0LPGvrQUa8aYpXEMqWcI5SDDw9gJftSwPVWPXDRc9C2asOz9ZHAaoFkawao7MVleqLf4N4Xp8Dccn2GpyNBEf2ga4vpVT9A9H6w0l5I37AGBP6aLOl3oielC98BJW324oOJXi+qc7is9iLXlv1eWErSuDUTn9uFLK2nZAb7tddq7sLdXwl68eED7fpha//N7hxQl3YEuI7m3+eyF4TeEizfckmxJX/ZiXhiwORZY2DggRn3jJLged4iZvgSkJhpq6BPwCf8rOlS3Q11mjyevgnEel95Yx43WBMOlqsjzezKFZrvXNz7/FICkoty2WzlZoB4l8ByEm6tyYJn4mSGJFwaKQsYH/4DI6mVWhPmPVpAsMga05YY+SaA1QyDfD3gzorgvGRnbg+PZPr8r5lwR9Wu/YRrRIcWMLJsIsvOIm2SQSlX/jg1OPkoQl01vtToWXR9T+IcERMEEjcnZxmn1bpfUp6nthdw7ed4ZpRukahXvGJSkheSZHlOfKLbPWsZ5ZrEAVJCy8E58pOWCVxDQ94dMW6QMOY5 3E3ZoTV4 JdAX2NWRTIDpnJQDH+e7FqSH+aOf31r0rRgUnBaEY0hhrQJzMy7tXgxT5ywdhVhHzDNoj9vpsAG2REvYJ85zbGccFWhBBODoFCOVrE9mnvnoHFCkCe5pbVaVdDcxjNJkd6h5aaxJg3+oCVLy5OkyDAW8NDbRGiUHANcZ6ZRBg7/Q1rOkIloogaEvY8vzCO3wHouZL/W1Z3Z4D9+Zu/MKZgr0iEv4jS/p1roLl+lSZML88hjjX5EfFVcMpAwg6MHJKF524edupQZCZg1bJ+d3grR6DdT/XcBjpubpUHNbMrXhNr7y9fmWDeBBVALdnnZmVvc2HtOb9lwBXrONniaO6aUsc7sfjgzfPeGXNgtsQxcz4PnY= 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: List-Subscribe: List-Unsubscribe: On 4/17/25 17:04, David Hildenbrand wrote: > On 17.04.25 10:55, Hugh Dickins wrote: >> On Thu, 17 Apr 2025, David Hildenbrand wrote: >>> On 17.04.25 09:18, David Hildenbrand wrote: >>>> On 17.04.25 07:36, Hugh Dickins wrote: >>>>> On Wed, 16 Apr 2025, David Hildenbrand wrote: >>>>>> >>>>>> Why not something like >>>>>> >>>>>> struct folio *entry_folio; >>>>>> >>>>>> if (folio) { >>>>>>   if (is_pmd_migration_entry(*pmd)) >>>>>>       entry_folio = pfn_swap_entry_folio(pmd_to_swp_entry(*pmd))); >>>>>>   else >>>>>>    entry_folio = pmd_folio(*pmd)); >>>>>> >>>>>>   if (folio != entry_folio) >>>>>>         return; >>>>>> } >>>>> >>>>> My own preference is to not add unnecessary code: >>>>> if folio and pmd_migration entry, we're not interested in entry_folio. >>>>> But yes it could be written in lots of other ways. >>>> >>>> While I don't disagree about "not adding unnecessary code" in general, >>>> in this particular case just looking the folio up properly might be the >>>> better alternative to reasoning about locking rules with conditional >>>> input parameters :) >>>> >>> >>> FWIW, I was wondering if we can rework that code, letting the caller >>> to the >>> checking and getting rid of the folio parameter. Something like this >>> (incomplete, just to >>> discuss if we could move the TTU_SPLIT_HUGE_PMD handling). >> >> Yes, I too dislike the folio parameter used for a single case, and agree >> it's better for the caller who chose pmd to check that *pmd fits the >> folio. >> >> I haven't checked your code below, but it looks like a much better way >> to proceed, using the page_vma_mapped_walk() to get pmd lock and check; >> and cutting out two or more layers of split_huge_pmd obscurity. >> >> Way to go.  However... what we want right now is a fix that can easily >> go to stable: the rearrangements here in 6.15-rc mean, I think, that >> whatever goes into the current tree will have to be placed differently >> for stable, no seamless backports; but Gavin's patch (reworked if you >> insist) can be adapted to stable (differently for different releases) >> more more easily than the future direction you're proposing here. > > I'm fine with going with the current patch and looking into cleaning it > up properly (if possible). > > So for this patch > > Acked-by: David Hildenbrand > > @Gavin, can you look into cleaning that up? Thank you for your review. Before I begin the cleanup, could you please confirm the following action items: Zi Yan's suggestions for the patch are: 1. Replace the page fault with an invalid address access in the commit description. 2. Simplify the nested if-statements into a single if-statement to reduce indentation. David, based on your comment, I understand that you are recommending the entry_folio implementation. Also, from your discussion with Hugh, it appears you agreed with my original approach of returning early when encountering a PMD migration entry, thereby avoiding unnecessary checks. Is that correct? If so, I will keep the current logic. Do you have any additional cleanup suggestions? I will start the cleanup work after confirmation. > >> >> (Hmm, that may be another reason for preferring the reasoning by >> folio lock: forgive me if I'm misremembering, but didn't those >> page migration swapops get renamed, some time around 5.11?) > > I remember that we did something to PTE handling stuff in the context of > PTE markers. But things keep changing all of the time .. :) >