From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mta0.migadu.com (out-6.mta0.migadu.com [91.218.175.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B70D84908BE for ; Thu, 13 Aug 2026 15:32:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.6 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786635130; cv=none; b=dmCtf9TWgyOfwLSf/fbdL+uiu0O2FU5NinIYwDstfZy0XOBRhIE3BcdDjtv8HV3Iu4uZPImFwyD0KDfUBlHRhWP7PEVLbidtP8kVYHGsSpcPgE+egnhRUQ5v8DB10XhUTtFgYP90VJlWvsRknGLgd3kEBK/3F+GIO33lfEYW0SQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786635130; c=relaxed/simple; bh=21CxL64E5wvwVmKuUEtjeFpitv04JpKOndGL+tFRCGk=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:Subject:From:To: References:In-Reply-To; b=WuQIgNyDMBK+1ncmq4tgFbH2uBRv8ub9HQ+PaGZHmWkL9VOBjwSBSpKWzAewC35/ckSIAleVXWYlNjE6eRnOXFM3rKk1PYvkqbmtFGWkJpF4nWYg+cL0r0Nh2UfMlHE8n4fK1xkjJv5ys4pNlqGh62aaUqOCoxkF5xp2vAPOc3U= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=I3wRhpkU; arc=none smtp.client-ip=91.218.175.6 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="I3wRhpkU" X-Envelope-To: linux-kernel@vger.kernel.org DKIM-Signature: a=rsa-sha256; bh=21CxL64E5wvwVmKuUEtjeFpitv04JpKOndGL+tFRCGk=; c=simple/simple; d=linux.dev; h=from:to:subject:date:message-id:mime-version:content-type; s=key1; t=1786635126; v=1; x=1787239926; b=I3wRhpkUPfBjO+pyhU2vatPE1Rr6QhTmjnk//ALSs1mDLEGUP1VH8MpC2+XRFFbLVOuY4db4 GvMhPcoFbJUpewvl1+7JXJG0O62lQWcoplb8Iz59qrLJJWqNNhlNa/m8ppeQ0xJVA0v/wmcHqEd N7ipwOCQ7hs+4MNkAIfq8/3U= X-Envelope-To: linux-kernel@vger.kernel.org Received: from localhost (77.97.51.77) by smtp.migadu.com with ESMTPS id 9cd5fe5c07c2ba09; Thu, 13 Aug 2026 15:32:06 +0000 X-Migadu-Flow: FLOW_OUT Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 13 Aug 2026 16:32:04 +0100 Message-Id: Cc: "Brendan Jackman" , "Borislav Petkov" , "Dave Hansen" , "Peter Zijlstra" , "Andrew Morton" , "David Hildenbrand" , "Vlastimil Babka" , "Mike Rapoport" , "Wei Xu" , "Johannes Weiner" , "Zi Yan" , "Lorenzo Stoakes" , , , , "Sumit Garg" , "Will Deacon" , , , "Itazuri, Takahiro" , "Andy Lutomirski" , "David Kaplan" , "Thomas Gleixner" , "Patrick Bellasi" , "Reiji Watanabe" , "Sean Christopherson" , "Nikita Kalyazin" , "Ackerley Tng" Subject: Re: [PATCH v3 03/26] mm: introduce AS_NO_DIRECT_MAP From: "Brendan Jackman" To: "Yosry Ahmed" , "Brendan Jackman" X-Mailer: aerc 0.21.0 References: <20260726-page_alloc-unmapped-v3-0-6f5729aa9832@google.com> <20260726-page_alloc-unmapped-v3-3-6f5729aa9832@google.com> In-Reply-To: Right, attempting this again now I've fixed my mail setup. I've no idea how that other "butt-dial" mail got sent, I was not able to sent mail _deliberately_ during that period but I guess somewhere in my setup I still had a copy of the password somewhere! On Fri Jul 31, 2026 at 8:28 PM BST, Yosry Ahmed wrote: >> > [..] >> >> return -EFAULT; >> >> =20 >> >> if (write) { >> >> @@ -2731,7 +2730,7 @@ EXPORT_SYMBOL(get_user_pages_unlocked); >> >> * This call assumes the caller has pinned the folio, that the lowes= t page table >> >> * level still points to this folio, and that interrupts have been d= isabled. >> >> * >> >> - * GUP-fast must reject all secretmem folios. >> >> + * GUP-fast must reject all folios without direct map entries (such = as secretmem). >> >> * >> >> * Writing to pinned file-backed dirty tracked folios is inherently = problematic >> >> * (see comment describing the writable_file_mapping_allowed() funct= ion). We >> >> @@ -2769,7 +2768,7 @@ static bool gup_fast_folio_allowed(struct folio= *folio, unsigned int flags) >> >> if (WARN_ON_ONCE(folio_test_slab(folio))) >> >> return false; >> >> =20 >> >> - /* hugetlb neither requires dirty-tracking nor can be secretmem. */ >> >> + /* hugetlb neither requires dirty-tracking nor can be without direc= t map. */ > > Is this necessarily true? I know there were discussions/proposals about > using some of the hugetlb infrastructure for guest_memfd. I am not sure > if those folios would remain hugetlb folios though. > > Adding Ackerley here. I think Google might have a bunch of patches for this, afraid I know very little about it though. >> >> if (folio_test_hugetlb(folio)) >> >> return true; >> >> =20 >> >> @@ -2812,7 +2811,7 @@ static bool gup_fast_folio_allowed(struct folio= *folio, unsigned int flags) >> >> * At this point, we know the mapping is non-null and points to an >> >> * address_space object. >> >> */ >> >> - if (check_secretmem && secretmem_mapping(mapping)) >> >> + if (mapping_no_direct_map(mapping)) >> >> return false; >> >> /* The only remaining allowed file system is shmem. */ >> >> return !reject_file_backed || shmem_mapping(mapping); >> >> diff --git a/mm/mlock.c b/mm/mlock.c >> >> index efa6716e4dfbd..045b6779440b1 100644 >> >> --- a/mm/mlock.c >> >> +++ b/mm/mlock.c >> >> @@ -474,7 +474,7 @@ static int mlock_fixup(struct vma_iterator *vmi, = struct vm_area_struct *vma, >> >> int ret =3D 0; >> >> =20 >> >> if (vma_flags_same_pair(&old_vma_flags, new_vma_flags) || >> >> - vma_is_secretmem(vma) || !vma_supports_mlock(vma)) { >> >> + vma_has_no_direct_map(vma) || !vma_supports_mlock(vma)) { >> > >> > I don't think this one is correct. From commit 1507f51255c9 ("mm: >> > introduce memfd_secret system call to create "secret" memory areas"): >> > >> > Since the secretmem mappings are locked in memory they cannot exceed >> > RLIMIT_MEMLOCK. Since these mappings are already locked independent= ly >> > from mlock(), an attempt to mlock()/munlock() secretmem range would >> > fail and mlockall()/munlockall() will ignore secretmem mappings. >> > >> > Seems like secretmem pages are just mlock()'d by default, hence the >> > check here. Maybe this also works for guest_memfd, but I don't think >> > it's a generalization that any pages without a direct mapping should >> > receive the same treatment here. >>=20 >> Ack, yeah this sounds correct to me. >>=20 >> I guess you could argue something like "the reason secretmem is >> implicitly mlocked is that it can't be reclaimed, because there's no >> direct map". But that doesn't generalise IMO, you could imagine letting >> the user say "remove this memory from the direct map, but I trust my >> swap system, you can swap it" and then use the mermap to implement >> reclaim. > > Exactly, I don't think no direct mapping implicitly means unreclaimable. > I don't think you actually need a direct mapping to read/write from > disk to memory? Oh. I never thought about that! I suppose the DMA is gonna happen via some other address space, either it's via an IOMMU or it works directly on physical RAM. So the kernel's direct map is irrelevant... Is that universal though? There must be cases where the CPU's mappings still matter... Umm... needs more research! (Don't think this blocks anything in this series though, let me know if you disagree...) >>=20 >> >> /* >> >> * Don't set VMA_LOCKED_BIT or VMA_LOCKONFAULT_BIT and don't >> >> * count. For secretmem, don't allow the memory to be unlocked. >> >> diff --git a/mm/secretmem.c b/mm/secretmem.c >> >> index 4a4934769f8ba..c043c53687d95 100644 >> >> --- a/mm/secretmem.c >> >> +++ b/mm/secretmem.c >> >> @@ -52,49 +52,20 @@ static vm_fault_t secretmem_fault(struct vm_fault= *vmf) >> >> struct address_space *mapping =3D vmf->vma->vm_file->f_mapping; >> >> struct inode *inode =3D file_inode(vmf->vma->vm_file); >> >> pgoff_t offset =3D vmf->pgoff; >> >> - gfp_t gfp =3D vmf->gfp_mask; >> >> struct folio *folio; >> >> vm_fault_t ret; >> >> - int err; >> >> =20 >> >> if (((loff_t)vmf->pgoff << PAGE_SHIFT) >=3D i_size_read(inode)) >> >> return vmf_error(-EINVAL); >> >> =20 >> >> filemap_invalidate_lock_shared(mapping); >> >> =20 >> >> -retry: >> >> - folio =3D filemap_lock_folio(mapping, offset); >> >> + folio =3D filemap_grab_folio(mapping, offset); >> >> if (IS_ERR(folio)) { >> >> - folio =3D folio_alloc(gfp | __GFP_ZERO, 0); >> >> - if (!folio) { >> >> - ret =3D VM_FAULT_OOM; >> >> - goto out; >> >> - } >> >> - >> >> - err =3D folio_zap_direct_map(folio); >> >> - if (err) { >> >> - folio_put(folio); >> >> - ret =3D vmf_error(err); >> >> - goto out; >> >> - } >> >> - >> >> - __folio_mark_uptodate(folio); >> >> - err =3D filemap_add_folio(mapping, folio, offset, gfp); >> >> - if (unlikely(err)) { >> >> - /* >> >> - * If a split of large page was required, it >> >> - * already happened when we marked the page invalid >> >> - * which guarantees that this call won't fail >> >> - */ >> >> - folio_restore_direct_map(folio); >> >> - folio_put(folio); >> >> - if (err =3D=3D -EEXIST) >> >> - goto retry; >> >> - >> >> - ret =3D vmf_error(err); >> >> - goto out; >> >> - } >> >> + ret =3D vmf_error(PTR_ERR(folio)); >> >> + goto out; >> >> } >> >> + folio_mark_uptodate(folio); >> > >> > This chunk seems like pure refactoring that should be done separately? >>=20 >> This is the adoption of AS_NO_DIRECT_MAP, i.e. the removal of the >> explicit folio_zap_direct_map() call. We could certainly separate out >> "create AS_NO_DIRECT_MAP" from "adopt it in secretmem" but the previous >> version of this patchset was on v12 and it hadn't split them so I assume >> nobody was calling for this split. > > Oh sorry I wasn't clear. I meant switching from folio_lock_folio() and > the rest of the logic to folio_grab_folio(). There are some subtle > differences AFAICT so this conversion shouldn't really be part of this > patch. > > I think maybe just drop the > folio_zap_direct_map()/folio_restore_direct_map() calls for this patch? Oh I see what you mean, yeah just dropping the zap/restore makes sense, then switch APIs as a followup.