From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6B75F38AC86 for ; Tue, 7 Apr 2026 10:48:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775558917; cv=none; b=JYDDM91ODD5o+O7Y9mpCoqYueUM7hl5Njs+eHVY6gbiHZ7u8ZzABPnEngGuPwcSYBYu+BZaI8oC4jUWRplwbhZPM8JizmDJ9kXb8ss4nY2bY8cZysemzZ79PjAeeH/3TI6OZwX12KJ024xKkGSpxZWuc5WbOHoxEkSblXWEpahA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775558917; c=relaxed/simple; bh=plvm98yXi0oiNeiJ5KWfoDRd1nOJdHlTIM+h4PPVQwA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=oO1jmbMf/fWF6Cp4Td4RuV57i3DrkHI/n51f6OZtLMX7lwuHqHW9J+q0cUM2UnelCECdDDwtyB+sGjoSiLXqvAEdWXpVTIkbUoi6J8+jfWucospHPrgy8eJGrkAcRqq8qZseg5SvfCOFEfj6InhgllDgj8lkpN5qNB+Qld9wLQo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=B9Hrl0YP; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="B9Hrl0YP" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4639AC116C6; Tue, 7 Apr 2026 10:48:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1775558917; bh=plvm98yXi0oiNeiJ5KWfoDRd1nOJdHlTIM+h4PPVQwA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=B9Hrl0YPWM3VFx6SsgTZ8TtbBCyKSnUdHdQRtEc3XObxhiKqdsjaPL+d40XYklTNX bggf8Jfdc/YYxn8zYxZyb61PdlVZJs1Rhf42xwVxhqL6ALT1DUYBMcJqRYFOY//5ls SC/YvjS0xi0jwkTpCnWJ04h8xov1CFLLclQOJfJKXKNVKQgYJh3yfZ1GxEoZdhMTgL mCoONRv75hKrOLtDgGBGVqAh1UuXR59rfwMSRx8077YOHSixyNdFrKSf5DFr3DlQkq cONDaKyJ4b7pn2ZLckA82VmZYTpJhAtDt/CSwByCfjc+sOynjtguVIL6ve0LEcvou5 g0JopWY60gt9A== Date: Tue, 7 Apr 2026 11:48:30 +0100 From: Lorenzo Stoakes To: Yin Tirui Cc: Andrew Morton , David Hildenbrand , Zi Yan , Baolin Wang , "Liam R . Howlett" , Nico Pache , Ryan Roberts , Dev Jain , Barry Song , Lance Yang , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , Kiryl Shutsemau , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 13/13] mm/huge_memory: add and use has_deposited_pgtable() Message-ID: References: <41b1ff54-c120-42ae-8b74-54767abf3554@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Thu, Apr 02, 2026 at 03:49:35PM +0800, Yin Tirui wrote: > > > On 4/2/26 14:46, Lorenzo Stoakes (Oracle) wrote: > > > > I mean you would have needed to handle this case in any event, since this change > > is strictly an equivalent reworking of zap_huge_pmd(). > > > > But it seems that doing so has clarified the requirements somewhat here :) > > > > I haven't had a look at that series yet (please cc this email if you weren't > > already, I do filter a lot of stuff due to how much mail I get daily) > > Hi Lorenzo, > > Thanks for the quick reply. I will definitely CC you on the v4 series. Thanks. > > > > > So if this is a PMD leaf entry it will be present and PFN map, so I'd have > > thought simply adding: > > > > /* Huge PFN map must deposit, as cannot refault. */ > > if (vma_test(vma, VMA_PFNMAP_BIT)) > > return true; > > > > Would suffice? > > Here is the dilemma: > > Currently, VFIO uses vmf_insert_pfn_pmd() to create huge pfnmaps on page > faults. This sets VM_PFNMAP in vfio_pci_core_mmap(), but it does not > deposit a pgtable (unless arch_needs_pgtable_deposit() is true). Hmmm... it's only the VFIO and hyperv drivers using this. Wouldn't we generally want a deposited huge page here now we're allowing huge PFN maps? Or are this _special cases_ where we have a PMD-sized entry but are not necessarily wanting to treat it as THP? This is a real wrinkle in this whole series no? David - any thoughts? > > To resolve this, > > Option A: Force VFIO (vmf_insert_pfn_pmd) to also deposit pgtables. This > unifies the VM_PFNMAP lifecycle. However, since VFIO can refault, > depositing pgtables here incurs unnecessary memory overhead. How can VFIO refault as a PFN mapping? Does it intentionally sometimes clear PTE entries to effect a refault, and implement a custom fault handler? I guess having a fault handler makes it refaultable... I mean obviously that then contradicts the suggested comment above :) That seems to me to cast a bit of a question over the whole series - having PMD mappings that are _sometimes_ THP and _sometimes_ not is weird (TM). And it'd suck to add - yet another very specific check - to determine if we do, in fact, assume THP for a PMD sized PFN map. > > Option B: Introduce a new VMA flag set during remap_pfn_range(), which > we can explicitly check in has_deposited_pgtable(). Yeah would rather not, that feels like a hack. > > Option C: Check vma->vm_ops->fault (and huge_fault). We would only > deposit pgtables for mappings without fault handlers. However, this is > fragile because a driver might still register a .fault() handler that > simply returns VM_FAULT_SIGBUS. I mean again this is yet another check (TM). But probably the most preferable I think. Wouldn't a driver doing that be being somewhat redundant? E.g. in do_fault(); if (!vma->vm_ops->fault) { vmf->pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl); if (unlikely(!vmf->pte)) ret = VM_FAULT_SIGBUS; And so can expect maybe some more redundancy if they also happen to map PMD-sized ranges? :) And the only two callers of vmf_insert_pfn_pmd() - hyperv and VFIO both implement actual fault handlers anyway. So I think this is fine? > > Do you have a preference among these, or perhaps another idea? > > > > > By the way, I am wondering if the prot bits are correctly preserved on page > > table deposit, as this is key for pfn map (e.g. if the range is uncached, for > > instance). That's something to check and ensure is correct. > > > > I _suspect_ they will be, as we have pretty well established mechanisms for that > > (propagate vma->vm_page_prot etc.) but definitely worth making sure. > > > > Yes, they are correctly preserved! > > During a PMD split in __split_huge_pmd_locked(), we populate the > deposited pgtable like this: > > entry = pfn_pte(pmd_pfn(old_pmd), pmd_pgprot(old_pmd)); > set_ptes(mm, haddr, pte, entry, HPAGE_PMD_NR); > > The newly refactored pmd_pgprot() correctly extracts the exact > protection bits (including crucial cache modes like UC/WC for device > memory) from the huge PMD, strips the hardware-specific huge bit, and > returns a pure PTE-level pgprot_t. OK good :) > > >> > >> [1] > >> https://lore.kernel.org/linux-mm/20260228070906.1418911-5-yintirui@huawei.com/ > > -- > Yin Tirui > Cheers, Lorenzo