linux-kernel - Re: [PATCH] mm:mempolicy: skip VM_HUGETLB and VM

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]

Date:	Tue, 19 Jan 2016 12:00:51 +0900
From:	SeongJae Park <sj38.park@...il.com>
To:	Gavin Guo <gavin.guo@...onical.com>
Cc:	Liang Chen <liangchen.linux@...il.com>, n-horiguchi@...jp.nec.com,
	linux-mm@...ck.org, Rik van Riel <riel@...hat.com>,
	Mel Gorman <mgorman@...e.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm:mempolicy: skip VM_HUGETLB and VM_MIXEDMAP VMA for
 lazy mbind

Hi Gavin,


On Tue, Jan 19, 2016 at 11:43 AM, Gavin Guo <gavin.guo@...onical.com> wrote:
> Hi SeongJae,
>
> On Tue, Jan 19, 2016 at 9:12 AM, SeongJae Park <sj38.park@...il.com> wrote:
>> Hello Liang,
>>
>> Just trivial comment below.
>>
>> On Mon, 18 Jan 2016, Liang Chen wrote:
>>
>>> VM_HUGETLB and VM_MIXEDMAP vma needs to be excluded to avoid compound
>>> pages being marked for migration and unexpected COWs when handling
>>> hugetlb fault.
>>>
>>> Thanks to Naoya Horiguchi for reminding me on these checks.
>>>
>>> Signed-off-by: Liang Chen <liangchen.linux@...il.com>
>>> Signed-off-by: Gavin Guo <gavin.guo@...onical.com>
>>> ---
>>> mm/mempolicy.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>>> index 436ff411..415de70 100644
>>> --- a/mm/mempolicy.c
>>> +++ b/mm/mempolicy.c
>>> @@ -610,8 +610,9 @@ static int queue_pages_test_walk(unsigned long start,
>>> unsigned long end,
>>>
>>>         if (flags & MPOL_MF_LAZY) {
>>>                 /* Similar to task_numa_work, skip inaccessible VMAs */
>>> -               if (vma_migratable(vma) &&
>>> -                       vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE))
>>> +               if (vma_migratable(vma) && !is_vm_hugetlb_page(vma) &&
>>> +                       (vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE))
>>> &&
>>> +                       !(vma->vm_flags & VM_MIXEDMAP))
>>
>>
>> Isn't there exists few unnecessary parenthesis? IMHO, it makes me hard to
>> read the code.
>>
>> How about below code, instead?
>>
>> +             if (vma_migratable(vma) && !is_vm_hugetlb_page(vma) &&
>> +                     vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE) &&
>
> Thanks for your suggestion, it's good for the above. However, it should be
> a typo for the following and I think you mean:
>
>         ~vma->vm_flags & VM_MIXEDMAP
>
> Even though the result is correct, I feel it's a bit of ambiguous for
> people to understand and away from it's original meaning.

Ah, you're right. That's my fault. Thanks for noting that.

BTW, now I think the line could be expressed in this way:
         vma->vm_flags & ~VM_MIXEDMAP

I feel this is sufficiently explicit and follows the meaning well.
However, I agree that Liang's first one is good enough, too.

Thanks,
SeongJae Park.

>
>> +                     !vma->vm_flags & VM_MIXEDMAP)
>>
>>
>> Thanks,
>> SeongJae Park.
>>
>>>                         change_prot_numa(vma, start, endvma);
>>>                 return 1;
>>>         }
>>> --
>>> 1.9.1
>>>
>>> --
>>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>>> the body to majordomo@...ck.org.  For more info on Linux MM,
>>> see: http://www.linux-mm.org/ .
>>> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>
>>>
>>