Messages in this thread Patch in this message | | | Subject | Re: [v3 PATCH 2/2] mm: thp: fix false negative of shmem vma's THP eligibility | From | Yang Shi <> | Date | Wed, 19 Jun 2019 09:28:42 -0700 |
| |
On 6/19/19 5:12 AM, Vlastimil Babka wrote: > On 6/13/19 6:44 AM, Yang Shi wrote: >> The commit 7635d9cbe832 ("mm, thp, proc: report THP eligibility for each >> vma") introduced THPeligible bit for processes' smaps. But, when checking >> the eligibility for shmem vma, __transparent_hugepage_enabled() is >> called to override the result from shmem_huge_enabled(). It may result >> in the anonymous vma's THP flag override shmem's. For example, running a >> simple test which create THP for shmem, but with anonymous THP disabled, >> when reading the process's smaps, it may show: > ... > >> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c >> index 01d4eb0..6a13882 100644 >> --- a/fs/proc/task_mmu.c >> +++ b/fs/proc/task_mmu.c >> @@ -796,7 +796,8 @@ static int show_smap(struct seq_file *m, void *v) >> >> __show_smap(m, &mss); >> >> - seq_printf(m, "THPeligible: %d\n", transparent_hugepage_enabled(vma)); >> + seq_printf(m, "THPeligible: %d\n", >> + transparent_hugepage_enabled(vma)); >> >> if (arch_pkeys_enabled()) >> seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma)); >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >> index 4bc2552..36f0225 100644 >> --- a/mm/huge_memory.c >> +++ b/mm/huge_memory.c >> @@ -65,10 +65,15 @@ >> >> bool transparent_hugepage_enabled(struct vm_area_struct *vma) >> { >> + /* The addr is used to check if the vma size fits */ >> + unsigned long addr = (vma->vm_end & HPAGE_PMD_MASK) - HPAGE_PMD_SIZE; >> + >> + if (!transhuge_vma_suitable(vma, addr)) >> + return false; > Sorry for replying rather late, and not in the v2 thread, but unlike > Hugh I'm not convinced that we should include vma size/alignment in the > test for reporting THPeligible, which was supposed to reflect > administrative settings and madvise hints. I guess it's mostly a matter > of personal feeling. But one objective distinction is that the admin > settings and madvise do have an exact binary result for the whole VMA, > while this check is more fuzzy - only part of the VMA's span might be > properly sized+aligned, and THPeligible will be 1 for the whole VMA.
I think THPeligible is used to tell us if the vma is suitable for allocating THP. Both anonymous and shmem THP checks vma size/alignment to decide to or not to allocate THP.
And, if vma size/alignment is not checked, THPeligible may show "true" for even 4K mapping. This doesn't make too much sense either.
> >> if (vma_is_anonymous(vma)) >> return __transparent_hugepage_enabled(vma); >> - if (vma_is_shmem(vma) && shmem_huge_enabled(vma)) >> - return __transparent_hugepage_enabled(vma); >> + if (vma_is_shmem(vma)) >> + return shmem_huge_enabled(vma); >> >> return false; >> } >> diff --git a/mm/shmem.c b/mm/shmem.c >> index 1bb3b8d..a807712 100644 >> --- a/mm/shmem.c >> +++ b/mm/shmem.c >> @@ -3872,6 +3872,9 @@ bool shmem_huge_enabled(struct vm_area_struct *vma) >> loff_t i_size; >> pgoff_t off; >> >> + if ((vma->vm_flags & VM_NOHUGEPAGE) || >> + test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags)) >> + return false; >> if (shmem_huge == SHMEM_HUGE_FORCE) >> return true; >> if (shmem_huge == SHMEM_HUGE_DENY) >>
| |