linux-kernel - Re: [v2 PATCH 2/2] mm: mempolicy: handle vma with unmovable pages mapped correctly in mbind

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:   Wed, 17 Jul 2019 11:23:16 -0700
From:   Yang Shi <yang.shi@...ux.alibaba.com>
To:     Vlastimil Babka <vbabka@...e.cz>, mhocko@...nel.org,
        mgorman@...hsingularity.net, akpm@...ux-foundation.org
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [v2 PATCH 2/2] mm: mempolicy: handle vma with unmovable pages
 mapped correctly in mbind



On 7/16/19 10:28 AM, Yang Shi wrote:
>
>
> On 7/16/19 5:07 AM, Vlastimil Babka wrote:
>> On 6/22/19 2:20 AM, Yang Shi wrote:
>>> @@ -969,10 +975,21 @@ static long do_get_mempolicy(int *policy, 
>>> nodemask_t *nmask,
>>>   /*
>>>    * page migration, thp tail pages can be passed.
>>>    */
>>> -static void migrate_page_add(struct page *page, struct list_head 
>>> *pagelist,
>>> +static int migrate_page_add(struct page *page, struct list_head 
>>> *pagelist,
>>>                   unsigned long flags)
>>>   {
>>>       struct page *head = compound_head(page);
>>> +
>>> +    /*
>>> +     * Non-movable page may reach here.  And, there may be
>>> +     * temporaty off LRU pages or non-LRU movable pages.
>>> +     * Treat them as unmovable pages since they can't be
>>> +     * isolated, so they can't be moved at the moment.  It
>>> +     * should return -EIO for this case too.
>>> +     */
>>> +    if (!PageLRU(head) && (flags & MPOL_MF_STRICT))
>>> +        return -EIO;
>>> +
>> Hm but !PageLRU() is not the only way why queueing for migration can
>> fail, as can be seen from the rest of the function. Shouldn't all cases
>> be reported?
>
> Do you mean the shared pages and isolation failed pages? I'm not sure 
> whether we should consider these cases break the semantics or not, so 
> I leave them as they are. But, strictly speaking they should be 
> reported too, at least for the isolation failed page.

By reading mbind man page, it says:

If MPOL_MF_MOVE is specified in flags, then the kernel will attempt to 
move all the existing pages in the memory range so that they follow the 
policy.  Pages that are shared with other processes will not be moved.  
If MPOL_MF_STRICT is also specified, then the call fails with the error 
EIO if some pages could not be moved.

It looks the code already handles shared page correctly, we just need 
return -EIO for isolation failed page if MPOL_MF_STRICT is specified.

>
> Thanks,
> Yang
>
>>
>>>       /*
>>>        * Avoid migrating a page that is shared with others.
>>>        */
>>> @@ -984,6 +1001,8 @@ static void migrate_page_add(struct page *page, 
>>> struct list_head *pagelist,
>>>                   hpage_nr_pages(head));
>>>           }
>>>       }
>>> +
>>> +    return 0;
>>>   }
>>>     /* page allocation callback for NUMA node migration */
>>> @@ -1186,9 +1205,10 @@ static struct page *new_page(struct page 
>>> *page, unsigned long start)
>>>   }
>>>   #else
>>>   -static void migrate_page_add(struct page *page, struct list_head 
>>> *pagelist,
>>> +static int migrate_page_add(struct page *page, struct list_head 
>>> *pagelist,
>>>                   unsigned long flags)
>>>   {
>>> +    return -EIO;
>>>   }
>>>     int do_migrate_pages(struct mm_struct *mm, const nodemask_t *from,
>>>
>