| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <15f665b4-2d33-41ca-ac50-fafe24ade32f@redhat.com>
Date: Mon, 18 Nov 2024 10:38:29 +0100
From: David Hildenbrand <david@...hat.com>
To: Elliot Berman <quic_eberman@...cinc.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Sean Christopherson <seanjc@...gle.com>, Fuad Tabba <tabba@...gle.com>,
Ackerley Tng <ackerleytng@...gle.com>, Mike Rapoport <rppt@...nel.org>,
"H. Peter Anvin" <hpa@...or.com>, Matthew Wilcox <willy@...radead.org>,
James Gowans <jgowans@...zon.com>, linux-fsdevel@...r.kernel.org,
kvm@...r.kernel.org, linux-coco@...ts.linux.dev,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org
Subject: Re: [PATCH RFC v3 1/2] KVM: guest_memfd: Convert .free_folio() to
.release_folio()
On 15.11.24 21:13, Elliot Berman wrote:
> On Fri, Nov 15, 2024 at 11:58:59AM +0100, David Hildenbrand wrote:
>> On 15.11.24 11:58, David Hildenbrand wrote:
>>> On 13.11.24 23:34, Elliot Berman wrote:
>>>> When guest_memfd becomes a library, a callback will need to be made to
>>>> the owner (KVM SEV) to transition pages back to hypervisor-owned/shared
>>>> state. This is currently being done as part of .free_folio() address
>>>> space op, but this callback shouldn't assume that the mapping still
>>>> exists. guest_memfd library will need the mapping to still exist to look
>>>> up its operations table.
>>>
>>> I assume you mean, that the mapping is no longer set for the folio (it
>>> sure still exists, because we are getting a callback from it :) )?
>>>
>>> Staring at filemap_remove_folio(), this is exactly what happens:
>>>
>>> We remember folio->mapping, call __filemap_remove_folio(), and then call
>>> filemap_free_folio() where we zap folio->mapping via page_cache_delete().
>>>
>>> Maybe it's easier+cleaner to also forward the mapping to the
>>> free_folio() callback, just like we do with filemap_free_folio()? Would
>>> that help?
>>>
>>> CCing Willy if that would be reasonable extension of the free_folio
>>> callback.
>>>
>
> I like this approach too. It would avoid the checks we have to do in the
> invalidate_folio() callback and is cleaner.
It really should be fairly simple
Documentation/filesystems/locking.rst | 2 +-
fs/nfs/dir.c | 9 +++++----
fs/orangefs/inode.c | 3 ++-
include/linux/fs.h | 2 +-
mm/filemap.c | 2 +-
mm/secretmem.c | 3 ++-
virt/kvm/guest_memfd.c | 3 ++-
7 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
index f5e3676db954b..f1a20ad5edbee 100644
--- a/Documentation/filesystems/locking.rst
+++ b/Documentation/filesystems/locking.rst
@@ -258,7 +258,7 @@ prototypes::
sector_t (*bmap)(struct address_space *, sector_t);
void (*invalidate_folio) (struct folio *, size_t start, size_t len);
bool (*release_folio)(struct folio *, gfp_t);
- void (*free_folio)(struct folio *);
+ void (*free_folio)(struct address_space *, struct folio *);
int (*direct_IO)(struct kiocb *, struct iov_iter *iter);
int (*migrate_folio)(struct address_space *, struct folio *dst,
struct folio *src, enum migrate_mode);
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 492cffd9d3d84..f7da6d7496b06 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -218,7 +218,8 @@ static void nfs_readdir_folio_init_array(struct folio *folio, u64 last_cookie,
/*
* we are freeing strings created by nfs_add_to_readdir_array()
*/
-static void nfs_readdir_clear_array(struct folio *folio)
+static void nfs_readdir_clear_array(struct address_space *mapping,
+ struct folio *folio)
{
struct nfs_cache_array *array;
unsigned int i;
@@ -233,7 +234,7 @@ static void nfs_readdir_clear_array(struct folio *folio)
static void nfs_readdir_folio_reinit_array(struct folio *folio, u64 last_cookie,
u64 change_attr)
{
- nfs_readdir_clear_array(folio);
+ nfs_readdir_clear_array(folio->mapping, folio);
nfs_readdir_folio_init_array(folio, last_cookie, change_attr);
}
@@ -249,7 +250,7 @@ nfs_readdir_folio_array_alloc(u64 last_cookie, gfp_t gfp_flags)
static void nfs_readdir_folio_array_free(struct folio *folio)
{
if (folio) {
- nfs_readdir_clear_array(folio);
+ nfs_readdir_clear_array(folio->mapping, folio);
folio_put(folio);
}
}
@@ -391,7 +392,7 @@ static void nfs_readdir_folio_init_and_validate(struct folio *folio, u64 cookie,
if (folio_test_uptodate(folio)) {
if (nfs_readdir_folio_validate(folio, cookie, change_attr))
return;
- nfs_readdir_clear_array(folio);
+ nfs_readdir_clear_array(folio->mapping, folio);
}
nfs_readdir_folio_init_array(folio, cookie, change_attr);
folio_mark_uptodate(folio);
diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index aae6d2b8767df..d936694b8e91f 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -470,7 +470,8 @@ static bool orangefs_release_folio(struct folio *folio, gfp_t foo)
return !folio_test_private(folio);
}
-static void orangefs_free_folio(struct folio *folio)
+static void orangefs_free_folio(struct address_space *mapping,
+ struct folio *folio)
{
kfree(folio_detach_private(folio));
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3559446279c15..4dd4013541c1b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -417,7 +417,7 @@ struct address_space_operations {
sector_t (*bmap)(struct address_space *, sector_t);
void (*invalidate_folio) (struct folio *, size_t offset, size_t len);
bool (*release_folio)(struct folio *, gfp_t);
- void (*free_folio)(struct folio *folio);
+ void (*free_folio)(struct address_space *, struct folio *folio);
ssize_t (*direct_IO)(struct kiocb *, struct iov_iter *iter);
/*
* migrate the contents of a folio to the specified target. If
diff --git a/mm/filemap.c b/mm/filemap.c
index e582a1545d2ae..86f975ba80746 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -239,7 +239,7 @@ void filemap_free_folio(struct address_space *mapping, struct folio *folio)
free_folio = mapping->a_ops->free_folio;
if (free_folio)
- free_folio(folio);
+ free_folio(mapping, folio);
if (folio_test_large(folio))
refs = folio_nr_pages(folio);
diff --git a/mm/secretmem.c b/mm/secretmem.c
index 399552814fd0f..1d2ed3391734d 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -152,7 +152,8 @@ static int secretmem_migrate_folio(struct address_space *mapping,
return -EBUSY;
}
-static void secretmem_free_folio(struct folio *folio)
+static void secretmem_free_folio(struct address_space *mapping,
+ struct folio *folio)
{
set_direct_map_default_noflush(&folio->page);
folio_zero_segment(folio, 0, folio_size(folio));
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 8f079a61a56db..573946c4fff51 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -353,7 +353,8 @@ static int kvm_gmem_error_folio(struct address_space *mapping, struct folio *fol
}
#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
-static void kvm_gmem_free_folio(struct folio *folio)
+static void kvm_gmem_free_folio(struct address_space *mapping,
+ struct folio *folio)
{
struct page *page = folio_page(folio, 0);
kvm_pfn_t pfn = page_to_pfn(page);
--
2.47.0
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists