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 4E611C38A02 for ; Fri, 28 Oct 2022 15:53:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CD29F8E0001; Fri, 28 Oct 2022 11:53:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C83726B0078; Fri, 28 Oct 2022 11:53:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B71B68E0001; Fri, 28 Oct 2022 11:53:15 -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 A568D6B0074 for ; Fri, 28 Oct 2022 11:53:15 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 834061610FF for ; Fri, 28 Oct 2022 15:53:15 +0000 (UTC) X-FDA: 80070802350.29.B93F590 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf24.hostedemail.com (Postfix) with ESMTP id D7A6C18003D for ; Fri, 28 Oct 2022 15:53:14 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id EB436B82870; Fri, 28 Oct 2022 15:53:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A5419C433D7; Fri, 28 Oct 2022 15:53:09 +0000 (UTC) Date: Fri, 28 Oct 2022 16:53:06 +0100 From: Catalin Marinas To: Muchun Song Cc: Anshuman Khandual , Wupeng Ma , Andrew Morton , Mike Kravetz , Muchun Song , Michal Hocko , Oscar Salvador , Linux Memory Management List , linux-kernel@vger.kernel.org Subject: Re: [PATCH -next 1/1] mm: hugetlb_vmemmap: Fix WARN_ON in vmemmap_remap_pte Message-ID: References: <20221025014215.3466904-1-mawupeng1@huawei.com> <614E3E83-1EAB-4C39-AF9C-83C0CCF26218@linux.dev> <35dd51eb-c266-f221-298a-21309c17971a@arm.com> <3D6FDA43-A812-4907-B9C8-C2B25567DBBC@linux.dev> <3c545133-71aa-9a8d-8a13-09186c4fa767@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1666972395; a=rsa-sha256; cv=none; b=wWmH6RZHe+FLOyICUANSnxSWOuglYSM/MB5Qp4hHDMVcp3ICw6dAaZfia62lUzvoHYyeY+ vdffQR8ergivjnqcSJ4ydETiNQvzbrugEH2qzuX2Sw2hAyEtfjg2EVQLfsSJe3oqdbLuJ/ QQNeFUQVJ7G4SGupk4cBUeAjwmMWeEQ= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=none; spf=pass (imf24.hostedemail.com: domain of cmarinas@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=cmarinas@kernel.org; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1666972395; 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: in-reply-to:in-reply-to:references:references; bh=U357t9LaYG+Ovf/UDWoeUpPyE18AOuyXI83iLq3jAp8=; b=7y4Nwb6WceYLVyepgBoPq2aJGp2d7xUibFfT2sO5E4cf7gk/xeQfb3lWxYhA281qBKPzE0 tcQ5unmJf5hcocUL2SJ8zdzvWKaDodchQ/qSuFfuFVAJxkPFx7Vhr3AKWwFLDCWc/l3vVE adxa0CBaOtBuH8i1hD3oRaVT2RZ1mQs= Authentication-Results: imf24.hostedemail.com; dkim=none; spf=pass (imf24.hostedemail.com: domain of cmarinas@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=cmarinas@kernel.org; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none) X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: D7A6C18003D X-Stat-Signature: h96ttba3xp3gwwr76oradckbp39z9xds X-Rspam-User: X-HE-Tag: 1666972394-403335 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 Fri, Oct 28, 2022 at 10:45:09AM +0800, Muchun Song wrote: > On Oct 27, 2022, at 18:50, Catalin Marinas wrote: > > On Wed, Oct 26, 2022 at 02:06:00PM +0530, Anshuman Khandual wrote: > >> On 10/26/22 12:31, Muchun Song wrote: > >>>> On 10/25/22 12:06, Muchun Song wrote: > >>>>>> On Oct 25, 2022, at 09:42, Wupeng Ma wrote: > >>>>>> From: Ma Wupeng > >>>>>> > >>>>>> Commit f41f2ed43ca5 ("mm: hugetlb: free the vmemmap pages associated with > >>>>>> each HugeTLB page") add vmemmap_remap_pte to remap the tail pages as > >>>>>> read-only to catch illegal write operation to the tail page. > >>>>>> > >>>>>> However this will lead to WARN_ON in arm64 in __check_racy_pte_update() > >>>>> > >>>>> Thanks for your finding this issue. > >>>>> > >>>>>> since this may lead to dirty state cleaned. This check is introduced by > >>>>>> commit 2f4b829c625e ("arm64: Add support for hardware updates of the > >>>>>> access and dirty pte bits") and the initial check is as follow: > >>>>>> > >>>>>> BUG_ON(pte_write(*ptep) && !pte_dirty(pte)); > >>>>>> > >>>>>> Since we do need to mark this pte as read-only to catch illegal write > >>>>>> operation to the tail pages, use set_pte to replace set_pte_at to bypass > >>>>>> this check. > >>>>> > >>>>> In theory, the waring does not affect anything since the tail vmemmap > >>>>> pages are supposed to be read-only. So, skipping this check for vmemmap > >>>> > >>>> Tails vmemmap pages are supposed to be read-only, in practice but their > >>>> backing pages do have pte_write() enabled. Otherwise the VM_WARN_ONCE() > >>>> warning would not have triggered. > >>> > >>> Right. > >>> > >>>> > >>>> VM_WARN_ONCE(pte_write(old_pte) && !pte_dirty(pte), > >>>> "%s: racy dirty state clearing: 0x%016llx -> 0x%016llx", > >>>> __func__, pte_val(old_pte), pte_val(pte)); > >>>> > >>>> Also, is not it true that the pte being remapped into a different page > >>>> as read only, than what it had originally (which will be freed up) i.e > >>>> the PFN in 'old_pte' and 'pte' will be different. Hence is there still > >>> > >>> Right. > >>> > >>>> a possibility for a race condition even when the PFN changes ? > >>> > >>> Sorry, I didn't get this question. Did you mean the PTE is changed from > >>> new (pte) to the old one (old_pte) by the hardware because of the update > >>> of dirty bit when a concurrent write operation to the tail vmemmap page? > >> > >> No, but is not vmemmap_remap_pte() reuses walk->reuse_page for all remaining > >> tails pages ? Is not there a PFN change, along with access permission change > >> involved in this remapping process ? > > > > For the record, as we discussed offline, changing the output address > > (pfn) of a pte is not safe without break-before-make if at least one of > > the mappings was writeable. The caller (vmemmap_remap_pte()) would need > > to be fixed to first invalidate the pte and then write the new pte. I > > Could you expose more details about what issue it will be caused? I am > not familiar with arm64. Well, it's not allowed by the architecture, so some CPU implementations may do weird things like accessing incorrect memory or triggering TLB conflict aborts if, for some reason, they end up with two entries in the TLB for the same VA but pointing to different pfns. The hardware expects an invalid PTE and TLB invalidation between such changes. In practice most likely nothing happens and this works fine but we need to stick to the architecture requirements in case some CPUs take advantage of this requirement. > > assume no other CPU accesses this part of the vmemmap while the pte is > > being remapped. > > However, there is no guarantee that no other CPU accesses this pte. > E.g. memory failure or memory compaction, both can obtain head page > from any tail struct pages (only read) anytime. Oh, so we cannot safely go through a break-before-make sequence here (zero the PTE, flush the TLB, write the new PTE) as some CPU may access this pte. -- Catalin