| Message ID | 20220611022314.21593-1-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/shmem.c: use helper transhuge_vma_enabled() | expand |
On Sat, 11 Jun 2022, Miaohe Lin wrote: > Use helper transhuge_vma_enabled() to check whether transhuge is enable > on vma. Minor readability improvement. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> No thanks, that's a readability regression, forcing reader to go and look up what transhuge_vma_enabled() actually means. What you call a helper, I call an obfuscator - as I implied in b9e2faaf6fa0 ("huge tmpfs: revert shmem's use of transhuge_vma_enabled()") Hugh > --- > mm/shmem.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/mm/shmem.c b/mm/shmem.c > index 133c67057d41..59cc2e980c95 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -480,8 +480,7 @@ bool shmem_is_huge(struct vm_area_struct *vma, > return false; > if (shmem_huge == SHMEM_HUGE_DENY) > return false; > - if (vma && ((vma->vm_flags & VM_NOHUGEPAGE) || > - test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))) > + if (vma && !transhuge_vma_enabled(vma, vma->vm_flags)) > return false; > if (shmem_huge == SHMEM_HUGE_FORCE) > return true; > -- > 2.23.0
On 2022/6/11 11:14, Hugh Dickins wrote: > On Sat, 11 Jun 2022, Miaohe Lin wrote: > >> Use helper transhuge_vma_enabled() to check whether transhuge is enable >> on vma. Minor readability improvement. >> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > > No thanks, that's a readability regression, forcing reader > to go and look up what transhuge_vma_enabled() actually means. > > What you call a helper, I call an obfuscator - as I implied in > b9e2faaf6fa0 ("huge tmpfs: revert shmem's use of transhuge_vma_enabled()") Oh, that commit also cc to me. But I forgot it... Sorry. > > Hugh >
On 6/11/22 08:44, Hugh Dickins wrote: > On Sat, 11 Jun 2022, Miaohe Lin wrote: > >> Use helper transhuge_vma_enabled() to check whether transhuge is enable >> on vma. Minor readability improvement. >> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > > No thanks, that's a readability regression, forcing reader > to go and look up what transhuge_vma_enabled() actually means. > > What you call a helper, I call an obfuscator - as I implied in > b9e2faaf6fa0 ("huge tmpfs: revert shmem's use of transhuge_vma_enabled()") The same reasoning should also be applicable for other calls sites for transhuge_vma_enabled(). Should not they be dropped as well ? > > Hugh > >> --- >> mm/shmem.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/mm/shmem.c b/mm/shmem.c >> index 133c67057d41..59cc2e980c95 100644 >> --- a/mm/shmem.c >> +++ b/mm/shmem.c >> @@ -480,8 +480,7 @@ bool shmem_is_huge(struct vm_area_struct *vma, >> return false; >> if (shmem_huge == SHMEM_HUGE_DENY) >> return false; >> - if (vma && ((vma->vm_flags & VM_NOHUGEPAGE) || >> - test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))) >> + if (vma && !transhuge_vma_enabled(vma, vma->vm_flags)) >> return false; >> if (shmem_huge == SHMEM_HUGE_FORCE) >> return true; >> -- >> 2.23.0 >
On Tue, Jun 14, 2022 at 10:02 PM Anshuman Khandual <anshuman.khandual@arm.com> wrote: > > > > On 6/11/22 08:44, Hugh Dickins wrote: > > On Sat, 11 Jun 2022, Miaohe Lin wrote: > > > >> Use helper transhuge_vma_enabled() to check whether transhuge is enable > >> on vma. Minor readability improvement. > >> > >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > > > > No thanks, that's a readability regression, forcing reader > > to go and look up what transhuge_vma_enabled() actually means. > > > > What you call a helper, I call an obfuscator - as I implied in > > b9e2faaf6fa0 ("huge tmpfs: revert shmem's use of transhuge_vma_enabled()") > > The same reasoning should also be applicable for other calls sites > for transhuge_vma_enabled(). Should not they be dropped as well ? Yes, the only caller was hugepage_vma_check(). It was changed to open-code and the helper was removed by series: https://lore.kernel.org/linux-mm/20220615172926.546974-1-shy828301@gmail.com/T/#m7610c2a4e5dda753cbc850986d5486f59a821c1a > > > > > Hugh > > > >> --- > >> mm/shmem.c | 3 +-- > >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> diff --git a/mm/shmem.c b/mm/shmem.c > >> index 133c67057d41..59cc2e980c95 100644 > >> --- a/mm/shmem.c > >> +++ b/mm/shmem.c > >> @@ -480,8 +480,7 @@ bool shmem_is_huge(struct vm_area_struct *vma, > >> return false; > >> if (shmem_huge == SHMEM_HUGE_DENY) > >> return false; > >> - if (vma && ((vma->vm_flags & VM_NOHUGEPAGE) || > >> - test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))) > >> + if (vma && !transhuge_vma_enabled(vma, vma->vm_flags)) > >> return false; > >> if (shmem_huge == SHMEM_HUGE_FORCE) > >> return true; > >> -- > >> 2.23.0 > > >
diff --git a/mm/shmem.c b/mm/shmem.c index 133c67057d41..59cc2e980c95 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -480,8 +480,7 @@ bool shmem_is_huge(struct vm_area_struct *vma, return false; if (shmem_huge == SHMEM_HUGE_DENY) return false; - if (vma && ((vma->vm_flags & VM_NOHUGEPAGE) || - test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))) + if (vma && !transhuge_vma_enabled(vma, vma->vm_flags)) return false; if (shmem_huge == SHMEM_HUGE_FORCE) return true;
Use helper transhuge_vma_enabled() to check whether transhuge is enable on vma. Minor readability improvement. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/shmem.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)