On Tue, Sep 08, 2020 at 09:26:13AM +0200, Oscar Salvador wrote:
> On Tue, Sep 08, 2020 at 07:02:12AM +0000, HORIGUCHI NAOYA(堀口 直也) wrote:
> > On Mon, Sep 07, 2020 at 08:44:42PM -0700, yaoaili126@xxxxxxx wrote:
> > > From: Aili Yao <yaoaili@xxxxxxxxxxxx>
> > >
> > > PageHuge(p) branch will never be true,but for compound page we need to set page_flags to correct value.
> > >
> > > Reviewed-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > > Signed-off-by: Yang Feng < yangfeng1@xxxxxxxxxxxx>
> > > Signed-off-by: Aili Yao <yaoaili@xxxxxxxxxxxx>
> >
> > I found that this PageHuge() check is removed and no long exists
> > in the latest mmotm, so we don't have worry about it.
>
> I might be missing something, so bear with me.
>
> It is true that the PageHuge check is gone, but we are storing the page's
> flags in page_flags, even if the page is a tail (e.g: part of a compound page
> ).
> Should not we store heads' flags instead?
I think that saving ->flags of the raw error page is OK, because even in thp
case we focus on the raw page instead of its head page from memory error
perspective. It's not assumed that we can contain the error thp in the thp
unit and instead we always rely on thp split. (For hugetlb, we now contain
the errored hugetlb in hugetlb unit, so saving its head page might be OK.)
Theoretically, it could happen that a error could be collapsed into a new
thp just after passing over the following block:
1408 if (PageTransHuge(hpage)) {
1409 if (try_to_split_thp_page(p, "Memory Failure") < 0) {
1410 action_result(pfn, MF_MSG_UNSPLIT_THP, MF_IGNORED);
1411 return -EBUSY;
1412 }
1413 VM_BUG_ON_PAGE(!page_count(p), p);
1414 }
So I feel that some check might be added after holding page lock to avoid
that case. Or acutally, it might better that moving the above block into
page lock is more better for simpler code.
Thanks,
Naoya Horiguchi
> AFAICS, hpage contains either the head of the compound page,
> or the page itself in case it is a normal page.
>
> --
> Oscar Salvador
> SUSE L3
>
