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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 854B3C7EE2E for ; Mon, 12 Jun 2023 09:18:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229591AbjFLJS3 (ORCPT ); Mon, 12 Jun 2023 05:18:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39730 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229942AbjFLJRo (ORCPT ); Mon, 12 Jun 2023 05:17:44 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 0CC81420E for ; Mon, 12 Jun 2023 02:10:57 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E60EE1FB; Mon, 12 Jun 2023 02:11:41 -0700 (PDT) Received: from [192.168.68.121] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EDF8E3F663; Mon, 12 Jun 2023 02:10:51 -0700 (PDT) Message-ID: <65a42ee0-170c-ec7c-519b-e66cdd901a52@arm.com> Date: Mon, 12 Jun 2023 10:10:50 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.11.2 Subject: Re: [PATCH v2 28/32] mm/memory: allow pte_offset_map[_lock]() to fail To: Hugh Dickins , Andrew Morton Cc: Mike Kravetz , Mike Rapoport , "Kirill A. Shutemov" , Matthew Wilcox , David Hildenbrand , Suren Baghdasaryan , Qi Zheng , Yang Shi , Mel Gorman , Peter Xu , Peter Zijlstra , Will Deacon , Yu Zhao , Alistair Popple , Ralph Campbell , Ira Weiny , Steven Price , SeongJae Park , Lorenzo Stoakes , Huang Ying , Naoya Horiguchi , Christophe Leroy , Zack Rusin , Jason Gunthorpe , Axel Rasmussen , Anshuman Khandual , Pasha Tatashin , Miaohe Lin , Minchan Kim , Christoph Hellwig , Song Liu , Thomas Hellstrom , linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20230609130632.ec6ffe72fc5f7952af4a3e54@linux-foundation.org> <11a9744a-e7f-33d0-474-c2f2eb7e079@google.com> From: Ryan Roberts In-Reply-To: <11a9744a-e7f-33d0-474-c2f2eb7e079@google.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/06/2023 21:11, Hugh Dickins wrote: > On Fri, 9 Jun 2023, Andrew Morton wrote: >> On Thu, 8 Jun 2023 18:43:38 -0700 (PDT) Hugh Dickins wrote: >> >>> copy_pte_range(): use pte_offset_map_nolock(), and allow for it to fail; >>> but with a comment on some further assumptions that are being made there. >>> >>> zap_pte_range() and zap_pmd_range(): adjust their interaction so that >>> a pte_offset_map_lock() failure in zap_pte_range() leads to a retry in >>> zap_pmd_range(); remove call to pmd_none_or_trans_huge_or_clear_bad(). >>> >>> Allow pte_offset_map_lock() to fail in many functions. Update comment >>> on calling pte_alloc() in do_anonymous_page(). Remove redundant calls >>> to pmd_trans_unstable(), pmd_devmap_trans_unstable(), pmd_none() and >>> pmd_bad(); but leave pmd_none_or_clear_bad() calls in free_pmd_range() >>> and copy_pmd_range(), those do simplify the next level down. >>> >>> ... >>> >>> @@ -3728,11 +3737,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) >>> vmf->page = pfn_swap_entry_to_page(entry); >>> vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, >>> vmf->address, &vmf->ptl); >>> - if (unlikely(!pte_same(*vmf->pte, vmf->orig_pte))) { >>> - spin_unlock(vmf->ptl); >>> - goto out; >>> - } >>> - >>> + if (unlikely(!vmf->pte || >>> + !pte_same(*vmf->pte, vmf->orig_pte))) >>> + goto unlock; >>> /* >>> * Get a page reference while we know the page can't be >>> * freed. >> >> This hunk falls afoul of >> https://lkml.kernel.org/r/20230602092949.545577-5-ryan.roberts@arm.com. >> >> I did this: >> >> @@ -3729,7 +3738,8 @@ vm_fault_t do_swap_page(struct vm_fault >> vmf->page = pfn_swap_entry_to_page(entry); >> vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, >> vmf->address, &vmf->ptl); >> - if (unlikely(!pte_same(*vmf->pte, vmf->orig_pte))) >> + if (unlikely(!vmf->pte || >> + !pte_same(*vmf->pte, vmf->orig_pte))) >> goto unlock; >> >> /* > > Yes, that's exactly right: thanks, Andrew. FWIW, I agree. Thanks, Ryan > > Hugh