lkml.org 
[lkml]   [2012]   [May]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: mm: move_pages syscall can't return ENOENT when pages are not present
    On 05/09/2012 09:33 PM, Zhouping Liu wrote:

    >
    >
    > ----- Original Message -----
    >> From: "Wanlong Gao" <gaowanlong@cn.fujitsu.com>
    >> To: "Xiaotian Feng" <xtfeng@gmail.com>
    >> Cc: "Zhouping Liu" <zliu@redhat.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org, "LTP List"
    >> <ltp-list@lists.sourceforge.net>
    >> Sent: Wednesday, May 9, 2012 8:50:07 PM
    >> Subject: Re: mm: move_pages syscall can't return ENOENT when pages are not present
    >>
    >> On 05/09/2012 05:28 PM, Xiaotian Feng wrote:
    >>
    >>> On Wed, May 9, 2012 at 4:58 PM, Zhouping Liu <zliu@redhat.com>
    >>> wrote:
    >>>> hi, all
    >>>>
    >>>> Recently, I found an error in move_pages syscall:
    >>>>
    >>>> depending on move_pages(2), when page is not present,
    >>>> it should fail with ENOENT, in fact, it's ok without
    >>>> any errno.
    >>>>
    >>>> the following reproducer can easily reproduce
    >>>> the issue, suggest you get more details by strace.
    >>>> inside reproducer, I try to move a non-exist page from
    >>>> node 1 to node 0.
    >>>>
    >>>> I have tested it on the latest kernel 3.4-rc5 with 2 and 4 numa
    >>>> nodes.
    >>>> [zliu@ZhoupingLiu ~]$ gcc -o reproducer reproducer.c -lnuma
    >>>> [zliu@ZhoupingLiu ~]$ ./reproducer
    >>>> from_node is 1, to_node is 0
    >>>> ERROR: move_pages expected FAIL.
    >>>>
    >>>
    >>> " If nodes is not NULL, move_pages returns the number of valid
    >>> migration requests which could not currently be performed.
    >>> Otherwise
    >>> it returns 0."
    >>
    >>
    >> FYI, actually,
    >> commit e78bbfa8262424417a29349a8064a535053912b9
    >> Author: Brice Goglin <Brice.Goglin@inria.fr>
    >> Date: Sat Oct 18 20:27:15 2008 -0700
    >>
    >> mm: stop returning -ENOENT from sys_move_pages() if nothing got
    >> migrated
    >
    > maybe you missed my thought :(
    > if I'm wrong, please correct me.
    >
    > IMO, the issue is different with the patch.
    > apparently, in the case(reproducer), I tried to move 4 pages from node 1 to node 0,
    > and the 4th page is an invalid page(absent and not aligned)
    > pages[TEST_PAGES - 1] = pages[TEST_PAGES - 2] - onepage * 4 + 1;
    > but the reproducer passed with any errors, I think it's not common.
    >
    > in the case, numa_free() return EINVAL, but we can't catch the err:


    So, as Brice said, if you want to catch error, you should check your status array after
    doing move_pages.


    Thanks,
    Wanlong Gao

    > [root@ZhoupingLiu zliu]# strace ./reproducer
    > ...
    > move_pages(0, 4, {0x7f029c459000, 0x7f029c458000, 0x7f029c457000, 0x7f029c453001}, {0, 0, 0, 0}, {0xfffffffe, 0xfffffffe, 0xfffffffe, 0xfffffff2}, MPOL_MF_MOVE) = 0
    > write(1, "ERROR: move_pages expected FAIL."..., 33ERROR: move_pages expected FAIL.
    > ) = 33
    > munmap(0x7f029c459000, 4096) = 0
    > munmap(0x7f029c458000, 4096) = 0
    > munmap(0x7f029c457000, 4096) = 0
    > munmap(0x7f029c453001, 4096) = -1 EINVAL (Invalid argument)
    > ...
    >
    > so I suggest we check pages' validity before move pages, if they are invalid, it should return
    > relevant error number to userspace, maybe it's other errno, not ENOENT, correct?
    >
    > I'm trying to make a patch, but I'm a newer to the part :(
    >
    >>
    >> this commit changed the behaviour.
    >>
    >> And the LTP has fixed to be consistent with this,
    >> https://github.com/linux-test-project/ltp/commit/338299da1ff27c7815183c1b07eb91e705f117ce
    >>
    >>
    >> Thanks,
    >> Wanlong Gao
    >>
    >>>
    >>>> I'm not in mail list, please CC me.
    >>>>
    >>>> /*
    >>>> * Copyright (C) 2012 Red Hat, Inc.
    >>>> *
    >>>> * This work is licensed under the terms of the GNU GPL, version
    >>>> 2. See
    >>>> * the COPYING file in the top-level directory.
    >>>> *
    >>>> * Compiled: gcc -o reproducer reproducer.c -lnuma
    >>>> * Description:
    >>>> * it's designed to check move_pages syscall, when
    >>>> * page is not present, it should fail with ENOENT.
    >>>> */
    >>>>
    >>>> #include <sys/mman.h>
    >>>> #include <sys/types.h>
    >>>> #include <sys/wait.h>
    >>>> #include <stdio.h>
    >>>> #include <unistd.h>
    >>>> #include <errno.h>
    >>>> #include <numa.h>
    >>>> #include <numaif.h>
    >>>>
    >>>> #define TEST_PAGES 4
    >>>>
    >>>> int main(int argc, char **argv)
    >>>> {
    >>>> void *pages[TEST_PAGES];
    >>>> int onepage;
    >>>> int nodes[TEST_PAGES];
    >>>> int status, ret;
    >>>> int i, from_node = 1, to_node = 0;
    >>>>
    >>>> onepage = getpagesize();
    >>>>
    >>>> for (i = 0; i < TEST_PAGES - 1; i++) {
    >>>> pages[i] = numa_alloc_onnode(onepage, from_node);
    >>>> nodes[i] = to_node;
    >>>> }
    >>>>
    >>>> nodes[TEST_PAGES - 1] = to_node;
    >>>>
    >>>> /*
    >>>> * the follow page is not available, also not aligned,
    >>>> * depend on move_pages(2), it can't be moved, and should
    >>>> * return ENOENT errno.
    >>>> */
    >>>> pages[TEST_PAGES - 1] = pages[TEST_PAGES - 2] - onepage * 4
    >>>> + 1;
    >>>>
    >>>> printf("from_node is %u, to_node is %u\n", from_node,
    >>>> to_node);
    >>>> ret = move_pages(0, TEST_PAGES, pages, nodes, &status,
    >>>> MPOL_MF_MOVE);
    >>>> if (ret == -1) {
    >>>> if (errno != ENOENT)
    >>>> perror("move_pages expected ENOENT errno,
    >>>> but it's");
    >>>> else
    >>>> printf("Succeed\n");
    >>>> } else {
    >>>> printf("ERROR: move_pages expected FAIL.\n");
    >>>> }
    >>>>
    >>>> for (i = 0; i < TEST_PAGES; i++)
    >>>> numa_free(pages[i], onepage);
    >>>>
    >>>> return 0;
    >>>> }
    >>>>
    >>>> --
    >>>> Thanks,
    >>>> Zhouping
    >>>> --
    >>>> To unsubscribe from this list: send the line "unsubscribe
    >>>> linux-kernel" in
    >>>> the body of a message to majordomo@vger.kernel.org
    >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
    >>>> Please read the FAQ at http://www.tux.org/lkml/
    >>> --
    >>> To unsubscribe from this list: send the line "unsubscribe
    >>> linux-kernel" in
    >>> the body of a message to majordomo@vger.kernel.org
    >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
    >>> Please read the FAQ at http://www.tux.org/lkml/
    >>>
    >>
    >>
    >>
    >




    \
     
     \ /
      Last update: 2012-05-09 17:41    [W:0.047 / U:0.036 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site