* [PATCH 0/5] KVM: guest_memfd: fix unbind race and NUMA selftests
@ 2026-08-23 13:36 Shivank Garg
2026-08-23 13:36 ` [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file Shivank Garg
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Shivank Garg @ 2026-08-23 13:36 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Shuah Khan, Jim Mattson,
Peter Shier, Ricardo Koller, David Hildenbrand, Ackerley Tng
Cc: kvm, linux-kernel, linux-kselftest, Shivank Garg, Sashiko
Split out the guest_memfd folio migration series [1], where migration part
needs more discussion. These fixes, reported by Sashiko bot [2][3], are
independent of it and can be reviewed on their own.
[1] https://lore.kernel.org/all/20260805-shivank-gmem-migrate-v3-0-00d8bdec4e1d@amd.com
[2] https://lore.kernel.org/all/20260728092027.225CF1F000E9@smtp.kernel.org
[3] https://lore.kernel.org/all/20260728091945.EA6C31F000E9@smtp.kernel.org
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
Shivank Garg (5):
KVM: guest_memfd: take the invalidate lock when unbinding a dying file
KVM: selftests: fix maxnode arguments in xapic_ipi_test
KVM: selftests: use BITS_PER_TYPE() for NUMA masks
KVM: selftests: add get_numa_mem_nodes()
KVM: selftests: use allowed NUMA nodes in guest_memfd_test
tools/testing/selftests/kvm/guest_memfd_test.c | 86 ++++++++++++++++--------
tools/testing/selftests/kvm/include/numaif.h | 52 ++------------
tools/testing/selftests/kvm/x86/xapic_ipi_test.c | 14 ++--
virt/kvm/guest_memfd.c | 23 ++++---
4 files changed, 84 insertions(+), 91 deletions(-)
---
base-commit: 2709dd5ae32f0828f386327c76bba9f39f63a1c6
change-id: 20260822-shivank-gmem-fix-split-b8a9057cf987
Best regards,
--
Shivank Garg <shivankg@amd.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file
2026-08-23 13:36 [PATCH 0/5] KVM: guest_memfd: fix unbind race and NUMA selftests Shivank Garg
@ 2026-08-23 13:36 ` Shivank Garg
2026-08-25 22:13 ` Sean Christopherson
2026-08-23 13:36 ` [PATCH 2/5] KVM: selftests: fix maxnode arguments in xapic_ipi_test Shivank Garg
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Shivank Garg @ 2026-08-23 13:36 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Shuah Khan, Jim Mattson,
Peter Shier, Ricardo Koller, David Hildenbrand, Ackerley Tng
Cc: kvm, linux-kernel, linux-kselftest, Shivank Garg, Sashiko
kvm_gmem_unbind() skips mapping->invalidate_lock when the guest_memfd
file is already dying. All other paths that modify f->bindings hold
that lock.
kvm_gmem_invalidate_{start,end}() checks f->bindings independently to
decide whether to begin or end KVM MMU invalidations. So, the bindings
must remain stable between the two calls. If a binding is removed in that
window, start increments mmu_invalidate_in_progress but end does not
decrement it. Example, unbind race with memory failure:
CPU 0: memory failure CPU 1: memslot delete
---------------------------------- ---------------------------
(guest_memfd file is dying)
kvm_gmem_error_folio()
kvm_gmem_invalidate_start()
finds binding
mmu_invalidate_in_progress++
kvm_gmem_unbind()
get_file_active() fails
store NULL in bindings
kvm_gmem_invalidate_end()
no binding found
counter stays elevated
mmu_invalidate_retry() then returns 1 forever, so guest page faults
retry without ever installing a mapping and the guest hangs.
Take the invalidate lock in the dying-file path too. This prevents unbind
from removing a binding and leaking mmu_invalidate_in_progress. This is
safe because any caller that reaches this path holds slots_lock, so
kvm_gmem_release() cannot nullify the slot->gmem.file, until
kvm_gmem_unbind() finishes.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260728092027.225CF1F000E9@smtp.kernel.org
Fixes: ae431059e75d ("KVM: guest_memfd: Remove bindings on memslot deletion when gmem is dying")
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
virt/kvm/guest_memfd.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index f0e5da490866..f848120af84b 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -721,6 +721,8 @@ static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
void kvm_gmem_unbind(struct kvm_memory_slot *slot)
{
+ struct file *gmem_file;
+
/*
* Nothing to do if the underlying file was _already_ closed, as
* kvm_gmem_release() invalidates and nullifies all bindings.
@@ -733,21 +735,24 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
/*
* However, if the file is _being_ closed, then the bindings need to be
* removed as kvm_gmem_release() might not run until after the memslot
- * is freed. Note, modifying the bindings is safe even though the file
- * is dying as kvm_gmem_release() nullifies slot->gmem.file under
+ * is freed. Note, dereferencing the dying file is safe as
+ * kvm_gmem_release() nullifies slot->gmem.file under
* slots_lock, and only puts its reference to KVM after destroying all
* bindings. I.e. reaching this point means kvm_gmem_release() hasn't
* yet destroyed the bindings or freed the gmem_file, and can't do so
* until the caller drops slots_lock.
*/
- if (!file) {
- __kvm_gmem_unbind(slot, slot->gmem.file->private_data);
- return;
- }
+ gmem_file = file ?: slot->gmem.file;
- filemap_invalidate_lock(file->f_mapping);
- __kvm_gmem_unbind(slot, file->private_data);
- filemap_invalidate_unlock(file->f_mapping);
+ /*
+ * Take the invalidate lock even for a dying file. Otherwise,
+ * kvm_gmem_invalidate_start() can find the binding and increment
+ * mmu_invalidate_in_progress while kvm_gmem_invalidate_end() misses
+ * the removed binding and skips decrement.
+ */
+ filemap_invalidate_lock(gmem_file->f_mapping);
+ __kvm_gmem_unbind(slot, gmem_file->private_data);
+ filemap_invalidate_unlock(gmem_file->f_mapping);
}
/* Returns a locked folio on success. */
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/5] KVM: selftests: fix maxnode arguments in xapic_ipi_test
2026-08-23 13:36 [PATCH 0/5] KVM: guest_memfd: fix unbind race and NUMA selftests Shivank Garg
2026-08-23 13:36 ` [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file Shivank Garg
@ 2026-08-23 13:36 ` Shivank Garg
2026-08-23 13:36 ` [PATCH 3/5] KVM: selftests: use BITS_PER_TYPE() for NUMA masks Shivank Garg
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Shivank Garg @ 2026-08-23 13:36 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Shuah Khan, Jim Mattson,
Peter Shier, Ricardo Koller, David Hildenbrand, Ackerley Tng
Cc: kvm, linux-kernel, linux-kselftest, Shivank Garg
migrate_pages() syscall expect maxnode to be one greater than the
number of bits in the nodemask. do_migrations() passes the size of
nodemask in bytes to migrate_pages(). This sets the maxnode to 8,
so kernel only checks node IDs 0-6 even though the nodemask covers
node IDs 0-63.
Pass the nodemask size in bits plus one because get_nodes() in
mempolicy does --maxnode.
Fixes: 678e90a349a4 ("KVM: selftests: Test IPI to halted vCPU in xAPIC while backing page moves")
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
tools/testing/selftests/kvm/x86/xapic_ipi_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
index 39ce9a9369f5..761e47e4cae2 100644
--- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
+++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
@@ -310,7 +310,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
* KVM_CREATE_VCPU ioctl. If that assumption ever changes this
* test may break or give a false positive signal.
*/
- pages_not_moved = migrate_pages(0, sizeof(nodemasks[from]),
+ pages_not_moved = migrate_pages(0, sizeof(nodemasks[from]) * 8 + 1,
&nodemasks[from],
&nodemasks[to]);
if (pages_not_moved < 0)
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/5] KVM: selftests: use BITS_PER_TYPE() for NUMA masks
2026-08-23 13:36 [PATCH 0/5] KVM: guest_memfd: fix unbind race and NUMA selftests Shivank Garg
2026-08-23 13:36 ` [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file Shivank Garg
2026-08-23 13:36 ` [PATCH 2/5] KVM: selftests: fix maxnode arguments in xapic_ipi_test Shivank Garg
@ 2026-08-23 13:36 ` Shivank Garg
2026-08-23 13:36 ` [PATCH 4/5] KVM: selftests: add get_numa_mem_nodes() Shivank Garg
2026-08-23 13:36 ` [PATCH 5/5] KVM: selftests: use allowed NUMA nodes in guest_memfd_test Shivank Garg
4 siblings, 0 replies; 9+ messages in thread
From: Shivank Garg @ 2026-08-23 13:36 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Shuah Khan, Jim Mattson,
Peter Shier, Ricardo Koller, David Hildenbrand, Ackerley Tng
Cc: kvm, linux-kernel, linux-kselftest, Shivank Garg
Replace the open-coded sizeof() * 8 calculations in do_migrations() with
BITS_PER_TYPE().
No functional change intended.
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
tools/testing/selftests/kvm/x86/xapic_ipi_test.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
index 761e47e4cae2..769d8d95ab2c 100644
--- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
+++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
@@ -252,7 +252,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
{
long pages_not_moved;
unsigned long nodemask = 0;
- unsigned long nodemasks[sizeof(nodemask) * 8];
+ unsigned long nodemasks[BITS_PER_TYPE(nodemask)];
int nodes = 0;
time_t start_time, last_update, now;
time_t interval_secs = 1;
@@ -267,18 +267,18 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
delay_usecs);
/* Get set of first 64 numa nodes available */
- kvm_get_mempolicy(NULL, &nodemask, sizeof(nodemask) * 8,
+ kvm_get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask),
0, MPOL_F_MEMS_ALLOWED);
fprintf(stderr, "Numa nodes found amongst first %lu possible nodes "
"(each 1-bit indicates node is present): %#lx\n",
- sizeof(nodemask) * 8, nodemask);
+ BITS_PER_TYPE(nodemask), nodemask);
/* Init array of masks containing a single-bit in each, one for each
* available node. migrate_pages called below requires specifying nodes
* as bit masks.
*/
- for (i = 0, bit = 1; i < sizeof(nodemask) * 8; i++, bit <<= 1) {
+ for (i = 0, bit = 1; i < BITS_PER_TYPE(nodemask); i++, bit <<= 1) {
if (nodemask & bit) {
nodemasks[nodes] = nodemask & bit;
nodes++;
@@ -310,7 +310,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
* KVM_CREATE_VCPU ioctl. If that assumption ever changes this
* test may break or give a false positive signal.
*/
- pages_not_moved = migrate_pages(0, sizeof(nodemasks[from]) * 8 + 1,
+ pages_not_moved = migrate_pages(0, BITS_PER_TYPE(nodemasks[from]) + 1,
&nodemasks[from],
&nodemasks[to]);
if (pages_not_moved < 0)
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/5] KVM: selftests: add get_numa_mem_nodes()
2026-08-23 13:36 [PATCH 0/5] KVM: guest_memfd: fix unbind race and NUMA selftests Shivank Garg
` (2 preceding siblings ...)
2026-08-23 13:36 ` [PATCH 3/5] KVM: selftests: use BITS_PER_TYPE() for NUMA masks Shivank Garg
@ 2026-08-23 13:36 ` Shivank Garg
2026-08-23 14:22 ` Garg, Shivank
2026-08-23 13:36 ` [PATCH 5/5] KVM: selftests: use allowed NUMA nodes in guest_memfd_test Shivank Garg
4 siblings, 1 reply; 9+ messages in thread
From: Shivank Garg @ 2026-08-23 13:36 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Shuah Khan, Jim Mattson,
Peter Shier, Ricardo Koller, David Hildenbrand, Ackerley Tng
Cc: kvm, linux-kernel, linux-kselftest, Shivank Garg
The xAPIC IPI test uses MPOL_F_MEMS_ALLOWED to get the memory nodes
available to the current process. Move the query to numaif.h as
get_numa_mem_nodes() so other KVM selftests can use it.
Call get_mempolicy() directly instead of using the assert-on-failure
kvm_get_mempolicy() wrapper. The xAPIC test already asserts its two
node requirement.
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
tools/testing/selftests/kvm/include/numaif.h | 12 ++++++++++++
tools/testing/selftests/kvm/x86/xapic_ipi_test.c | 6 ++----
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/numaif.h b/tools/testing/selftests/kvm/include/numaif.h
index 29572a6d789c..55124e2330ab 100644
--- a/tools/testing/selftests/kvm/include/numaif.h
+++ b/tools/testing/selftests/kvm/include/numaif.h
@@ -75,6 +75,18 @@ static bool is_numa_available(void)
(errno != ENOSYS && errno != EPERM);
}
+static inline unsigned long get_numa_mem_nodes(void)
+{
+ unsigned long nodemask = 0;
+
+ /* Get set of first 64 numa nodes available */
+ if (get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask), NULL,
+ MPOL_F_MEMS_ALLOWED))
+ return 0;
+
+ return nodemask;
+}
+
static inline bool is_multi_numa_node_system(void)
{
return is_numa_available() && get_max_numa_node() >= 1;
diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
index 769d8d95ab2c..66dcf36398aa 100644
--- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
+++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
@@ -251,7 +251,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
u64 *pipis_rcvd)
{
long pages_not_moved;
- unsigned long nodemask = 0;
+ unsigned long nodemask;
unsigned long nodemasks[BITS_PER_TYPE(nodemask)];
int nodes = 0;
time_t start_time, last_update, now;
@@ -266,9 +266,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
fprintf(stderr, "Calling migrate_pages every %d microseconds\n",
delay_usecs);
- /* Get set of first 64 numa nodes available */
- kvm_get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask),
- 0, MPOL_F_MEMS_ALLOWED);
+ nodemask = get_numa_mem_nodes();
fprintf(stderr, "Numa nodes found amongst first %lu possible nodes "
"(each 1-bit indicates node is present): %#lx\n",
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/5] KVM: selftests: use allowed NUMA nodes in guest_memfd_test
2026-08-23 13:36 [PATCH 0/5] KVM: guest_memfd: fix unbind race and NUMA selftests Shivank Garg
` (3 preceding siblings ...)
2026-08-23 13:36 ` [PATCH 4/5] KVM: selftests: add get_numa_mem_nodes() Shivank Garg
@ 2026-08-23 13:36 ` Shivank Garg
4 siblings, 0 replies; 9+ messages in thread
From: Shivank Garg @ 2026-08-23 13:36 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Shuah Khan, Jim Mattson,
Peter Shier, Ricardo Koller, David Hildenbrand, Ackerley Tng
Cc: kvm, linux-kernel, linux-kselftest, Shivank Garg
guest_memfd_test assumes that nodes 0 and 1 exist and have memory.
is_multi_numa_node_system() only checks that the maximum node ID is
nonzero, which is not enough for sparse or memoryless nodes.
Select the required nodes from MPOL_F_MEMS_ALLOWED instead. Use the full
nodemask width plus one to mbind(), and let test_mbind() run when only
one memory node is available.
The sysfs helpers for finding maxnode are no longer needed.
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
tools/testing/selftests/kvm/guest_memfd_test.c | 86 +++++++++++++++++---------
tools/testing/selftests/kvm/include/numaif.h | 52 ----------------
2 files changed, 58 insertions(+), 80 deletions(-)
diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index 2233d871a38f..aee80dda6229 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -76,33 +76,53 @@ static void test_mmap_supported(int fd, size_t total_size)
kvm_munmap(mem, total_size);
}
+/*
+ * Fill @nids with the first @nr_nids nodes in the allowed mask.
+ * Return false if the mask contains fewer than @nr_nids nodes.
+ */
+static bool get_numa_node_ids(int *nids, int nr_nids)
+{
+ unsigned long nodemask = get_numa_mem_nodes();
+ unsigned long nid;
+ int nr_found = 0;
+
+ for_each_set_bit(nid, &nodemask, BITS_PER_TYPE(nodemask)) {
+ nids[nr_found++] = nid;
+ if (nr_found == nr_nids)
+ return true;
+ }
+
+ return false;
+}
+
static void test_mbind(int fd, size_t total_size)
{
- const unsigned long nodemask_0 = 1; /* nid: 0 */
- unsigned long nodemask = 0;
- unsigned long maxnode = BITS_PER_TYPE(nodemask);
+ unsigned long nodemask, bind_nodemask;
+ unsigned long maxnode = BITS_PER_TYPE(nodemask) + 1;
int policy;
char *mem;
+ int nid;
int ret;
- if (!is_multi_numa_node_system())
+ if (!get_numa_node_ids(&nid, 1))
return;
+ bind_nodemask = 1UL << nid;
mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
/* Test MPOL_INTERLEAVE policy */
- kvm_mbind(mem, page_size * 2, MPOL_INTERLEAVE, &nodemask_0, maxnode, 0);
+ kvm_mbind(mem, page_size * 2, MPOL_INTERLEAVE, &bind_nodemask, maxnode, 0);
kvm_get_mempolicy(&policy, &nodemask, maxnode, mem, MPOL_F_ADDR);
- TEST_ASSERT(policy == MPOL_INTERLEAVE && nodemask == nodemask_0,
+ TEST_ASSERT(policy == MPOL_INTERLEAVE && nodemask == bind_nodemask,
"Wanted MPOL_INTERLEAVE (%u) and nodemask 0x%lx, got %u and 0x%lx",
- MPOL_INTERLEAVE, nodemask_0, policy, nodemask);
+ MPOL_INTERLEAVE, bind_nodemask, policy, nodemask);
/* Test basic MPOL_BIND policy */
- kvm_mbind(mem + page_size * 2, page_size * 2, MPOL_BIND, &nodemask_0, maxnode, 0);
+ kvm_mbind(mem + page_size * 2, page_size * 2, MPOL_BIND, &bind_nodemask, maxnode, 0);
kvm_get_mempolicy(&policy, &nodemask, maxnode, mem + page_size * 2, MPOL_F_ADDR);
- TEST_ASSERT(policy == MPOL_BIND && nodemask == nodemask_0,
+ TEST_ASSERT(policy == MPOL_BIND && nodemask == bind_nodemask,
"Wanted MPOL_BIND (%u) and nodemask 0x%lx, got %u and 0x%lx",
- MPOL_BIND, nodemask_0, policy, nodemask);
+ MPOL_BIND, bind_nodemask, policy, nodemask);
/* Test MPOL_DEFAULT policy */
kvm_mbind(mem, total_size, MPOL_DEFAULT, NULL, 0, 0);
@@ -112,7 +132,7 @@ static void test_mbind(int fd, size_t total_size)
MPOL_DEFAULT, policy, nodemask);
/* Test with invalid policy */
- ret = mbind(mem, page_size, 999, &nodemask_0, maxnode, 0);
+ ret = mbind(mem, page_size, 999, &bind_nodemask, maxnode, 0);
TEST_ASSERT(ret == -1 && errno == EINVAL,
"mbind with invalid policy should fail with EINVAL");
@@ -121,17 +141,19 @@ static void test_mbind(int fd, size_t total_size)
static void test_numa_allocation(int fd, size_t total_size)
{
- unsigned long node0_mask = 1; /* Node 0 */
- unsigned long node1_mask = 2; /* Node 1 */
- unsigned long maxnode = 8;
+ unsigned long bind_nodemasks[2];
+ unsigned long maxnode = BITS_PER_TYPE(bind_nodemasks[0]) + 1;
void *pages[4];
+ int nids[2];
int status[4];
char *mem;
int i;
- if (!is_multi_numa_node_system())
+ if (!get_numa_node_ids(nids, ARRAY_SIZE(nids)))
return;
+ bind_nodemasks[0] = 1UL << nids[0];
+ bind_nodemasks[1] = 1UL << nids[1];
mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
for (i = 0; i < 4; i++)
@@ -139,34 +161,42 @@ static void test_numa_allocation(int fd, size_t total_size)
/* Set NUMA policy after allocation */
memset(mem, 0xaa, page_size);
- kvm_mbind(pages[0], page_size, MPOL_BIND, &node0_mask, maxnode, 0);
+ kvm_mbind(pages[0], page_size, MPOL_BIND, &bind_nodemasks[0], maxnode, 0);
kvm_fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0, page_size);
/* Set NUMA policy before allocation */
- kvm_mbind(pages[0], page_size * 2, MPOL_BIND, &node1_mask, maxnode, 0);
- kvm_mbind(pages[2], page_size * 2, MPOL_BIND, &node0_mask, maxnode, 0);
+ kvm_mbind(pages[0], page_size * 2, MPOL_BIND, &bind_nodemasks[1], maxnode, 0);
+ kvm_mbind(pages[2], page_size * 2, MPOL_BIND, &bind_nodemasks[0], maxnode, 0);
memset(mem, 0xaa, total_size);
/* Validate if pages are allocated on specified NUMA nodes */
kvm_move_pages(0, 4, pages, NULL, status, 0);
- TEST_ASSERT(status[0] == 1, "Expected page 0 on node 1, got it on node %d", status[0]);
- TEST_ASSERT(status[1] == 1, "Expected page 1 on node 1, got it on node %d", status[1]);
- TEST_ASSERT(status[2] == 0, "Expected page 2 on node 0, got it on node %d", status[2]);
- TEST_ASSERT(status[3] == 0, "Expected page 3 on node 0, got it on node %d", status[3]);
+ TEST_ASSERT(status[0] == nids[1],
+ "Expected page 0 on node %d, got it on node %d", nids[1], status[0]);
+ TEST_ASSERT(status[1] == nids[1],
+ "Expected page 1 on node %d, got it on node %d", nids[1], status[1]);
+ TEST_ASSERT(status[2] == nids[0],
+ "Expected page 2 on node %d, got it on node %d", nids[0], status[2]);
+ TEST_ASSERT(status[3] == nids[0],
+ "Expected page 3 on node %d, got it on node %d", nids[0], status[3]);
/* Punch hole for all pages */
kvm_fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0, total_size);
/* Change NUMA policy nodes and reallocate */
- kvm_mbind(pages[0], page_size * 2, MPOL_BIND, &node0_mask, maxnode, 0);
- kvm_mbind(pages[2], page_size * 2, MPOL_BIND, &node1_mask, maxnode, 0);
+ kvm_mbind(pages[0], page_size * 2, MPOL_BIND, &bind_nodemasks[0], maxnode, 0);
+ kvm_mbind(pages[2], page_size * 2, MPOL_BIND, &bind_nodemasks[1], maxnode, 0);
memset(mem, 0xaa, total_size);
kvm_move_pages(0, 4, pages, NULL, status, 0);
- TEST_ASSERT(status[0] == 0, "Expected page 0 on node 0, got it on node %d", status[0]);
- TEST_ASSERT(status[1] == 0, "Expected page 1 on node 0, got it on node %d", status[1]);
- TEST_ASSERT(status[2] == 1, "Expected page 2 on node 1, got it on node %d", status[2]);
- TEST_ASSERT(status[3] == 1, "Expected page 3 on node 1, got it on node %d", status[3]);
+ TEST_ASSERT(status[0] == nids[0],
+ "Expected page 0 on node %d, got it on node %d", nids[0], status[0]);
+ TEST_ASSERT(status[1] == nids[0],
+ "Expected page 1 on node %d, got it on node %d", nids[0], status[1]);
+ TEST_ASSERT(status[2] == nids[1],
+ "Expected page 2 on node %d, got it on node %d", nids[1], status[2]);
+ TEST_ASSERT(status[3] == nids[1],
+ "Expected page 3 on node %d, got it on node %d", nids[1], status[3]);
kvm_munmap(mem, total_size);
}
diff --git a/tools/testing/selftests/kvm/include/numaif.h b/tools/testing/selftests/kvm/include/numaif.h
index 55124e2330ab..4bbbf314e9d7 100644
--- a/tools/testing/selftests/kvm/include/numaif.h
+++ b/tools/testing/selftests/kvm/include/numaif.h
@@ -4,8 +4,6 @@
#ifndef SELFTEST_KVM_NUMAIF_H
#define SELFTEST_KVM_NUMAIF_H
-#include <dirent.h>
-
#include <linux/mempolicy.h>
#include "kvm_syscalls.h"
@@ -30,51 +28,6 @@ KVM_SYSCALL_DEFINE(mbind, 6, void *, addr, unsigned long, size, int, mode,
const unsigned long *, nodemask, unsigned long, maxnode,
unsigned int, flags);
-static inline int get_max_numa_node(void)
-{
- struct dirent *de;
- int max_node = 0;
- DIR *d;
-
- /*
- * Assume there's a single node if the kernel doesn't support NUMA,
- * or if no nodes are found.
- */
- d = opendir("/sys/devices/system/node");
- if (!d)
- return 0;
-
- while ((de = readdir(d)) != NULL) {
- int node_id;
- char *endptr;
-
- if (strncmp(de->d_name, "node", 4) != 0)
- continue;
-
- node_id = strtol(de->d_name + 4, &endptr, 10);
- if (*endptr != '\0')
- continue;
-
- if (node_id > max_node)
- max_node = node_id;
- }
- closedir(d);
-
- return max_node;
-}
-
-static bool is_numa_available(void)
-{
- /*
- * Probe for NUMA by doing a dummy get_mempolicy(). If the syscall
- * fails with ENOSYS, then the kernel was built without NUMA support.
- * if the syscall fails with EPERM, then the process/user lacks the
- * necessary capabilities (CAP_SYS_NICE).
- */
- return !get_mempolicy(NULL, NULL, 0, NULL, 0) ||
- (errno != ENOSYS && errno != EPERM);
-}
-
static inline unsigned long get_numa_mem_nodes(void)
{
unsigned long nodemask = 0;
@@ -87,9 +40,4 @@ static inline unsigned long get_numa_mem_nodes(void)
return nodemask;
}
-static inline bool is_multi_numa_node_system(void)
-{
- return is_numa_available() && get_max_numa_node() >= 1;
-}
-
#endif /* SELFTEST_KVM_NUMAIF_H */
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 4/5] KVM: selftests: add get_numa_mem_nodes()
2026-08-23 13:36 ` [PATCH 4/5] KVM: selftests: add get_numa_mem_nodes() Shivank Garg
@ 2026-08-23 14:22 ` Garg, Shivank
0 siblings, 0 replies; 9+ messages in thread
From: Garg, Shivank @ 2026-08-23 14:22 UTC (permalink / raw)
To: jmattson@google.com, seanjc@google.com, david@kernel.org,
ackerleytng@google.com, pshier@google.com, shuah@kernel.org,
pbonzini@redhat.com, ricarkol@google.com
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest@vger.kernel.org
On Sun, 2026-08-23 at 13:36 +0000, Shivank Garg wrote:
> The xAPIC IPI test uses MPOL_F_MEMS_ALLOWED to get the memory nodes
> available to the current process. Move the query to numaif.h as
> get_numa_mem_nodes() so other KVM selftests can use it.
>
> Call get_mempolicy() directly instead of using the assert-on-failure
> kvm_get_mempolicy() wrapper. The xAPIC test already asserts its two
> node requirement.
>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
> tools/testing/selftests/kvm/include/numaif.h | 12 ++++++++++++
> tools/testing/selftests/kvm/x86/xapic_ipi_test.c | 6 ++----
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/numaif.h b/tools/testing/selftests/kvm/include/numaif.h
> index 29572a6d789c..55124e2330ab 100644
> --- a/tools/testing/selftests/kvm/include/numaif.h
> +++ b/tools/testing/selftests/kvm/include/numaif.h
> @@ -75,6 +75,18 @@ static bool is_numa_available(void)
> (errno != ENOSYS && errno != EPERM);
> }
>
> +static inline unsigned long get_numa_mem_nodes(void)
> +{
> + unsigned long nodemask = 0;
> +
> + /* Get set of first 64 numa nodes available */
> + if (get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask), NULL,
> + MPOL_F_MEMS_ALLOWED))
> + return 0;
> +
> + return nodemask;
> +}
> +
> static inline bool is_multi_numa_node_system(void)
> {
> return is_numa_available() && get_max_numa_node() >= 1;
> diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> index 769d8d95ab2c..66dcf36398aa 100644
> --- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> +++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> @@ -251,7 +251,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
> u64 *pipis_rcvd)
> {
> long pages_not_moved;
> - unsigned long nodemask = 0;
> + unsigned long nodemask;
> unsigned long nodemasks[BITS_PER_TYPE(nodemask)];
> int nodes = 0;
> time_t start_time, last_update, now;
> @@ -266,9 +266,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
> fprintf(stderr, "Calling migrate_pages every %d microseconds\n",
> delay_usecs);
>
> - /* Get set of first 64 numa nodes available */
> - kvm_get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask),
> - 0, MPOL_F_MEMS_ALLOWED);
> + nodemask = get_numa_mem_nodes();
>
> fprintf(stderr, "Numa nodes found amongst first %lu possible nodes "
> "(each 1-bit indicates node is present): %#lx\n",
>
From fd118b48e046c87d9b645b4ce7fc4ac5c0656431 Mon Sep 17 00:00:00 2001
From: Shivank Garg <shivankg@amd.com>
Date: Sun, 23 Aug 2026 14:11:42 +0000
Subject: [PATCH] Fix potential compilation issues
Sashiko reported that numaif.h does not include header that defines
BITS_PER_TYPE. If a test includes numaif.h without previously including
the header defining this macro, it might fail to compile due to a missing
definition, breaking header self-containment.
Link: https://lore.kernel.org/kvm/20260823134800.596C51F000E9@smtp.kernel.org
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
tools/testing/selftests/kvm/include/numaif.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/kvm/include/numaif.h b/tools/testing/selftests/kvm/include/numaif.h
index 4bbbf314e9d7..b89559b6ea3c 100644
--- a/tools/testing/selftests/kvm/include/numaif.h
+++ b/tools/testing/selftests/kvm/include/numaif.h
@@ -4,6 +4,7 @@
#ifndef SELFTEST_KVM_NUMAIF_H
#define SELFTEST_KVM_NUMAIF_H
+#include <linux/bits.h>
#include <linux/mempolicy.h>
#include "kvm_syscalls.h"
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file
2026-08-23 13:36 ` [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file Shivank Garg
@ 2026-08-25 22:13 ` Sean Christopherson
2026-08-26 10:28 ` Garg, Shivank
0 siblings, 1 reply; 9+ messages in thread
From: Sean Christopherson @ 2026-08-25 22:13 UTC (permalink / raw)
To: Shivank Garg
Cc: Paolo Bonzini, Shuah Khan, Jim Mattson, Peter Shier,
Ricardo Koller, David Hildenbrand, Ackerley Tng, kvm,
linux-kernel, linux-kselftest, Sashiko
On Sun, Aug 23, 2026, Shivank Garg wrote:
> kvm_gmem_unbind() skips mapping->invalidate_lock when the guest_memfd
> file is already dying. All other paths that modify f->bindings hold
> that lock.
>
> kvm_gmem_invalidate_{start,end}() checks f->bindings independently to
> decide whether to begin or end KVM MMU invalidations. So, the bindings
> must remain stable between the two calls. If a binding is removed in that
> window, start increments mmu_invalidate_in_progress but end does not
> decrement it. Example, unbind race with memory failure:
>
> CPU 0: memory failure CPU 1: memslot delete
> ---------------------------------- ---------------------------
> (guest_memfd file is dying)
> kvm_gmem_error_folio()
> kvm_gmem_invalidate_start()
> finds binding
> mmu_invalidate_in_progress++
> kvm_gmem_unbind()
> get_file_active() fails
> store NULL in bindings
> kvm_gmem_invalidate_end()
> no binding found
> counter stays elevated
>
> mmu_invalidate_retry() then returns 1 forever, so guest page faults
> retry without ever installing a mapping and the guest hangs.
>
> Take the invalidate lock in the dying-file path too. This prevents unbind
> from removing a binding and leaking mmu_invalidate_in_progress. This is
> safe because any caller that reaches this path holds slots_lock, so
> kvm_gmem_release() cannot nullify the slot->gmem.file, until
> kvm_gmem_unbind() finishes.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/20260728092027.225CF1F000E9@smtp.kernel.org
> Fixes: ae431059e75d ("KVM: guest_memfd: Remove bindings on memslot deletion when gmem is dying")
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
> virt/kvm/guest_memfd.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index f0e5da490866..f848120af84b 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -721,6 +721,8 @@ static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
>
> void kvm_gmem_unbind(struct kvm_memory_slot *slot)
> {
> + struct file *gmem_file;
> +
> /*
> * Nothing to do if the underlying file was _already_ closed, as
> * kvm_gmem_release() invalidates and nullifies all bindings.
> @@ -733,21 +735,24 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
> /*
> * However, if the file is _being_ closed, then the bindings need to be
> * removed as kvm_gmem_release() might not run until after the memslot
> - * is freed. Note, modifying the bindings is safe even though the file
> - * is dying as kvm_gmem_release() nullifies slot->gmem.file under
> + * is freed. Note, dereferencing the dying file is safe as
> + * kvm_gmem_release() nullifies slot->gmem.file under
> * slots_lock, and only puts its reference to KVM after destroying all
> * bindings. I.e. reaching this point means kvm_gmem_release() hasn't
> * yet destroyed the bindings or freed the gmem_file, and can't do so
> * until the caller drops slots_lock.
> */
> - if (!file) {
> - __kvm_gmem_unbind(slot, slot->gmem.file->private_data);
> - return;
> - }
> + gmem_file = file ?: slot->gmem.file;
>
> - filemap_invalidate_lock(file->f_mapping);
> - __kvm_gmem_unbind(slot, file->private_data);
> - filemap_invalidate_unlock(file->f_mapping);
> + /*
> + * Take the invalidate lock even for a dying file. Otherwise,
> + * kvm_gmem_invalidate_start() can find the binding and increment
> + * mmu_invalidate_in_progress while kvm_gmem_invalidate_end() misses
> + * the removed binding and skips decrement.
Hmm, so as called out in commit ae431059e75d ("KVM: guest_memfd: Remove bindings
on memslot deletion when gmem is dying"), this assumes that file->f_mapping and
everything "underneath" remains valid for dying files, which makes me a bit
uncomfortable.
Deliberately don't acquire filemap invalid lock when the file is dying as
the lifecycle of f_mapping is outside the purview of KVM. Dereferencing
the mapping is *probably* fine, but there's no need to invalidate anything
as memslot deletion is responsible for zapping SPTEs, and the only code
that can access the dying file is kvm_gmem_release(), whose core code is
mutually exclusive with unbinding.
Oh, but kvm_gmem_release() takes the same filemap_invalidate_unlock() and
holding slots_lock guarantees that this code would run before release() if it
sees a non-null slot->gmem.file, i.e. past me's concern is completely unfounded.
Rather than make this seem like something special, IMO we should treat this as
a more normal thing. kvm->slots_lock is already load bearing, might as well
double down on that. I.e. the exceptional part is doing all the work even though
the file is dying, but the flows themselves should be identical.
E.g.
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index b596486d184c..5cc043466c89 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -668,48 +668,40 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
return r;
}
-static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
+void kvm_gmem_unbind(struct kvm_memory_slot *slot)
{
+ struct file *file = slot->gmem.file;
unsigned long start = slot->gmem.pgoff;
unsigned long end = start + slot->npages;
+ struct gmem_file *f;
- xa_store_range(&f->bindings, start, end - 1, NULL, GFP_KERNEL);
-
- /*
- * synchronize_srcu(&kvm->srcu) ensured that kvm_gmem_get_pfn()
- * cannot see this memslot.
- */
- WRITE_ONCE(slot->gmem.file, NULL);
-}
-
-void kvm_gmem_unbind(struct kvm_memory_slot *slot)
-{
/*
* Nothing to do if the underlying file was _already_ closed, as
* kvm_gmem_release() invalidates and nullifies all bindings.
*/
- if (!slot->gmem.file)
+ if (!file)
return;
- CLASS(gmem_get_file, file)(slot);
-
/*
* However, if the file is _being_ closed, then the bindings need to be
* removed as kvm_gmem_release() might not run until after the memslot
- * is freed. Note, modifying the bindings is safe even though the file
- * is dying as kvm_gmem_release() nullifies slot->gmem.file under
- * slots_lock, and only puts its reference to KVM after destroying all
- * bindings. I.e. reaching this point means kvm_gmem_release() hasn't
- * yet destroyed the bindings or freed the gmem_file, and can't do so
- * until the caller drops slots_lock.
+ * is freed. Modifying the bindings is safe even if the file is dying
+ * as kvm_gmem_release() nullifies slot->gmem.file under slots_lock,
+ * and only puts its reference to KVM after destroying all bindings.
+ * I.e. reaching this point means kvm_gmem_release() hasn't destroyed
+ * the bindings or freed the gmem_file and can't do so until the caller
+ * drops slots_lock, so there's no need to verify the file is live.
*/
- if (!file) {
- __kvm_gmem_unbind(slot, slot->gmem.file->private_data);
- return;
- }
+ f = file->private_data;
filemap_invalidate_lock(file->f_mapping);
- __kvm_gmem_unbind(slot, file->private_data);
+ xa_store_range(&f->bindings, start, end - 1, NULL, GFP_KERNEL);
+
+ /*
+ * synchronize_srcu(&kvm->srcu) ensured that kvm_gmem_get_pfn()
+ * cannot see this memslot.
+ */
+ WRITE_ONCE(slot->gmem.file, NULL);
filemap_invalidate_unlock(file->f_mapping);
}
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file
2026-08-25 22:13 ` Sean Christopherson
@ 2026-08-26 10:28 ` Garg, Shivank
0 siblings, 0 replies; 9+ messages in thread
From: Garg, Shivank @ 2026-08-26 10:28 UTC (permalink / raw)
To: seanjc@google.com
Cc: jmattson@google.com, david@kernel.org, ackerleytng@google.com,
pshier@google.com, shuah@kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest@vger.kernel.org, pbonzini@redhat.com,
ricarkol@google.com, kvm@vger.kernel.org, sashiko-bot@kernel.org
On Tue, 2026-08-25 at 15:13 -0700, Sean Christopherson wrote:
> On Sun, Aug 23, 2026, Shivank Garg wrote:
> > kvm_gmem_unbind() skips mapping->invalidate_lock when the guest_memfd
> > file is already dying. All other paths that modify f->bindings hold
> > that lock.
> >
> > kvm_gmem_invalidate_{start,end}() checks f->bindings independently to
> > decide whether to begin or end KVM MMU invalidations. So, the bindings
> > must remain stable between the two calls. If a binding is removed in that
> > window, start increments mmu_invalidate_in_progress but end does not
> > decrement it. Example, unbind race with memory failure:
> >
> > CPU 0: memory failure CPU 1: memslot delete
> > ---------------------------------- ---------------------------
> > (guest_memfd file is dying)
> > kvm_gmem_error_folio()
> > kvm_gmem_invalidate_start()
> > finds binding
> > mmu_invalidate_in_progress++
> > kvm_gmem_unbind()
> > get_file_active() fails
> > store NULL in bindings
> > kvm_gmem_invalidate_end()
> > no binding found
> > counter stays elevated
> >
> > mmu_invalidate_retry() then returns 1 forever, so guest page faults
> > retry without ever installing a mapping and the guest hangs.
> >
> > Take the invalidate lock in the dying-file path too. This prevents unbind
> > from removing a binding and leaking mmu_invalidate_in_progress. This is
> > safe because any caller that reaches this path holds slots_lock, so
> > kvm_gmem_release() cannot nullify the slot->gmem.file, until
> > kvm_gmem_unbind() finishes.
> >
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/all/20260728092027.225CF1F000E9@smtp.kernel.org
> > Fixes: ae431059e75d ("KVM: guest_memfd: Remove bindings on memslot deletion when gmem is dying")
> > Signed-off-by: Shivank Garg <shivankg@amd.com>
> > ---
> > virt/kvm/guest_memfd.c | 23 ++++++++++++++---------
> > 1 file changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index f0e5da490866..f848120af84b 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -721,6 +721,8 @@ static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
> >
> > void kvm_gmem_unbind(struct kvm_memory_slot *slot)
> > {
> > + struct file *gmem_file;
> > +
> > /*
> > * Nothing to do if the underlying file was _already_ closed, as
> > * kvm_gmem_release() invalidates and nullifies all bindings.
> > @@ -733,21 +735,24 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
> > /*
> > * However, if the file is _being_ closed, then the bindings need to be
> > * removed as kvm_gmem_release() might not run until after the memslot
> > - * is freed. Note, modifying the bindings is safe even though the file
> > - * is dying as kvm_gmem_release() nullifies slot->gmem.file under
> > + * is freed. Note, dereferencing the dying file is safe as
> > + * kvm_gmem_release() nullifies slot->gmem.file under
> > * slots_lock, and only puts its reference to KVM after destroying all
> > * bindings. I.e. reaching this point means kvm_gmem_release() hasn't
> > * yet destroyed the bindings or freed the gmem_file, and can't do so
> > * until the caller drops slots_lock.
> > */
> > - if (!file) {
> > - __kvm_gmem_unbind(slot, slot->gmem.file->private_data);
> > - return;
> > - }
> > + gmem_file = file ?: slot->gmem.file;
> >
> > - filemap_invalidate_lock(file->f_mapping);
> > - __kvm_gmem_unbind(slot, file->private_data);
> > - filemap_invalidate_unlock(file->f_mapping);
> > + /*
> > + * Take the invalidate lock even for a dying file. Otherwise,
> > + * kvm_gmem_invalidate_start() can find the binding and increment
> > + * mmu_invalidate_in_progress while kvm_gmem_invalidate_end() misses
> > + * the removed binding and skips decrement.
>
> Hmm, so as called out in commit ae431059e75d ("KVM: guest_memfd: Remove bindings
> on memslot deletion when gmem is dying"), this assumes that file->f_mapping and
> everything "underneath" remains valid for dying files, which makes me a bit
> uncomfortable.
>
> Deliberately don't acquire filemap invalid lock when the file is dying as
> the lifecycle of f_mapping is outside the purview of KVM. Dereferencing
> the mapping is *probably* fine, but there's no need to invalidate anything
> as memslot deletion is responsible for zapping SPTEs, and the only code
> that can access the dying file is kvm_gmem_release(), whose core code is
> mutually exclusive with unbinding.
>
> Oh, but kvm_gmem_release() takes the same filemap_invalidate_unlock() and
> holding slots_lock guarantees that this code would run before release() if it
> sees a non-null slot->gmem.file, i.e. past me's concern is completely unfounded.
>
> Rather than make this seem like something special, IMO we should treat this as
> a more normal thing. kvm->slots_lock is already load bearing, might as well
> double down on that. I.e. the exceptional part is doing all the work even though
> the file is dying, but the flows themselves should be identical.
>
> E.g.
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index b596486d184c..5cc043466c89 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -668,48 +668,40 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
> return r;
> }
>
> -static void __kvm_gmem_unbind(struct kvm_memory_slot *slot, struct gmem_file *f)
> +void kvm_gmem_unbind(struct kvm_memory_slot *slot)
> {
> + struct file *file = slot->gmem.file;
> unsigned long start = slot->gmem.pgoff;
> unsigned long end = start + slot->npages;
> + struct gmem_file *f;
>
> - xa_store_range(&f->bindings, start, end - 1, NULL, GFP_KERNEL);
> -
> - /*
> - * synchronize_srcu(&kvm->srcu) ensured that kvm_gmem_get_pfn()
> - * cannot see this memslot.
> - */
> - WRITE_ONCE(slot->gmem.file, NULL);
> -}
> -
> -void kvm_gmem_unbind(struct kvm_memory_slot *slot)
> -{
> /*
> * Nothing to do if the underlying file was _already_ closed, as
> * kvm_gmem_release() invalidates and nullifies all bindings.
> */
> - if (!slot->gmem.file)
> + if (!file)
> return;
>
> - CLASS(gmem_get_file, file)(slot);
> -
> /*
> * However, if the file is _being_ closed, then the bindings need to be
> * removed as kvm_gmem_release() might not run until after the memslot
> - * is freed. Note, modifying the bindings is safe even though the file
> - * is dying as kvm_gmem_release() nullifies slot->gmem.file under
> - * slots_lock, and only puts its reference to KVM after destroying all
> - * bindings. I.e. reaching this point means kvm_gmem_release() hasn't
> - * yet destroyed the bindings or freed the gmem_file, and can't do so
> - * until the caller drops slots_lock.
> + * is freed. Modifying the bindings is safe even if the file is dying
> + * as kvm_gmem_release() nullifies slot->gmem.file under slots_lock,
> + * and only puts its reference to KVM after destroying all bindings.
> + * I.e. reaching this point means kvm_gmem_release() hasn't destroyed
> + * the bindings or freed the gmem_file and can't do so until the caller
> + * drops slots_lock, so there's no need to verify the file is live.
> */
> - if (!file) {
> - __kvm_gmem_unbind(slot, slot->gmem.file->private_data);
> - return;
> - }
> + f = file->private_data;
>
> filemap_invalidate_lock(file->f_mapping);
> - __kvm_gmem_unbind(slot, file->private_data);
> + xa_store_range(&f->bindings, start, end - 1, NULL, GFP_KERNEL);
> +
> + /*
> + * synchronize_srcu(&kvm->srcu) ensured that kvm_gmem_get_pfn()
> + * cannot see this memslot.
> + */
> + WRITE_ONCE(slot->gmem.file, NULL);
> filemap_invalidate_unlock(file->f_mapping);
> }
Thanks Sean.
This version is more simple and clear.
Best regards,
Shivank
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-26 10:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-23 13:36 [PATCH 0/5] KVM: guest_memfd: fix unbind race and NUMA selftests Shivank Garg
2026-08-23 13:36 ` [PATCH 1/5] KVM: guest_memfd: take the invalidate lock when unbinding a dying file Shivank Garg
2026-08-25 22:13 ` Sean Christopherson
2026-08-26 10:28 ` Garg, Shivank
2026-08-23 13:36 ` [PATCH 2/5] KVM: selftests: fix maxnode arguments in xapic_ipi_test Shivank Garg
2026-08-23 13:36 ` [PATCH 3/5] KVM: selftests: use BITS_PER_TYPE() for NUMA masks Shivank Garg
2026-08-23 13:36 ` [PATCH 4/5] KVM: selftests: add get_numa_mem_nodes() Shivank Garg
2026-08-23 14:22 ` Garg, Shivank
2026-08-23 13:36 ` [PATCH 5/5] KVM: selftests: use allowed NUMA nodes in guest_memfd_test Shivank Garg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox