From: Michal Hocko <mhocko@suse.com>
To: Piotr Jaroszynski <pjaroszynski@nvidia.com>
Cc: p.jaroszynski@gmail.com, linux-mm@kvack.org,
Jan Stancek <jstancek@redhat.com>, Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH] Fix do_move_pages_to_node() error handling
Date: Fri, 16 Nov 2018 12:49:55 +0100
Message-ID: <20181116114955.GJ14706@dhcp22.suse.cz> (raw)
In-Reply-To: <22b8c91d-1c65-eba2-214e-0696d0e771fb@nvidia.com>
On Thu 15-11-18 10:58:33, Piotr Jaroszynski wrote:
> On 11/15/18 12:47 AM, Michal Hocko wrote:
> > On Wed 14-11-18 17:04:37, Piotr Jaroszynski wrote:
[...]
> >> The proposed solution adds a new case to handle, but it will just tell
> >> us the page status is unusable and all we can do is just retry blindly.
> >> If it was possible to plumb through the migration status for each page
> >> accurately that would allow us to save redoing the call for pages that
> >> actually worked. Perhaps we would need a special status for pages
> >> skipped due to errors.
> >
> > This would be possible but with this patch applied you should know how
> > many pages to skip from the tail of the array.
>
> At least in our case the node target is the same for all the pages so we
> would just learn that all the pages failed to migrate as they would be
> all batched together to the do_move_pages_to_node() call.
Anyway, could you give this patch a try please? I would appreciate some
Tested-bys to push this forward ;)
> >> But maybe this is all a tiny corner case short of the bug I hit (see
> >> more below) and it's not worth thinking too much about.
> >>
> >>>>>> Just wondering, how have you found out? Is there any real application
> >>>>>> failing because of the change or this is a result of some test?
> >>>>
> >>>> I have a test that creates a tmp file, mmaps it as shared, memsets the
> >>>> memory and then attempts to move it to a different node. It used to
> >>>> work, but now fails. I suspect the filesystem's migratepage() callback
> >>>> regressed and will look into it next. So far I have only tested this on
> >>>> powerpc with the xfs filesystem.
> >>>
> >>> I would be surprise if the rewor changed the migration behavior.
> >>
> >> It didn't, I tracked it down to the new fs/iomap.c code used by xfs not
> >> being compatible with migrate_page_move_mapping() and prepared a perhaps
> >> naive fix in [1].
> >
> > I am not familiar with iomap code much TBH so I cannot really judge your
> > fix.
> >
>
> Christoph reviewed it already (thanks!) so it should be good after all.
> But in its context, I wanted to ask about migrate_page_move_mapping()
> page count checks that it was hitting. Is it true that the count checks
> are to handle the case when a page might be temporarily pinned and hence
> have the count too high temporarily?
Yes. We cannot really migrate pinned pages.
> That would explain why it returns
> EAGAIN in this case. But should having the count too low (what the bug
> was hitting) be a fatal error with a WARN maybe? Or are there expected
> cases where the count is too low temporarily too?
Nope, page reference count too low is a bug.
> I could send a patch
> for that, but also just wanted to understand the expectations.
--
Michal Hocko
SUSE Labs
prev parent reply index
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-14 0:40 p.jaroszynski
2018-11-14 7:34 ` Michal Hocko
2018-11-14 11:29 ` Michal Hocko
2018-11-14 18:04 ` Piotr Jaroszynski
2018-11-14 21:22 ` Michal Hocko
2018-11-15 1:04 ` Piotr Jaroszynski
2018-11-15 8:47 ` Michal Hocko
2018-11-15 18:58 ` Piotr Jaroszynski
2018-11-16 11:49 ` Michal Hocko [this message]
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=20181116114955.GJ14706@dhcp22.suse.cz \
--to=mhocko@suse.com \
--cc=hch@lst.de \
--cc=jstancek@redhat.com \
--cc=linux-mm@kvack.org \
--cc=p.jaroszynski@gmail.com \
--cc=pjaroszynski@nvidia.com \
/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
Linux-mm Archive on lore.kernel.org
Archives are clonable:
git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
linux-mm@kvack.org
public-inbox-index linux-mm
Example config snippet for mirrors
Newsgroup available over NNTP:
nntp://nntp.lore.kernel.org/org.kvack.linux-mm
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git