linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: enforce THP for VM_NOHUGEPAGE dax mappings
@ 2018-09-28 22:31 Yang Shi
  2018-09-28 22:36 ` Dan Williams
  0 siblings, 1 reply; 3+ messages in thread
From: Yang Shi @ 2018-09-28 22:31 UTC (permalink / raw)
  To: dan.j.williams, jack, ross.zwisler, kirill.shutemov, akpm
  Cc: yang.shi, linux-mm, linux-kernel

commit baabda261424517110ea98c6651f632ebf2561e3 ("mm: always enable thp
for dax mappings") says madvise hguepage policy makes less sense for
dax, and force enabling thp for dax mappings in all cases, even though
THP is set to "never".

However, transparent_hugepage_enabled() may return false if
VM_NOHUGEPAGE is set even though the mapping is dax.

So, move is_vma_dax() check to the very beginning to enforce THP for dax
mappings in all cases.

Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
---
I didn't find anyone mention the check should be before VM_NOHUGEPAGE in
the review for Dan's original patch. And, that patch commit log states
clearly that THP for dax mapping for all cases even though THP is never.
So, I'm supposed it should behave in this way.

 include/linux/huge_mm.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 99c19b0..b2ad305 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -95,6 +95,9 @@ extern ssize_t single_hugepage_flag_show(struct kobject *kobj,
 
 static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
 {
+	if (vma_is_dax(vma))
+		return true;
+
 	if (vma->vm_flags & VM_NOHUGEPAGE)
 		return false;
 
@@ -107,9 +110,6 @@ static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
 	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
 		return true;
 
-	if (vma_is_dax(vma))
-		return true;
-
 	if (transparent_hugepage_flags &
 				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
 		return !!(vma->vm_flags & VM_HUGEPAGE);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm: enforce THP for VM_NOHUGEPAGE dax mappings
  2018-09-28 22:31 [PATCH] mm: enforce THP for VM_NOHUGEPAGE dax mappings Yang Shi
@ 2018-09-28 22:36 ` Dan Williams
  2018-09-28 22:43   ` Yang Shi
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Williams @ 2018-09-28 22:36 UTC (permalink / raw)
  To: yang.shi
  Cc: Jan Kara, Ross Zwisler, Kirill A. Shutemov, Andrew Morton,
	Linux MM, Linux Kernel Mailing List

On Fri, Sep 28, 2018 at 3:34 PM <yang.shi@linux.alibaba.com> wrote:
>
> commit baabda261424517110ea98c6651f632ebf2561e3 ("mm: always enable thp
> for dax mappings") says madvise hguepage policy makes less sense for
> dax, and force enabling thp for dax mappings in all cases, even though
> THP is set to "never".
>
> However, transparent_hugepage_enabled() may return false if
> VM_NOHUGEPAGE is set even though the mapping is dax.
>
> So, move is_vma_dax() check to the very beginning to enforce THP for dax
> mappings in all cases.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
> ---
> I didn't find anyone mention the check should be before VM_NOHUGEPAGE in
> the review for Dan's original patch. And, that patch commit log states
> clearly that THP for dax mapping for all cases even though THP is never.
> So, I'm supposed it should behave in this way.

No, if someone explicitly does MADV_NOHUGEPAGE then the kernel should
honor that, even if the mapping is DAX.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm: enforce THP for VM_NOHUGEPAGE dax mappings
  2018-09-28 22:36 ` Dan Williams
@ 2018-09-28 22:43   ` Yang Shi
  0 siblings, 0 replies; 3+ messages in thread
From: Yang Shi @ 2018-09-28 22:43 UTC (permalink / raw)
  To: Dan Williams
  Cc: Jan Kara, Ross Zwisler, Kirill A. Shutemov, Andrew Morton,
	Linux MM, Linux Kernel Mailing List



On 9/28/18 3:36 PM, Dan Williams wrote:
> On Fri, Sep 28, 2018 at 3:34 PM <yang.shi@linux.alibaba.com> wrote:
>> commit baabda261424517110ea98c6651f632ebf2561e3 ("mm: always enable thp
>> for dax mappings") says madvise hguepage policy makes less sense for
>> dax, and force enabling thp for dax mappings in all cases, even though
>> THP is set to "never".
>>
>> However, transparent_hugepage_enabled() may return false if
>> VM_NOHUGEPAGE is set even though the mapping is dax.
>>
>> So, move is_vma_dax() check to the very beginning to enforce THP for dax
>> mappings in all cases.
>>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: Jan Kara <jack@suse.cz>
>> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
>> ---
>> I didn't find anyone mention the check should be before VM_NOHUGEPAGE in
>> the review for Dan's original patch. And, that patch commit log states
>> clearly that THP for dax mapping for all cases even though THP is never.
>> So, I'm supposed it should behave in this way.
> No, if someone explicitly does MADV_NOHUGEPAGE then the kernel should
> honor that, even if the mapping is DAX.

Thanks for confirming this. Actually, I had the same question before I 
came up with this patch. "all cases" sounds a little bit misleading.



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-09-28 22:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-28 22:31 [PATCH] mm: enforce THP for VM_NOHUGEPAGE dax mappings Yang Shi
2018-09-28 22:36 ` Dan Williams
2018-09-28 22:43   ` Yang Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).