On Tue, 2026-09-01 at 08:04 -0700, Sean Christopherson wrote: > On Tue, Sep 01, 2026, Shivank Garg wrote: > > 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. > > Ok, I'm not crazy. I read all of this multiple times and the manpages seemed > completely nonsensical. Looking at QEMU's use of mbind(), it's the kernel > that > sucks: > > /* > * We can have up to MAX_NODES nodes, but we need to pass maxnode+1 > * as argument to mbind() due to an old Linux bug (feature?) which > * cuts off the last specified node. This means backend->host_nodes > * must have MAX_NODES+1 bits available. > */ > assert(sizeof(backend->host_nodes) >= > BITS_TO_LONGS(MAX_NODES + 1) * sizeof(unsigned long)); > > > And from > https://lore.kernel.org/all/[email protected]: > > : And this is I think the reason why we can't change this now. I assume > : numactl allocates 1024 bits (0 to 1023) and passes 1025 to make sure all > : 1024 bits are processed. If we change it now, kernel will process 1025 > : bits (0 to 1024) and overflow the allocated bitmask. If it happens to be > : at the border of mmaped vma, it's a segfault... > > This is quite possibly the most confusing syscall interface ever, and IMO the > manpage is still straight up wrong (well, the kernel is the one that's buggy, > but the manpage doesn't reflect the kernel's behavior): > > : The maxnode argument is the maximum node number in the bit mask plus one > > Beacuse it's not the maximum node number plus one, it's the number of nodes in > the bitask plus one. > > What a confusing mess.
Agreed. The discussion resurfaced here too: https://lore.kernel.org/linux-mm/[email protected] I'll fix the manpage for this. > > > > Fixes: 678e90a349a4 ("KVM: selftests: Test IPI to halted vCPU in xAPIC > > while backing page moves") > > Signed-off-by: Shivank Garg <[email protected]> > > --- > > 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 469e3ab16460..0f11f4d7cc3d 100644 > > --- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c > > +++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c > > @@ -291,7 +291,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, > > BITS_PER_TYPE() > > However, given that it's basically impossible for developers to get this > right, > we should add "#define MAXNODE_FOR_MASK(mask) (BITS_PER_TYPE(mask) + 1)" with > a > big comment explain why the code looks wrong. > > E.g. patch 3 gets it wrong in get_numa_mem_nodes(): > > 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; > } > > because that will only get the mask for bits 62:0. Even Sashiko got confused > in > patch 4: > > When maxnode is passed to kvm_get_mempolicy() here, the kernel must write at > least 5 bytes to return 33 bits of node status. This rounds up to 8 bytes > (two > 32-bit words). > > since the disaster of a syscall that is get_mempolicy() and friends will only > provide 32 bits of node status. > > Looking at the rest of the patches in this series, the main goal of fixing the > extremely-unlikely-to-happen-in-practice bug in patch 4 needs a lot of work. > To > move along the other cleanups, I'll send the below plus rebased versions of > patches 1 and 2. > > diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c > b/tools/testing/selftests/kvm/guest_memfd_test.c > index 2233d871a38f..cd5df88bc642 100644 > --- a/tools/testing/selftests/kvm/guest_memfd_test.c > +++ b/tools/testing/selftests/kvm/guest_memfd_test.c > @@ -80,7 +80,7 @@ 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 maxnode = MAXNODE_FOR_MASK(nodemask); > int policy; > char *mem; > int ret; > diff --git a/tools/testing/selftests/kvm/include/numaif.h > b/tools/testing/selftests/kvm/include/numaif.h > index 29572a6d789c..84e82857f1c5 100644 > --- a/tools/testing/selftests/kvm/include/numaif.h > +++ b/tools/testing/selftests/kvm/include/numaif.h > @@ -30,6 +30,16 @@ KVM_SYSCALL_DEFINE(mbind, 6, void *, addr, unsigned long, > size, int, mode, > const unsigned long *, nodemask, unsigned long, maxnode, > unsigned int, flags); > > +/* > + * Caclucate the @maxnode param for the above syscalls given the mask that > will > + * be passed to the kernel, to account for a longstanding off-by-one bug in > the > + * kernel that isn't properly documented in the manpages. The manpages say > + * that @maxnode is "the maximum node ID plus one", but the kernel's actual > + * behavior is "the number of bits in the mask plus one", i.e. "the maximum > + * node ID plus two". > + */ > +#define MAXNODE_FOR_MASK(mask) (BITS_PER_TYPE(mask) + 1) > + > static inline int get_max_numa_node(void) > { > struct dirent *de; > diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c > b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c > index 469e3ab16460..1ddcf95d7fe4 100644 > --- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c > +++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c > @@ -248,7 +248,7 @@ 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, MAXNODE_FOR_MASK(nodemask), > 0, MPOL_F_MEMS_ALLOWED); > > fprintf(stderr, "Numa nodes found amongst first %lu possible nodes " Thanks for this patch. I noticed you've already spun out a series with this, I'll review it shortly. Best regards, Shivank
