| Message ID | b44e3619-712e-90af-89d2-e4ba654c5110@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tmpfs: HUGEPAGE and MEM_LOCK fcntls and memfds | expand |
On Fri, Jul 30, 2021 at 12:36 AM Hugh Dickins <hughd@google.com> wrote: > > 5.14 commit e6be37b2e7bd ("mm/huge_memory.c: add missing read-only THP > checking in transparent_hugepage_enabled()") added transhuge_vma_enabled() > as a wrapper for two very different checks: shmem_huge_enabled() prefers > to show those two checks explicitly, as before. Basically I have no objection to separating them again. But IMHO they seem not very different. Or just makes things easier for the following patches? > > Signed-off-by: Hugh Dickins <hughd@google.com> > --- > mm/shmem.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/shmem.c b/mm/shmem.c > index ce3ccaac54d6..c6fa6f4f2db8 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -4003,7 +4003,8 @@ bool shmem_huge_enabled(struct vm_area_struct *vma) > loff_t i_size; > pgoff_t off; > > - if (!transhuge_vma_enabled(vma, vma->vm_flags)) > + 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; > -- > 2.26.2 >
On Fri, 30 Jul 2021, Yang Shi wrote: > On Fri, Jul 30, 2021 at 12:36 AM Hugh Dickins <hughd@google.com> wrote: > > > > 5.14 commit e6be37b2e7bd ("mm/huge_memory.c: add missing read-only THP > > checking in transparent_hugepage_enabled()") added transhuge_vma_enabled() > > as a wrapper for two very different checks: shmem_huge_enabled() prefers > > to show those two checks explicitly, as before. > > Basically I have no objection to separating them again. But IMHO they > seem not very different. Or just makes things easier for the following > patches? Well, it made it easier to apply the patch I'd prepared earlier, but that was not the point; and I thought it best to be upfront about the reversion, rather than hiding it in the movement. The end result of the two checks is the same (don't try for huge pages), and they have been grouped together because they occurred together in several places, and both rely on "vma". But one check is whether the app has marked that address range not to use THPs; and the other check is whether the process is running in a hierarchy that has been marked never to use THPs (which just uses vma to get to mm to get to mm->flags (whether current->mm would be more relevant is not an argument I want to get into, I'm not at all sure)). To me those are very different; and I'm particularly concerned to make MMF_DISABLE_THP references visible, since it did not exist when Kirill and I first implemented shmem huge pages, and I've tended to forget it: but consider it more in this series. Hugh > > > > > Signed-off-by: Hugh Dickins <hughd@google.com> > > --- > > mm/shmem.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/mm/shmem.c b/mm/shmem.c > > index ce3ccaac54d6..c6fa6f4f2db8 100644 > > --- a/mm/shmem.c > > +++ b/mm/shmem.c > > @@ -4003,7 +4003,8 @@ bool shmem_huge_enabled(struct vm_area_struct *vma) > > loff_t i_size; > > pgoff_t off; > > > > - if (!transhuge_vma_enabled(vma, vma->vm_flags)) > > + 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; > > -- > > 2.26.2
On Sat, Jul 31, 2021 at 9:01 PM Hugh Dickins <hughd@google.com> wrote: > > On Fri, 30 Jul 2021, Yang Shi wrote: > > On Fri, Jul 30, 2021 at 12:36 AM Hugh Dickins <hughd@google.com> wrote: > > > > > > 5.14 commit e6be37b2e7bd ("mm/huge_memory.c: add missing read-only THP > > > checking in transparent_hugepage_enabled()") added transhuge_vma_enabled() > > > as a wrapper for two very different checks: shmem_huge_enabled() prefers > > > to show those two checks explicitly, as before. > > > > Basically I have no objection to separating them again. But IMHO they > > seem not very different. Or just makes things easier for the following > > patches? > > Well, it made it easier to apply the patch I'd prepared earlier, > but that was not the point; and I thought it best to be upfront > about the reversion, rather than hiding it in the movement. > > The end result of the two checks is the same (don't try for huge pages), > and they have been grouped together because they occurred together in > several places, and both rely on "vma". > > But one check is whether the app has marked that address range not to use > THPs; and the other check is whether the process is running in a hierarchy > that has been marked never to use THPs (which just uses vma to get to mm > to get to mm->flags (whether current->mm would be more relevant is not an > argument I want to get into, I'm not at all sure)). > > To me those are very different; and I'm particularly concerned to make > MMF_DISABLE_THP references visible, since it did not exist when Kirill > and I first implemented shmem huge pages, and I've tended to forget it: > but consider it more in this series. Yes, I agree one checks vma the other one checks mm, they are different from this perspective. Anyway, as I said I have no objection to this change. You could add Reviewed-by: Yang Shi <shy828301@gmail.com> > > Hugh > > > > > > > > > Signed-off-by: Hugh Dickins <hughd@google.com> > > > --- > > > mm/shmem.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/mm/shmem.c b/mm/shmem.c > > > index ce3ccaac54d6..c6fa6f4f2db8 100644 > > > --- a/mm/shmem.c > > > +++ b/mm/shmem.c > > > @@ -4003,7 +4003,8 @@ bool shmem_huge_enabled(struct vm_area_struct *vma) > > > loff_t i_size; > > > pgoff_t off; > > > > > > - if (!transhuge_vma_enabled(vma, vma->vm_flags)) > > > + 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; > > > -- > > > 2.26.2
diff --git a/mm/shmem.c b/mm/shmem.c index ce3ccaac54d6..c6fa6f4f2db8 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -4003,7 +4003,8 @@ bool shmem_huge_enabled(struct vm_area_struct *vma) loff_t i_size; pgoff_t off; - if (!transhuge_vma_enabled(vma, vma->vm_flags)) + 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;
5.14 commit e6be37b2e7bd ("mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled()") added transhuge_vma_enabled() as a wrapper for two very different checks: shmem_huge_enabled() prefers to show those two checks explicitly, as before. Signed-off-by: Hugh Dickins <hughd@google.com> --- mm/shmem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)