linux-kernel - Re: [PATCH] mm: migrate: fix an incorrect call of prep_transhuge

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20171122145307.52klaq4ouorngsss@dhcp22.suse.cz>
Date:   Wed, 22 Nov 2017 15:53:07 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Zi Yan <zi.yan@...rutgers.edu>
Cc:     Zi Yan <zi.yan@...t.com>,
        Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Andrea Reale <ar@...ux.vnet.ibm.com>,
        Jérôme Glisse <jglisse@...hat.com>,
        stable@...r.kernel.org
Subject: Re: [PATCH] mm: migrate: fix an incorrect call of
 prep_transhuge_page()

On Wed 22-11-17 09:43:46, Zi Yan wrote:
> 
> 
> Michal Hocko wrote:
> > On Wed 22-11-17 09:54:16, Michal Hocko wrote:
> >> On Mon 20-11-17 21:18:55, Zi Yan wrote:
> > [...]
> >>> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> >>> index 895ec0c4942e..a2246cf670ba 100644
> >>> --- a/include/linux/migrate.h
> >>> +++ b/include/linux/migrate.h
> >>> @@ -54,7 +54,7 @@ static inline struct page *new_page_nodemask(struct page *page,
> >>>  	new_page = __alloc_pages_nodemask(gfp_mask, order,
> >>>  				preferred_nid, nodemask);
> >>>  
> >>> -	if (new_page && PageTransHuge(page))
> >>> +	if (new_page && PageTransHuge(new_page))
> >>>  		prep_transhuge_page(new_page);
> >> I would keep the two checks consistent. But that leads to a more
> >> interesting question. new_page_nodemask does
> >>
> >> 	if (thp_migration_supported() && PageTransHuge(page)) {
> >> 		order = HPAGE_PMD_ORDER;
> >> 		gfp_mask |= GFP_TRANSHUGE;
> >> 	}
> > 
> > And one more question/note. Why do we need thp_migration_supported
> > in the first place? 9c670ea37947 ("mm: thp: introduce
> > CONFIG_ARCH_ENABLE_THP_MIGRATION") says
> > : Introduce CONFIG_ARCH_ENABLE_THP_MIGRATION to limit thp migration
> > : functionality to x86_64, which should be safer at the first step.
> > 
> > but why is unsafe to enable the feature on other arches which support
> > THP? Is there any plan to do the next step and remove this config
> > option?
> 
> Because different architectures have their own way of specifying a swap
> entry. This means, to support THP migration, each architecture needs to
> add its own __pmd_to_swp_entry() and __swp_entry_to_pmd(), which are
> used for arch-independent pmd_to_swp_entry() and swp_entry_to_pmd().

I understand that part. But this smells like a matter of coding, no?
I was suprised to see the note about safety which didn't make much sense
to me.
-- 
Michal Hocko
SUSE Labs