Re: [RFC PATCH v2 01/16] mm,hwpoison: cleanup unused PageHuge() check

[Aneesh Kumar K.V]

Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx> writes:

> On Fri, Oct 18, 2019 at 01:48:32PM +0200, Michal Hocko wrote:
>> On Thu 17-10-19 16:21:08, Oscar Salvador wrote:
>> > From: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
>> > 
>> > Drop the PageHuge check since memory_failure forks into memory_failure_hugetlb()
>> > for hugetlb pages.
>> > 
>> > Signed-off-by: Oscar Salvador <osalvador@xxxxxxx>
>> > Signed-off-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
>> 
>> s-o-b chain is reversed.
>> 
>> The code is a bit confusing. Doesn't this check aim for THP?
>
> No, PageHuge() is false for thp, so this if branch is just dead code.

memory_failure()
{

	if (PageTransHuge(hpage)) {
		lock_page(p);
		if (!PageAnon(p) || unlikely(split_huge_page(p))) {
			unlock_page(p);
			if (!PageAnon(p))
				pr_err("Memory failure: %#lx: non anonymous thp\n",
					pfn);
			else
				pr_err("Memory failure: %#lx: thp split failed\n",
					pfn);
			if (TestClearPageHWPoison(p))
				num_poisoned_pages_dec();
			put_hwpoison_page(p);
			return -EBUSY;
		}
		unlock_page(p);
		VM_BUG_ON_PAGE(!page_count(p), p);
		hpage = compound_head(p);
	}

}

Do we need that hpage = compund_head(p) conversion there? We should just
be able to say hpage = p, or even better after this change use p
directly instead of hpage in the code following?

-aneesh