Re: [PATCH] mm:mempolicy: skip VM_HUGETLB and VM_MIXEDMAP VMA for lazy mbind

[Liang Chen <liangchen.linux@gmail.com>]

Thanks a lot for the checking and discussion. So I will just leave it
as it was:)

Thanks,
Liang

On Tue, Jan 19, 2016 at 11:00 AM, SeongJae Park <sj38.park@gmail.com> wrote:
> Hi Gavin,
>
>
> On Tue, Jan 19, 2016 at 11:43 AM, Gavin Guo <gavin.guo@canonical.com> wrote:
>> Hi SeongJae,
>>
>> On Tue, Jan 19, 2016 at 9:12 AM, SeongJae Park <sj38.park@gmail.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@gmail.com>
>>>> Signed-off-by: Gavin Guo <gavin.guo@canonical.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@kvack.org.  For more info on Linux MM,
>>>> see: http://www.linux-mm.org/ .
>>>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>>>>
>>>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>