On 01/10/2015, 06:01 AM, Hugh Dickins wrote:
> On Fri, 9 Jan 2015, Jiri Slaby wrote:
> 
>> From: Hugh Dickins <hughd@google.com>
>>
>> 3.12-stable review patch.  If anyone has any objections, please let me know.
>>
>> ===============
>>
>> commit f72e7dcdd25229446b102e587ef2f826f76bff28 upstream.
...
> Fine for this to go in, but there is one catch, which I discovered when
> backporting to v3.11: it needed one more hunk.  I haven't checked your
> base tree, but if this applies then I believe you need it - most of the
> time no problem, but it can case page migration to fail to find a
> migration entry it inserted earlier, then BUG_ON(!PageLocked(p)) in
> migration_entry_to_page() soon after.  Here's what I wrote back then:
> 
> Note on rebase to v3.11: added a hunk to replace the use of mm_find_pmd()
> in page_check_address_pmd().  This call had been similarly replaced by
> the time of my v3.16 commit, in Kirill Shutemov's v3.15 b5a8cad376ee
> ("thp: close race between split and zap huge pages"): which we do not
> need as such, since it's fixing v3.13 117b0791ac42 ("mm, thp: move ptl
> taking inside page_check_address_pmd()"), from a split page-table-lock
> series we are not backporting.  But without this additional hunk, rmap
> sometimes broke when the new semantic for mm_find_pmd() was used here.
> 
> (Adding Kirill to Cc: shouldn't he have been Cc'ed already?)
> 
> Hugh

Thanks, I see. So the diff between the hunk below and 117b0791ac42 are
two things:

> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1584,12 +1584,20 @@ pmd_t *page_check_address_pmd(struct page *page,
>  			      unsigned long address,
>  			      enum page_check_address_pmd_flag flag)
>  {
> +	pgd_t *pgd;
> +	pud_t *pud;
>  	pmd_t *pmd, *ret = NULL;
>  
>  	if (address & ~HPAGE_PMD_MASK)
>  		goto out;
>  
> -	pmd = mm_find_pmd(mm, address);
> +	pgd = pgd_offset(mm, address);
> +	if (!pgd_present(*pgd))
> +		goto out;
> +	pud = pud_offset(pgd, address);
> +	if (!pud_present(*pud))
> +		goto out;
> +	pmd = pmd_offset(pud, address);
>  	if (!pmd)
>  		goto out;

This check is removed by 117b0791ac42. Can actually pmd returned from
pmd_offset be NULL?

>  	if (pmd_none(*pmd))

pmd_none() is replaced by !pmd_present().

My question is: is it OK to take the backport of 117b0791ac42 attached
(to stay with what upstream has)?

thanks,
-- 
js
suse labs