lkml.org 
[lkml]   [2022]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/8] mm/memory-failure.c: remove unneeded orig_head
    On Thu, Feb 10, 2022 at 10:17:29PM +0800, Miaohe Lin wrote:
    > orig_head is used to check whether the page have changed compound pages
    > during the locking. But it's always equal to hpage. So we can use hpage
    > directly and remove this redundant one.
    >
    > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
    > ---
    > mm/memory-failure.c | 5 ++---
    > 1 file changed, 2 insertions(+), 3 deletions(-)
    >
    > diff --git a/mm/memory-failure.c b/mm/memory-failure.c
    > index 2dd7f35ee65a..4370c2f407c5 100644
    > --- a/mm/memory-failure.c
    > +++ b/mm/memory-failure.c
    > @@ -1691,7 +1691,6 @@ int memory_failure(unsigned long pfn, int flags)
    > {
    > struct page *p;
    > struct page *hpage;
    > - struct page *orig_head;
    > struct dev_pagemap *pgmap;
    > int res = 0;
    > unsigned long page_flags;
    > @@ -1737,7 +1736,7 @@ int memory_failure(unsigned long pfn, int flags)
    > goto unlock_mutex;
    > }
    >
    > - orig_head = hpage = compound_head(p);
    > + hpage = compound_head(p);
    > num_poisoned_pages_inc();
    >
    > /*
    > @@ -1821,7 +1820,7 @@ int memory_failure(unsigned long pfn, int flags)
    > * The page could have changed compound pages during the locking.
    > * If this happens just bail out.
    > */
    > - if (PageCompound(p) && compound_head(p) != orig_head) {
    > + if (PageCompound(p) && compound_head(p) != hpage) {

    I think that this if-check was intended to detect the case that page p
    belongs to a thp when memory_failure() is called and belongs to a compound
    page in different size (like slab or some driver page) after the thp is
    split. But your suggestion makes me aware that the page p could be embedded
    on a thp again after thp split. I think this might be rare, but if it
    happens the current if-check (or suggested one) cannot detect it.
    So I feel that simply dropping compound_head() check might be better?

    - if (PageCompound(p) && compound_head(p) != orig_head) {
    + if (PageCompound(p)) {

    This should ensure the assumption (mentioned in 8/8 patch) more that
    the error page never belongs to compound page after taking page lock.

    Thanks,
    Naoya Horiguchi

    \
     
     \ /
      Last update: 2022-02-14 15:50    [W:2.513 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site