linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: John Hubbard <jhubbard@nvidia.com>
To: Felix Abecassis <fabecassis@nvidia.com>, <linux-mm@kvack.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Subject: Re: bug: move_pages(2) does not udpate "status" if no pages are moved
Date: Wed, 4 Dec 2019 11:21:37 -0800	[thread overview]
Message-ID: <1a8ccccb-e429-45d3-3615-b3b8bf04c6fe@nvidia.com> (raw)
In-Reply-To: <f95aae1d-0328-7beb-2130-4f59ba38abf1@nvidia.com>

On 12/4/19 11:01 AM, Felix Abecassis wrote:
> Hello all,
> 

Hi Felix,

Thanks for writing up a very clear description of the problem.

> 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.


The way to detect this case would be to zero the array before calling move_pages().
Then, if move_pages() returns 0, and the array remains full of zeroes, you can
conclude that move_pages() "succeeded", and that there were no errors for any
of the pages. So the pages are where you requested them to end up.


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


I looked at that code and the surrounding function, and it's been pretty much
unchanged for quite a while. The above was last touched in April, 2018, for 
example.

Yes, we could change the kernel code to fill in the array with zeroes in that
situation, but the man page doesn't actually cover this case at all. We'd have
to also change the man page, to say something like, "if pages were not moved 
because they were already in the requested location, then the status array
will contain <SOME_VALUE> for such pages". In other words, the kernel matches
the requirements (the man page) as it stands today, at least as I'm reading it.

And given that one can already figure all this out with the existing kernel
and libnuma behavior, I'm guessing that the linux-mm folks will not see any
reason to make such a change--but maybe I'm guessing wrong. Anyone on CC want
to weigh in there?


thanks,
-- 
John Hubbard
NVIDIA


  reply	other threads:[~2019-12-04 19:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1a8ccccb-e429-45d3-3615-b3b8bf04c6fe@nvidia.com \
    --to=jhubbard@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=fabecassis@nvidia.com \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).