From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 11DA9C43603 for ; Wed, 4 Dec 2019 23:38:01 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B4C3C206DB for ; Wed, 4 Dec 2019 23:38:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="HYRE7hbP" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B4C3C206DB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 622A66B0D26; Wed, 4 Dec 2019 18:38:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5D2F56B0D27; Wed, 4 Dec 2019 18:38:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4E8C46B0D28; Wed, 4 Dec 2019 18:38:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0007.hostedemail.com [216.40.44.7]) by kanga.kvack.org (Postfix) with ESMTP id 36B736B0D26 for ; Wed, 4 Dec 2019 18:38:00 -0500 (EST) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with SMTP id BD71F8248047 for ; Wed, 4 Dec 2019 23:37:59 +0000 (UTC) X-FDA: 76229074278.08.wren57_62d6bf762c11d X-HE-Tag: wren57_62d6bf762c11d X-Filterd-Recvd-Size: 7886 Received: from mail-ed1-f66.google.com (mail-ed1-f66.google.com [209.85.208.66]) by imf42.hostedemail.com (Postfix) with ESMTP for ; Wed, 4 Dec 2019 23:37:59 +0000 (UTC) Received: by mail-ed1-f66.google.com with SMTP id m8so948098edi.13 for ; Wed, 04 Dec 2019 15:37:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=tVzjA87BTmXSECOnGLTsMOKq26H0O/GbpMxRvsfC0gg=; b=HYRE7hbP30dwJLJUDbFysTSAgD/9Kk6a71QT47EDTpn+oS4OXFhO+oQDP/BadePznl nT9Pk9iCrUca5tFsks96jhXMMBGEHuWaQ45yaEFH9Nn4d9CZZ/BlXkXYq77h9gSeK0Qv njsid8P3mdCz2yG14Y+Nu4IQt0cwZ/5ef8qF9wOuK76c7O2Jfw0ZiZQHxHUFwvbhIJBm YgpmCrIx5Hpp+c4uP1WJ/Cxr3DoHTdIc3EYdBLXrxP5kec6QE8TU0yvJPlRCRPDw5t3s WCSknxhiI6zglXoRjmslwa24SIboWYBQA8KpyMtwq14TNtiGoSWLpyaJh3leDkpSpolf NWrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=tVzjA87BTmXSECOnGLTsMOKq26H0O/GbpMxRvsfC0gg=; b=Kn8fvRP3HqvuV9rwFN2RMIvJLkvvXZRurRBgNlOCckjNOAiCpeWasnZAK6ttsAj1nF KyAeWQvuOqlOgIV/aYrASD6OtxePKVkCRFh5opCTIgB8I3W0ZrLoSndrK9uY7Ed1SrU8 vK/voaOCDBA24SWPQIoMaTWFrMFtl/6jPB3aabqvNErWilZ5XxmSygmRJUiQB6uS24sK +S4tlxox7QYQDYDP64UXE+z42Ot85fh79MZzZOx8dXp7XFNz/khBu4DAtptS7oFLXKR6 HK5U+XuqKw5Pw5+Mlhsn/hgL7wpH1T4GDIVU3stlkDdovD4cRuYWEb/fMIl15WW8BoDW QAyw== X-Gm-Message-State: APjAAAWuMBdh9+cVTUvCvLwdMdgdBv9BaVITfqe/FmBKXj8JCzcn3wQY Jkq/vVMoo9pSuyqixZwlMYCOVHQvexvU24e6tII= X-Google-Smtp-Source: APXvYqxTVfonIp70cHYNrehkysyhNkCWjR6+P0ecIDclBvK/WkEAo04c0Y4HxIhCvENJWZRrQKHED8nxo3wYk04q3HQ= X-Received: by 2002:a50:dacd:: with SMTP id s13mr7408301edj.194.1575502677773; Wed, 04 Dec 2019 15:37:57 -0800 (PST) MIME-Version: 1.0 References: <20191204225413.GA625719@nvidia.com> In-Reply-To: <20191204225413.GA625719@nvidia.com> From: Yang Shi Date: Wed, 4 Dec 2019 15:37:47 -0800 Message-ID: Subject: Re: bug: move_pages(2) does not udpate "status" if no pages are moved To: Felix Abecassis Cc: Linux MM , Andrew Morton Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Dec 4, 2019 at 2:54 PM Felix Abecassis wrote: > > On Wed, Dec 04, 2019 at 12:17:59PM -0800, Yang Shi wrote: > > On Wed, Dec 4, 2019 at 11:01 AM Felix Abecassis wrote: > > > > > > 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; > > > > May you please give the below patch a try? I just did build test. > > > > diff --git a/mm/migrate.c b/mm/migrate.c > > index a8f87cb..f2f1279 100644 > > --- a/mm/migrate.c > > +++ b/mm/migrate.c > > @@ -1517,7 +1517,8 @@ static int do_move_pages_to_node(struct mm_struct *mm, > > * the target node > > */ > > static int add_page_for_migration(struct mm_struct *mm, unsigned long addr, > > - int node, struct list_head *pagelist, bool migrate_all) > > + int node, struct list_head *pagelist, bool migrate_all, > > + int __user *status, int start) > > { > > struct vm_area_struct *vma; > > struct page *page; > > @@ -1543,8 +1544,10 @@ static int add_page_for_migration(struct > > mm_struct *mm, unsigned long addr, > > goto out; > > > > err = 0; > > - if (page_to_nid(page) == node) > > + if (page_to_nid(page) == node) { > > + err = store_status(status, start, node, 1); > > goto out_putpage; > > + } > > > > err = -EACCES; > > if (page_mapcount(page) > 1 && !migrate_all) > > @@ -1639,7 +1642,9 @@ static int do_pages_move(struct mm_struct *mm, > > nodemask_t task_nodes, > > * report them via status > > */ > > err = add_page_for_migration(mm, addr, current_node, > > - &pagelist, flags & MPOL_MF_MOVE_ALL); > > + &pagelist, flags & MPOL_MF_MOVE_ALL, status, > > + i); > > + > > if (!err) > > continue; > > > > Thanks! I have tested this patch on top of 5.4-rc8, and I can comfirm it fixes > this particular test case. Thanks for confirming this. I will send out formal patch soon. > > > > > > > Here is a sample C program to reproduce the problem: > > > /* $ gcc move_pages_bug.c -lnuma -o move_pages_bug */ > > > > > > #define _DEFAULT_SOURCE > > > #include > > > > > > #include > > > #include > > > #include > > > #include > > > > > > 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, > > >