All of lore.kernel.org
 help / color / mirror / Atom feed
* bug: move_pages(2) does not udpate "status" if no pages are moved
@ 2019-12-04 19:01 Felix Abecassis
  2019-12-04 19:21 ` John Hubbard
  2019-12-04 20:17 ` Yang Shi
  0 siblings, 2 replies; 10+ messages in thread
From: Felix Abecassis @ 2019-12-04 19:01 UTC (permalink / raw)
  To: linux-mm; +Cc: Andrew Morton

Hello all,

On kernel 5.3, when using the move_pages syscall (wrapped by libnuma) and all
pages happen to be on the right node already, this function returns 0 but the
"status" array is not updated. This array potentially contains garbage values
(e.g. from malloc(3)), and I don't see a way to detect this.

Looking at the kernel code, we are probably exiting do_pages_move here:
out_flush:
    if (list_empty(&pagelist))
        return err;

Here is a sample C program to reproduce the problem:
/* $ gcc move_pages_bug.c -lnuma -o move_pages_bug */

#define _DEFAULT_SOURCE
#include <sys/mman.h>

#include <numaif.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>

int main(void)
{
    const long node_id = 1;
    const long page_size = sysconf(_SC_PAGESIZE);
    const int64_t num_pages = 8;

    unsigned long nodemask =  1 << node_id;
    long ret = set_mempolicy(MPOL_BIND, &nodemask, sizeof(nodemask));
    if (ret < 0)
        return (EXIT_FAILURE);

    void **pages = malloc(sizeof(void*) * num_pages);
    for (int i = 0; i < num_pages; ++i) {
        pages[i] = mmap(NULL, page_size, PROT_WRITE | PROT_READ,
                MAP_PRIVATE | MAP_POPULATE | MAP_ANONYMOUS,
                -1, 0);
        if (pages[i] == MAP_FAILED)
            return (EXIT_FAILURE);
    }

    ret = set_mempolicy(MPOL_DEFAULT, NULL, 0);
    if (ret < 0)
        return (EXIT_FAILURE);

    int *nodes = malloc(sizeof(int) * num_pages);
    int *status = malloc(sizeof(int) * num_pages);
    for (int i = 0; i < num_pages; ++i) {
        nodes[i] = node_id;
        status[i] = 0xd0; /* simulate garbage values */
    }

    ret = move_pages(0, num_pages, pages, nodes, status, MPOL_MF_MOVE);
    printf("move_pages: %ld\n", ret);
    for (int i = 0; i < num_pages; ++i)
        printf("status[%d] = %d\n", i, status[i]);
}

And here is a sample output showing the "garbage" values:
$ ./move_pages_bug
move_pages: 0
status[0] = 208
status[1] = 208
status[2] = 208
status[3] = 208
status[4] = 208
status[5] = 208
status[6] = 208
status[7] = 208

Note that passing NULL as the "nodes" argument works as expected here.

Also, it seems that it's the last "run-length" of pages on the right node(s)
that triggers this problem, e.g. if I add "nodes[0] = nodes[1] = 0", then the
output becomes:
$ ./move_pages_bug
move_pages: 0
status[0] = 0
status[1] = 0
status[2] = 208
status[3] = 208
status[4] = 208
status[5] = 208
status[6] = 208
status[7] = 208

And with just "nodes[7] = 0;", the first run-length of pages gets assigned
correctly:
$ ./move_pages_bug
move_pages: 0
status[0] = 1
status[1] = 1
status[2] = 1
status[3] = 1
status[4] = 1
status[5] = 1
status[6] = 1
status[7] = 0


Thank you,


^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-12-05  0:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-04 19:01 bug: move_pages(2) does not udpate "status" if no pages are moved Felix Abecassis
2019-12-04 19:21 ` John Hubbard
2019-12-04 20:04   ` Yang Shi
2019-12-04 23:45     ` John Hubbard
2019-12-05  0:43       ` Yang Shi
2019-12-04 20:17 ` Yang Shi
2019-12-04 22:54   ` Felix Abecassis
2019-12-04 23:37     ` Yang Shi
2019-12-05  0:03   ` John Hubbard
2019-12-05  0:40     ` Yang Shi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.