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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 Nov 2019 17:52:58 +0530
From:   "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>
To:     Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
        Michal Hocko <mhocko@...nel.org>
Cc:     Oscar Salvador <osalvador@...e.de>,
        "mike.kravetz\@oracle.com" <mike.kravetz@...cle.com>,
        "linux-mm\@kvack.org" <linux-mm@...ck.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v2 01/16] mm,hwpoison: cleanup unused PageHuge() check

Naoya Horiguchi <n-horiguchi@...jp.nec.com> 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@...jp.nec.com>
>> > 
>> > Drop the PageHuge check since memory_failure forks into memory_failure_hugetlb()
>> > for hugetlb pages.
>> > 
>> > Signed-off-by: Oscar Salvador <osalvador@...e.de>
>> > Signed-off-by: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
>> 
>> 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