Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Wei Yang <richard.weiyang@gmail.com>
To: David Hildenbrand <david@redhat.com>
Cc: Wei Yang <richardw.yang@linux.intel.com>,
	n-horiguchi@ah.jp.nec.com, akpm@linux-foundation.org,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] mm/memory-failure.c: not necessary to recalculate hpage
Date: Mon, 2 Dec 2019 22:28:27 +0000
Message-ID: <20191202222827.isaelnqmuyn7zrns@master> (raw)
In-Reply-To: <fdba31c8-d0c0-83a8-62d1-c04c1e894218@redhat.com>

On Wed, Nov 20, 2019 at 04:07:38PM +0100, David Hildenbrand wrote:
>On 18.11.19 09:20, Wei Yang wrote:
>> hpage is not changed.
>> 
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>>   mm/memory-failure.c | 1 -
>>   1 file changed, 1 deletion(-)
>> 
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index 392ac277b17d..9784f4339ae7 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -1319,7 +1319,6 @@ int memory_failure(unsigned long pfn, int flags)
>>   		}
>>   		unlock_page(p);
>>   		VM_BUG_ON_PAGE(!page_count(p), p);
>> -		hpage = compound_head(p);
>>   	}
>>   	/*
>> 
>
>I am *absolutely* no transparent huge page expert (sorry :) ), but won't the
>split_huge_page(p) eventually split the compound page, such that
>compound_head(p) will return something else after that call?
>

Hi, David

Took sometime to look into the code and re-think about it. Found maybe we can
simplify this in another way.

First, code touches here means split_huge_page() succeeds and "p" is now a PTE
page. So compound_head(p) == p.

Then let's look at who will use hpage in the following function. There are two
uses in current upstream:

    * page_flags calculation
    * hwpoison_user_mappings()

The first one would be removed in next patch since PageHuge is handled at the
beginning.

And in the second place, comment says if split succeeds, hpage points to page
"p".

After all, we don't need to re-calculate hpage after split, and just replace
hpage in hwpoison_user_mappings() with p is enough.

>-- 
>
>Thanks,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me


  parent reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-18  8:20 [PATCH 1/2] mm/memory-failure.c: PageHuge is handled at the beginning of memory_failure Wei Yang
2019-11-18  8:20 ` [PATCH 2/2] mm/memory-failure.c: not necessary to recalculate hpage Wei Yang
2019-11-20 15:07   ` David Hildenbrand
2019-11-21  1:05     ` Wei Yang
2019-12-02 22:28     ` Wei Yang [this message]
2019-12-05 15:06       ` David Hildenbrand
2019-12-06  1:48         ` Wei Yang
2020-01-08 12:20           ` David Hildenbrand
2020-01-09  1:58             ` Wei Yang
2019-11-19 12:23 ` [PATCH 1/2] mm/memory-failure.c: PageHuge is handled at the beginning of memory_failure David Hildenbrand
2019-11-20  0:46   ` Wei Yang
2019-11-20 15:04     ` David Hildenbrand
2019-11-21  1:07       ` Wei Yang

Reply instructions:

You may reply publically 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=20191202222827.isaelnqmuyn7zrns@master \
    --to=richard.weiyang@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=richardw.yang@linux.intel.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