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 B70F2C369B2 for ; Thu, 17 Apr 2025 12:02:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E22736B009E; Thu, 17 Apr 2025 08:02:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DF7856B009F; Thu, 17 Apr 2025 08:02:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CBEE16B00A0; Thu, 17 Apr 2025 08:02:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id AAC7A6B009E for ; Thu, 17 Apr 2025 08:02:29 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 6A5F45D7F3 for ; Thu, 17 Apr 2025 12:02:30 +0000 (UTC) X-FDA: 83343398460.28.49DC4AF Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) by imf08.hostedemail.com (Postfix) with ESMTP id 5E8DD160018 for ; Thu, 17 Apr 2025 12:02:28 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=Xpe1E9ar; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf08.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=1744891348; 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=NRpqZn3xmGYP9OsNgVoADYvzcsi/0ghTFp6KfLY5hhA=; b=ztGvtkDwnU8HDTYy/mfSyMNxWJrIejlR2z0aIy3fW/xrwa4CJ4d48YhrHJiYpcEpYIH+z5 8gL/26DhgpHUpr2ayaiYVb9KOY6JiTt8n8PLDuUwXYoSSGwM/l5ZcRE3kLcSvnJl7W1tgS lS4vyYUp+gFuXFasAh1n1gv3LlC/SRs= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=Xpe1E9ar; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf08.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=1744891348; a=rsa-sha256; cv=none; b=nhIycI9Oh0grWiVVWdM8nd9HMsS7AvDQ4pzyhQPtziG8KzfmiPanuABqiigtwcTO7nJtsn stlO53zpC8ulI84/0ddtdSneL+Z1teormiyY4NGGtrUvwO5DjTe1RVRFykL6czal+Tb8g/ VGhlt9wOiLuWPeI85pmF22N3PwYUgAc= 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=NRpqZn3xmGYP9OsNgVoADYvzcsi/0ghTFp6KfLY5hhA=; b=Xpe1E9arCgNAsu++BcdEUDFEmR O/ZrpRhw8xi0s7ovvVoom57PQ3yaqQt1yyiAAeG4NdNO3dWHgLpxPJ2EfWpxV7oN2GXHibfEA/hHR X2wZHsd9dTps9WTBHScwscfFEpgi7dypoL8THYu1Oa2kNifXkVgviX02xKHGakl2bK/3Osbfuvksd a0r0ZgG7NENtNA5Kpqi/2u6r7kdlx7VnugplGSDxQtLrmS1cD2jZ42zgDBFeq3pOzP2MM3fn40CLl my+WGMiUo04bnqMZ1IFtskElp/geHY90YdlStw0JD7scj7tBx9BGTsVuMsVr/cfBxsdMwJGkW4Fon 8vJIEgsA==; 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 1u5Nwj-000nG0-2Z; Thu, 17 Apr 2025 14:02:21 +0200 Message-ID: <667354f3-7076-4e64-9506-56e81e7d9234@igalia.com> Date: Thu, 17 Apr 2025 20:02:14 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm/huge_memory: fix dereferencing invalid pmd migration entry To: Zi Yan Cc: David Hildenbrand , Hugh Dickins , linux-mm@kvack.org, akpm@linux-foundation.org, willy@infradead.org, 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> <412E70A4-4775-4AF7-A878-7FEBF9A122D8@nvidia.com> Content-Language: en-US From: Gavin Guo In-Reply-To: <412E70A4-4775-4AF7-A878-7FEBF9A122D8@nvidia.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Stat-Signature: gpokdp1wc9budehpb9fjxe8nxkrmqf9i X-Rspamd-Queue-Id: 5E8DD160018 X-Rspam-User: X-Rspamd-Server: rspam05 X-HE-Tag: 1744891348-625908 X-HE-Meta: U2FsdGVkX18z0vEsKLEZwMtr5cDRErRwTaVNKGV8YN6HT9ko3Ckr9AGDFmz4DqPO9PPyiHQhrkaj1hBUxrQxX7dCChcZXEI1vyObGXwp6hSha7YsM+Cz9l33UybE+fzGIJZyaetbtLGS8VtTWN35hKL2kUFInUbppDOtwCfMqnrE04NUdx+nY3AZERk2zW9+4N1yOJJB0g0u0f7g1GnkWUHgFisJdoETpo4iMBeznWPNHe+oROSXMSP8DRwpUmHS4rU0tGkI5E3zTOKORZPVFsK8UdeR++tQmVaNqhkcDazRpsZ0FP/yU89jcvB+o4F0sNOXDU9a8ex1kGMS6QrmEow5K/qAA88wVc5YhfdvTh3DqihNv1LEFp+QevXBb3i3fewdtgWN55FLxBtapTRVJ30ApgASucShQthrbPDtFdNSNQK0C8hkPMJ9+vUYBSkk89NVAXsYy8NwMeNo9ZrP5hPiGO5hHYCakCEzARdUZyMu9Hh1koQajoR20Whbeao6iskc1zzCpvTkW7mINchcYqVX8Qp1UyuEbnvU8Roe+WBTrlFxs6QDFK01NFhzFZw3JlVA7vTlFqGdH0M4lKa9/bY5C7j+/fsKMCZTYZ2xhJl78eoeunfyzz06MLCu4nvlY1gSbELylZmzzkyuXYBM2b22A5pT2DIi+PsD204dufS4OMRE3OemPB6HzZKUePX2Tf9BCrh0wPIh1Kmqw3mVOKbmdPRG05a7bOd/4XpWWNcf6it/PUGqE52ZQ1BfNzGPKA/ASw93sFYPpEgDt6K/8+YOhI/DTTVK7GoY0djFAYdrbyH7BIoJnjvJRg6PTvzES9YZxQLWwd358JrkGjTqrUIW3rojvs0cWODdjw9Xd+fVoNvHOb1rkOiwftPGVbNAH/5QU5+7PVR/BO6pa3bmiMwFtfUYAWhH5Bvcvmyc4OWYlpyjO29LjA0txC9EjokY2TxM/ChVeSjok/7/NXK mt6xq3Jt RT6BEz7CtnhMieANyizS2dOgi0x5HFt/FIH0uZ1VnFFw3Iu6pqh1gixF7cJhKieVXEShP0UjgWU9A1VazTqHVvMNcDbzWjfue8mSBrPBBF6gXSqqx7cZsT5NUFd0teKP/Tu16UALHuDQPPZu8/mJAbSXlN5gOx9CKAh3/RUgD0VBxP0rxr/0ikHZow7Mt/xxJEvD39YJ9YiizHJkdZ3l1sY656q44c+0sGniw+Xe1GPYXG6iKA6aM+FI8l6zumjEEjHHb6vDDItinsWKepU70xTPRWSCIc0US5nljhk0m79fwcVG/W4DGNb2Wvoh/KYeM6zBRAmaDLS9OZhTmNdKhCLZzMR/lParjqYjn3BvkQXzv3xIUAGWyXaGO4PgDl1R2pHv6 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 19:32, Zi Yan wrote: > On 17 Apr 2025, at 7:21, Gavin Guo wrote: > >> 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. > > 3. Can you please add Huge’s explanation below in the commit log? > That clarifies the issue. Thank you for the fix. Sure, will send out another patch for your review. Thank you for the review. > > “ > an anon_vma lookup points to a > location which may contain the folio of interest, but might instead > contain another folio: and weeding out those other folios is precisely > what the "folio != pmd_folio((*pmd)" check (and the "risk of replacing > the wrong folio" comment a few lines above it) is for. > ” > > With that, Acked-by: Zi Yan > >> >> 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 .. :) >>> > > > Best Regards, > Yan, Zi