git.ipfire.org Git - thirdparty/kernel/stable-queue.git/blob

   1 From e0153fc2c7606f101392b682e720a7a456d6c766 Mon Sep 17 00:00:00 2001
   2 From: Yang Shi <yang.shi@linux.alibaba.com>
   3 Date: Sat, 4 Jan 2020 12:59:46 -0800
   4 Subject: mm: move_pages: return valid node id in status if the page is already on the target node
   5
   6 From: Yang Shi <yang.shi@linux.alibaba.com>
   7
   8 commit e0153fc2c7606f101392b682e720a7a456d6c766 upstream.
   9
  10 Felix Abecassis reports move_pages() would return random status if the
  11 pages are already on the target node by the below test program:
  12
  13   int main(void)
  14   {
  15         const long node_id = 1;
  16         const long page_size = sysconf(_SC_PAGESIZE);
  17         const int64_t num_pages = 8;
  18
  19         unsigned long nodemask =  1 << node_id;
  20         long ret = set_mempolicy(MPOL_BIND, &nodemask, sizeof(nodemask));
  21         if (ret < 0)
  22                 return (EXIT_FAILURE);
  23
  24         void **pages = malloc(sizeof(void*) * num_pages);
  25         for (int i = 0; i < num_pages; ++i) {
  26                 pages[i] = mmap(NULL, page_size, PROT_WRITE | PROT_READ,
  27                                 MAP_PRIVATE | MAP_POPULATE | MAP_ANONYMOUS,
  28                                 -1, 0);
  29                 if (pages[i] == MAP_FAILED)
  30                         return (EXIT_FAILURE);
  31         }
  32
  33         ret = set_mempolicy(MPOL_DEFAULT, NULL, 0);
  34         if (ret < 0)
  35                 return (EXIT_FAILURE);
  36
  37         int *nodes = malloc(sizeof(int) * num_pages);
  38         int *status = malloc(sizeof(int) * num_pages);
  39         for (int i = 0; i < num_pages; ++i) {
  40                 nodes[i] = node_id;
  41                 status[i] = 0xd0; /* simulate garbage values */
  42         }
  43
  44         ret = move_pages(0, num_pages, pages, nodes, status, MPOL_MF_MOVE);
  45         printf("move_pages: %ld\n", ret);
  46         for (int i = 0; i < num_pages; ++i)
  47                 printf("status[%d] = %d\n", i, status[i]);
  48   }
  49
  50 Then running the program would return nonsense status values:
  51
  52   $ ./move_pages_bug
  53   move_pages: 0
  54   status[0] = 208
  55   status[1] = 208
  56   status[2] = 208
  57   status[3] = 208
  58   status[4] = 208
  59   status[5] = 208
  60   status[6] = 208
  61   status[7] = 208
  62
  63 This is because the status is not set if the page is already on the
  64 target node, but move_pages() should return valid status as long as it
  65 succeeds.  The valid status may be errno or node id.
  66
  67 We can't simply initialize status array to zero since the pages may be
  68 not on node 0.  Fix it by updating status with node id which the page is
  69 already on.
  70
  71 Link: http://lkml.kernel.org/r/1575584353-125392-1-git-send-email-yang.shi@linux.alibaba.com
  72 Fixes: a49bd4d71637 ("mm, numa: rework do_pages_move")
  73 Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
  74 Reported-by: Felix Abecassis <fabecassis@nvidia.com>
  75 Tested-by: Felix Abecassis <fabecassis@nvidia.com>
  76 Suggested-by: Michal Hocko <mhocko@suse.com>
  77 Reviewed-by: John Hubbard <jhubbard@nvidia.com>
  78 Acked-by: Christoph Lameter <cl@linux.com>
  79 Acked-by: Michal Hocko <mhocko@suse.com>
  80 Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
  81 Cc: Mel Gorman <mgorman@techsingularity.net>
  82 Cc: <stable@vger.kernel.org>    [4.17+]
  83 Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
  84 Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  85 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  86
  87 ---
  88  mm/migrate.c |   23 +++++++++++++++++------
  89  1 file changed, 17 insertions(+), 6 deletions(-)
  90
  91 --- a/mm/migrate.c
  92 +++ b/mm/migrate.c
  93 @@ -1508,9 +1508,11 @@ static int do_move_pages_to_node(struct
  94  /*
  95   * Resolves the given address to a struct page, isolates it from the LRU and
  96   * puts it to the given pagelist.
  97 - * Returns -errno if the page cannot be found/isolated or 0 when it has been
  98 - * queued or the page doesn't need to be migrated because it is already on
  99 - * the target node
 100 + * Returns:
 101 + *     errno - if the page cannot be found/isolated
 102 + *     0 - when it doesn't have to be migrated because it is already on the
 103 + *         target node
 104 + *     1 - when it has been queued
 105   */
 106  static int add_page_for_migration(struct mm_struct *mm, unsigned long addr,
 107                 int node, struct list_head *pagelist, bool migrate_all)
 108 @@ -1549,7 +1551,7 @@ static int add_page_for_migration(struct
 109         if (PageHuge(page)) {
 110                 if (PageHead(page)) {
 111                         isolate_huge_page(page, pagelist);
 112 -                       err = 0;
 113 +                       err = 1;
 114                 }
 115         } else {
 116                 struct page *head;
 117 @@ -1559,7 +1561,7 @@ static int add_page_for_migration(struct
 118                 if (err)
 119                         goto out_putpage;
 120
 121 -               err = 0;
 122 +               err = 1;
 123                 list_add_tail(&head->lru, pagelist);
 124                 mod_node_page_state(page_pgdat(head),
 125                         NR_ISOLATED_ANON + page_is_file_cache(head),
 126 @@ -1636,8 +1638,17 @@ static int do_pages_move(struct mm_struc
 127                  */
 128                 err = add_page_for_migration(mm, addr, current_node,
 129                                 &pagelist, flags & MPOL_MF_MOVE_ALL);
 130 -               if (!err)
 131 +
 132 +               if (!err) {
 133 +                       /* The page is already on the target node */
 134 +                       err = store_status(status, i, current_node, 1);
 135 +                       if (err)
 136 +                               goto out_flush;
 137                         continue;
 138 +               } else if (err > 0) {
 139 +                       /* The page is successfully queued for migration */
 140 +                       continue;
 141 +               }
 142
 143                 err = store_status(status, i, err, 1);
 144                 if (err)