[PATCH v2 0/2] mm: force enable thp for dax

[PATCH v2 0/2] mm: force enable thp for dax

[Dan Williams]

Changes since v1 [1]:
1/ Fix the transparent_hugepage_enabled() rewrite to be functionally
   equivalent to the old state (Ross)

2/ Add a note as to why we are including fs.h in huge_mm.h so that we
   remember to clean this up if vma_is_dax() is ever moved, or we add a
   VM_* flag for this case. (prompted by Kirill's feedback).

3/ Add some ack and review tags.

[1]: https://www.spinics.net/lists/linux-mm/msg128852.html

---

Hi Andrew,

Please consider taking these 2 patches for 4.13. I spent some time
debugging why a user's device-dax configuration was always failing and
it turned out that their thp policy was set to 'never'. DAX should be
exempt from the policy since it is statically allocated and does not
suffer from any of the potentially negative side effects of thp. More
details in patch 2.

---

Dan Williams (2):
      mm: improve readability of transparent_hugepage_enabled()
      mm: always enable thp for dax mappings


 include/linux/dax.h     |    5 -----
 include/linux/fs.h      |    6 ++++++
 include/linux/huge_mm.h |   37 ++++++++++++++++++++++++++-----------
 3 files changed, 32 insertions(+), 16 deletions(-)
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v2 0/2] mm: force enable thp for dax

[Dan Williams]

Changes since v1 [1]:
1/ Fix the transparent_hugepage_enabled() rewrite to be functionally
   equivalent to the old state (Ross)

2/ Add a note as to why we are including fs.h in huge_mm.h so that we
   remember to clean this up if vma_is_dax() is ever moved, or we add a
   VM_* flag for this case. (prompted by Kirill's feedback).

3/ Add some ack and review tags.

[1]: https://www.spinics.net/lists/linux-mm/msg128852.html

---

Hi Andrew,

Please consider taking these 2 patches for 4.13. I spent some time
debugging why a user's device-dax configuration was always failing and
it turned out that their thp policy was set to 'never'. DAX should be
exempt from the policy since it is statically allocated and does not
suffer from any of the potentially negative side effects of thp. More
details in patch 2.

---

Dan Williams (2):
      mm: improve readability of transparent_hugepage_enabled()
      mm: always enable thp for dax mappings


 include/linux/dax.h     |    5 -----
 include/linux/fs.h      |    6 ++++++
 include/linux/huge_mm.h |   37 ++++++++++++++++++++++++++-----------
 3 files changed, 32 insertions(+), 16 deletions(-)

--
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>

[PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Dan Williams]

Turn the macro into a static inline and rewrite the condition checks for
better readability in preparation for adding another condition.

Cc: Jan Kara <jack@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
[ross: fix logic to make conversion equivalent]
Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/linux/huge_mm.h |   32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index a3762d49ba39..c8119e856eb1 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -85,14 +85,23 @@ extern struct kobj_attribute shmem_enabled_attr;
 
 extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
 
-#define transparent_hugepage_enabled(__vma)				\
-	((transparent_hugepage_flags &					\
-	  (1<<TRANSPARENT_HUGEPAGE_FLAG) ||				\
-	  (transparent_hugepage_flags &					\
-	   (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG) &&			\
-	   ((__vma)->vm_flags & VM_HUGEPAGE))) &&			\
-	 !((__vma)->vm_flags & VM_NOHUGEPAGE) &&			\
-	 !is_vma_temporary_stack(__vma))
+extern unsigned long transparent_hugepage_flags;
+
+static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
+{
+	if ((vma->vm_flags & VM_NOHUGEPAGE) || is_vma_temporary_stack(vma))
+		return false;
+
+	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
+		return true;
+
+	if (transparent_hugepage_flags &
+				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
+		return !!(vma->vm_flags & VM_HUGEPAGE);
+
+	return false;
+}
+
 #define transparent_hugepage_use_zero_page()				\
 	(transparent_hugepage_flags &					\
 	 (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
@@ -104,8 +113,6 @@ extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
 #define transparent_hugepage_debug_cow() 0
 #endif /* CONFIG_DEBUG_VM */
 
-extern unsigned long transparent_hugepage_flags;
-
 extern unsigned long thp_get_unmapped_area(struct file *filp,
 		unsigned long addr, unsigned long len, unsigned long pgoff,
 		unsigned long flags);
@@ -223,7 +230,10 @@ void mm_put_huge_zero_page(struct mm_struct *mm);
 
 #define hpage_nr_pages(x) 1
 
-#define transparent_hugepage_enabled(__vma) 0
+static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
+{
+	return false;
+}
 
 static inline void prep_transhuge_page(struct page *page) {}
 

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Dan Williams]

Turn the macro into a static inline and rewrite the condition checks for
better readability in preparation for adding another condition.

Cc: Jan Kara <jack@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
[ross: fix logic to make conversion equivalent]
Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/linux/huge_mm.h |   32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index a3762d49ba39..c8119e856eb1 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -85,14 +85,23 @@ extern struct kobj_attribute shmem_enabled_attr;
 
 extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
 
-#define transparent_hugepage_enabled(__vma)				\
-	((transparent_hugepage_flags &					\
-	  (1<<TRANSPARENT_HUGEPAGE_FLAG) ||				\
-	  (transparent_hugepage_flags &					\
-	   (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG) &&			\
-	   ((__vma)->vm_flags & VM_HUGEPAGE))) &&			\
-	 !((__vma)->vm_flags & VM_NOHUGEPAGE) &&			\
-	 !is_vma_temporary_stack(__vma))
+extern unsigned long transparent_hugepage_flags;
+
+static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
+{
+	if ((vma->vm_flags & VM_NOHUGEPAGE) || is_vma_temporary_stack(vma))
+		return false;
+
+	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
+		return true;
+
+	if (transparent_hugepage_flags &
+				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
+		return !!(vma->vm_flags & VM_HUGEPAGE);
+
+	return false;
+}
+
 #define transparent_hugepage_use_zero_page()				\
 	(transparent_hugepage_flags &					\
 	 (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
@@ -104,8 +113,6 @@ extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
 #define transparent_hugepage_debug_cow() 0
 #endif /* CONFIG_DEBUG_VM */
 
-extern unsigned long transparent_hugepage_flags;
-
 extern unsigned long thp_get_unmapped_area(struct file *filp,
 		unsigned long addr, unsigned long len, unsigned long pgoff,
 		unsigned long flags);
@@ -223,7 +230,10 @@ void mm_put_huge_zero_page(struct mm_struct *mm);
 
 #define hpage_nr_pages(x) 1
 
-#define transparent_hugepage_enabled(__vma) 0
+static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
+{
+	return false;
+}
 
 static inline void prep_transhuge_page(struct page *page) {}
 

--
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>

[PATCH v2 2/2] mm: always enable thp for dax mappings

[Dan Williams]

The madvise policy for transparent huge pages is meant to avoid unwanted
allocations of transparent huge pages. It allows a policy of disabling
the extra memory pressure and effort to arrange for a huge page when it
is not needed.

DAX by definition never incurs this overhead since it is statically
allocated. The policy choice makes even less sense for device-dax which
tries to guarantee a given tlb-fault size. Specifically, the following
setting:

	echo never > /sys/kernel/mm/transparent_hugepage/enabled

...violates that guarantee and silently disables all device-dax
instances with a 2M or 1G alignment. So, let's avoid that non-obvious
side effect by force enabling thp for dax mappings in all cases.

It is worth noting that the reason this uses vma_is_dax(), and the
resulting header include changes, is that previous attempts to add a
VM_DAX flag were NAKd.

Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/linux/dax.h     |    5 -----
 include/linux/fs.h      |    6 ++++++
 include/linux/huge_mm.h |    5 +++++
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/linux/dax.h b/include/linux/dax.h
index 1f6b6072af64..cbaf3d53d66b 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -151,11 +151,6 @@ static inline unsigned int dax_radix_order(void *entry)
 #endif
 int dax_pfn_mkwrite(struct vm_fault *vmf);
 
-static inline bool vma_is_dax(struct vm_area_struct *vma)
-{
-	return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host);
-}
-
 static inline bool dax_mapping(struct address_space *mapping)
 {
 	return mapping->host && IS_DAX(mapping->host);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 803e5a9b2654..5916ab3a12d5 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -18,6 +18,7 @@
 #include <linux/bug.h>
 #include <linux/mutex.h>
 #include <linux/rwsem.h>
+#include <linux/mm_types.h>
 #include <linux/capability.h>
 #include <linux/semaphore.h>
 #include <linux/fiemap.h>
@@ -3042,6 +3043,11 @@ static inline bool io_is_direct(struct file *filp)
 	return (filp->f_flags & O_DIRECT) || IS_DAX(filp->f_mapping->host);
 }
 
+static inline bool vma_is_dax(struct vm_area_struct *vma)
+{
+	return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host);
+}
+
 static inline int iocb_flags(struct file *file)
 {
 	int res = 0;
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index c8119e856eb1..5a86f615f3cb 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -1,6 +1,8 @@
 #ifndef _LINUX_HUGE_MM_H
 #define _LINUX_HUGE_MM_H
 
+#include <linux/fs.h> /* only for vma_is_dax() */
+
 extern int do_huge_pmd_anonymous_page(struct vm_fault *vmf);
 extern int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 			 pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long addr,
@@ -95,6 +97,9 @@ 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);

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v2 2/2] mm: always enable thp for dax mappings

[Dan Williams]

The madvise policy for transparent huge pages is meant to avoid unwanted
allocations of transparent huge pages. It allows a policy of disabling
the extra memory pressure and effort to arrange for a huge page when it
is not needed.

DAX by definition never incurs this overhead since it is statically
allocated. The policy choice makes even less sense for device-dax which
tries to guarantee a given tlb-fault size. Specifically, the following
setting:

	echo never > /sys/kernel/mm/transparent_hugepage/enabled

...violates that guarantee and silently disables all device-dax
instances with a 2M or 1G alignment. So, let's avoid that non-obvious
side effect by force enabling thp for dax mappings in all cases.

It is worth noting that the reason this uses vma_is_dax(), and the
resulting header include changes, is that previous attempts to add a
VM_DAX flag were NAKd.

Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/linux/dax.h     |    5 -----
 include/linux/fs.h      |    6 ++++++
 include/linux/huge_mm.h |    5 +++++
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/linux/dax.h b/include/linux/dax.h
index 1f6b6072af64..cbaf3d53d66b 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -151,11 +151,6 @@ static inline unsigned int dax_radix_order(void *entry)
 #endif
 int dax_pfn_mkwrite(struct vm_fault *vmf);
 
-static inline bool vma_is_dax(struct vm_area_struct *vma)
-{
-	return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host);
-}
-
 static inline bool dax_mapping(struct address_space *mapping)
 {
 	return mapping->host && IS_DAX(mapping->host);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 803e5a9b2654..5916ab3a12d5 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -18,6 +18,7 @@
 #include <linux/bug.h>
 #include <linux/mutex.h>
 #include <linux/rwsem.h>
+#include <linux/mm_types.h>
 #include <linux/capability.h>
 #include <linux/semaphore.h>
 #include <linux/fiemap.h>
@@ -3042,6 +3043,11 @@ static inline bool io_is_direct(struct file *filp)
 	return (filp->f_flags & O_DIRECT) || IS_DAX(filp->f_mapping->host);
 }
 
+static inline bool vma_is_dax(struct vm_area_struct *vma)
+{
+	return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host);
+}
+
 static inline int iocb_flags(struct file *file)
 {
 	int res = 0;
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index c8119e856eb1..5a86f615f3cb 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -1,6 +1,8 @@
 #ifndef _LINUX_HUGE_MM_H
 #define _LINUX_HUGE_MM_H
 
+#include <linux/fs.h> /* only for vma_is_dax() */
+
 extern int do_huge_pmd_anonymous_page(struct vm_fault *vmf);
 extern int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 			 pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long addr,
@@ -95,6 +97,9 @@ 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);

--
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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Jan Kara]

On Tue 13-06-17 16:08:26, Dan Williams wrote:
> Turn the macro into a static inline and rewrite the condition checks for
> better readability in preparation for adding another condition.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> [ross: fix logic to make conversion equivalent]
> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/linux/huge_mm.h |   32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index a3762d49ba39..c8119e856eb1 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -85,14 +85,23 @@ extern struct kobj_attribute shmem_enabled_attr;
>  
>  extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  
> -#define transparent_hugepage_enabled(__vma)				\
> -	((transparent_hugepage_flags &					\
> -	  (1<<TRANSPARENT_HUGEPAGE_FLAG) ||				\
> -	  (transparent_hugepage_flags &					\
> -	   (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG) &&			\
> -	   ((__vma)->vm_flags & VM_HUGEPAGE))) &&			\
> -	 !((__vma)->vm_flags & VM_NOHUGEPAGE) &&			\
> -	 !is_vma_temporary_stack(__vma))
> +extern unsigned long transparent_hugepage_flags;
> +
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	if ((vma->vm_flags & VM_NOHUGEPAGE) || is_vma_temporary_stack(vma))
> +		return false;
> +
> +	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
> +		return true;
> +
> +	if (transparent_hugepage_flags &
> +				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
> +		return !!(vma->vm_flags & VM_HUGEPAGE);
> +
> +	return false;
> +}
> +
>  #define transparent_hugepage_use_zero_page()				\
>  	(transparent_hugepage_flags &					\
>  	 (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
> @@ -104,8 +113,6 @@ extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  #define transparent_hugepage_debug_cow() 0
>  #endif /* CONFIG_DEBUG_VM */
>  
> -extern unsigned long transparent_hugepage_flags;
> -
>  extern unsigned long thp_get_unmapped_area(struct file *filp,
>  		unsigned long addr, unsigned long len, unsigned long pgoff,
>  		unsigned long flags);
> @@ -223,7 +230,10 @@ void mm_put_huge_zero_page(struct mm_struct *mm);
>  
>  #define hpage_nr_pages(x) 1
>  
> -#define transparent_hugepage_enabled(__vma) 0
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	return false;
> +}
>  
>  static inline void prep_transhuge_page(struct page *page) {}
>  
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Jan Kara]

On Tue 13-06-17 16:08:26, Dan Williams wrote:
> Turn the macro into a static inline and rewrite the condition checks for
> better readability in preparation for adding another condition.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> [ross: fix logic to make conversion equivalent]
> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/linux/huge_mm.h |   32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index a3762d49ba39..c8119e856eb1 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -85,14 +85,23 @@ extern struct kobj_attribute shmem_enabled_attr;
>  
>  extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  
> -#define transparent_hugepage_enabled(__vma)				\
> -	((transparent_hugepage_flags &					\
> -	  (1<<TRANSPARENT_HUGEPAGE_FLAG) ||				\
> -	  (transparent_hugepage_flags &					\
> -	   (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG) &&			\
> -	   ((__vma)->vm_flags & VM_HUGEPAGE))) &&			\
> -	 !((__vma)->vm_flags & VM_NOHUGEPAGE) &&			\
> -	 !is_vma_temporary_stack(__vma))
> +extern unsigned long transparent_hugepage_flags;
> +
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	if ((vma->vm_flags & VM_NOHUGEPAGE) || is_vma_temporary_stack(vma))
> +		return false;
> +
> +	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
> +		return true;
> +
> +	if (transparent_hugepage_flags &
> +				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
> +		return !!(vma->vm_flags & VM_HUGEPAGE);
> +
> +	return false;
> +}
> +
>  #define transparent_hugepage_use_zero_page()				\
>  	(transparent_hugepage_flags &					\
>  	 (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
> @@ -104,8 +113,6 @@ extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  #define transparent_hugepage_debug_cow() 0
>  #endif /* CONFIG_DEBUG_VM */
>  
> -extern unsigned long transparent_hugepage_flags;
> -
>  extern unsigned long thp_get_unmapped_area(struct file *filp,
>  		unsigned long addr, unsigned long len, unsigned long pgoff,
>  		unsigned long flags);
> @@ -223,7 +230,10 @@ void mm_put_huge_zero_page(struct mm_struct *mm);
>  
>  #define hpage_nr_pages(x) 1
>  
> -#define transparent_hugepage_enabled(__vma) 0
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	return false;
> +}
>  
>  static inline void prep_transhuge_page(struct page *page) {}
>  
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

--
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>

Re: [PATCH v2 2/2] mm: always enable thp for dax mappings

[Jan Kara]

On Tue 13-06-17 16:08:31, Dan Williams wrote:
> The madvise policy for transparent huge pages is meant to avoid unwanted
> allocations of transparent huge pages. It allows a policy of disabling
> the extra memory pressure and effort to arrange for a huge page when it
> is not needed.
> 
> DAX by definition never incurs this overhead since it is statically
> allocated. The policy choice makes even less sense for device-dax which
> tries to guarantee a given tlb-fault size. Specifically, the following
> setting:
> 
> 	echo never > /sys/kernel/mm/transparent_hugepage/enabled
> 
> ...violates that guarantee and silently disables all device-dax
> instances with a 2M or 1G alignment. So, let's avoid that non-obvious
> side effect by force enabling thp for dax mappings in all cases.
> 
> It is worth noting that the reason this uses vma_is_dax(), and the
> resulting header include changes, is that previous attempts to add a
> VM_DAX flag were NAKd.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

OK, makes sense. I can imagine some people would like to tune whether DAX
uses huge pages or not but that would make sense as a separate knob
(possibly on per-fs basis) and let's wait for real requests for this
functionality. So:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/linux/dax.h     |    5 -----
>  include/linux/fs.h      |    6 ++++++
>  include/linux/huge_mm.h |    5 +++++
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/dax.h b/include/linux/dax.h
> index 1f6b6072af64..cbaf3d53d66b 100644
> --- a/include/linux/dax.h
> +++ b/include/linux/dax.h
> @@ -151,11 +151,6 @@ static inline unsigned int dax_radix_order(void *entry)
>  #endif
>  int dax_pfn_mkwrite(struct vm_fault *vmf);
>  
> -static inline bool vma_is_dax(struct vm_area_struct *vma)
> -{
> -	return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host);
> -}
> -
>  static inline bool dax_mapping(struct address_space *mapping)
>  {
>  	return mapping->host && IS_DAX(mapping->host);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 803e5a9b2654..5916ab3a12d5 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -18,6 +18,7 @@
>  #include <linux/bug.h>
>  #include <linux/mutex.h>
>  #include <linux/rwsem.h>
> +#include <linux/mm_types.h>
>  #include <linux/capability.h>
>  #include <linux/semaphore.h>
>  #include <linux/fiemap.h>
> @@ -3042,6 +3043,11 @@ static inline bool io_is_direct(struct file *filp)
>  	return (filp->f_flags & O_DIRECT) || IS_DAX(filp->f_mapping->host);
>  }
>  
> +static inline bool vma_is_dax(struct vm_area_struct *vma)
> +{
> +	return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host);
> +}
> +
>  static inline int iocb_flags(struct file *file)
>  {
>  	int res = 0;
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index c8119e856eb1..5a86f615f3cb 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -1,6 +1,8 @@
>  #ifndef _LINUX_HUGE_MM_H
>  #define _LINUX_HUGE_MM_H
>  
> +#include <linux/fs.h> /* only for vma_is_dax() */
> +
>  extern int do_huge_pmd_anonymous_page(struct vm_fault *vmf);
>  extern int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  			 pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long addr,
> @@ -95,6 +97,9 @@ 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);
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 2/2] mm: always enable thp for dax mappings

[Jan Kara]

On Tue 13-06-17 16:08:31, Dan Williams wrote:
> The madvise policy for transparent huge pages is meant to avoid unwanted
> allocations of transparent huge pages. It allows a policy of disabling
> the extra memory pressure and effort to arrange for a huge page when it
> is not needed.
> 
> DAX by definition never incurs this overhead since it is statically
> allocated. The policy choice makes even less sense for device-dax which
> tries to guarantee a given tlb-fault size. Specifically, the following
> setting:
> 
> 	echo never > /sys/kernel/mm/transparent_hugepage/enabled
> 
> ...violates that guarantee and silently disables all device-dax
> instances with a 2M or 1G alignment. So, let's avoid that non-obvious
> side effect by force enabling thp for dax mappings in all cases.
> 
> It is worth noting that the reason this uses vma_is_dax(), and the
> resulting header include changes, is that previous attempts to add a
> VM_DAX flag were NAKd.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

OK, makes sense. I can imagine some people would like to tune whether DAX
uses huge pages or not but that would make sense as a separate knob
(possibly on per-fs basis) and let's wait for real requests for this
functionality. So:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/linux/dax.h     |    5 -----
>  include/linux/fs.h      |    6 ++++++
>  include/linux/huge_mm.h |    5 +++++
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/dax.h b/include/linux/dax.h
> index 1f6b6072af64..cbaf3d53d66b 100644
> --- a/include/linux/dax.h
> +++ b/include/linux/dax.h
> @@ -151,11 +151,6 @@ static inline unsigned int dax_radix_order(void *entry)
>  #endif
>  int dax_pfn_mkwrite(struct vm_fault *vmf);
>  
> -static inline bool vma_is_dax(struct vm_area_struct *vma)
> -{
> -	return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host);
> -}
> -
>  static inline bool dax_mapping(struct address_space *mapping)
>  {
>  	return mapping->host && IS_DAX(mapping->host);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 803e5a9b2654..5916ab3a12d5 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -18,6 +18,7 @@
>  #include <linux/bug.h>
>  #include <linux/mutex.h>
>  #include <linux/rwsem.h>
> +#include <linux/mm_types.h>
>  #include <linux/capability.h>
>  #include <linux/semaphore.h>
>  #include <linux/fiemap.h>
> @@ -3042,6 +3043,11 @@ static inline bool io_is_direct(struct file *filp)
>  	return (filp->f_flags & O_DIRECT) || IS_DAX(filp->f_mapping->host);
>  }
>  
> +static inline bool vma_is_dax(struct vm_area_struct *vma)
> +{
> +	return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host);
> +}
> +
>  static inline int iocb_flags(struct file *file)
>  {
>  	int res = 0;
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index c8119e856eb1..5a86f615f3cb 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -1,6 +1,8 @@
>  #ifndef _LINUX_HUGE_MM_H
>  #define _LINUX_HUGE_MM_H
>  
> +#include <linux/fs.h> /* only for vma_is_dax() */
> +
>  extern int do_huge_pmd_anonymous_page(struct vm_fault *vmf);
>  extern int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  			 pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long addr,
> @@ -95,6 +97,9 @@ 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);
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

--
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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Michal Hocko]

On Tue 13-06-17 16:08:26, Dan Williams wrote:
> Turn the macro into a static inline and rewrite the condition checks for
> better readability in preparation for adding another condition.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> [ross: fix logic to make conversion equivalent]
> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

This is really a nice deobfuscation! Please note this will conflict with
http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com

Trivial to resolve but I thought I should give you a heads up.

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  include/linux/huge_mm.h |   32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index a3762d49ba39..c8119e856eb1 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -85,14 +85,23 @@ extern struct kobj_attribute shmem_enabled_attr;
>  
>  extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  
> -#define transparent_hugepage_enabled(__vma)				\
> -	((transparent_hugepage_flags &					\
> -	  (1<<TRANSPARENT_HUGEPAGE_FLAG) ||				\
> -	  (transparent_hugepage_flags &					\
> -	   (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG) &&			\
> -	   ((__vma)->vm_flags & VM_HUGEPAGE))) &&			\
> -	 !((__vma)->vm_flags & VM_NOHUGEPAGE) &&			\
> -	 !is_vma_temporary_stack(__vma))
> +extern unsigned long transparent_hugepage_flags;
> +
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	if ((vma->vm_flags & VM_NOHUGEPAGE) || is_vma_temporary_stack(vma))
> +		return false;
> +
> +	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
> +		return true;
> +
> +	if (transparent_hugepage_flags &
> +				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
> +		return !!(vma->vm_flags & VM_HUGEPAGE);
> +
> +	return false;
> +}
> +
>  #define transparent_hugepage_use_zero_page()				\
>  	(transparent_hugepage_flags &					\
>  	 (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
> @@ -104,8 +113,6 @@ extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  #define transparent_hugepage_debug_cow() 0
>  #endif /* CONFIG_DEBUG_VM */
>  
> -extern unsigned long transparent_hugepage_flags;
> -
>  extern unsigned long thp_get_unmapped_area(struct file *filp,
>  		unsigned long addr, unsigned long len, unsigned long pgoff,
>  		unsigned long flags);
> @@ -223,7 +230,10 @@ void mm_put_huge_zero_page(struct mm_struct *mm);
>  
>  #define hpage_nr_pages(x) 1
>  
> -#define transparent_hugepage_enabled(__vma) 0
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	return false;
> +}
>  
>  static inline void prep_transhuge_page(struct page *page) {}
>  
> 
> --
> 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>

-- 
Michal Hocko
SUSE Labs
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Michal Hocko]

On Tue 13-06-17 16:08:26, Dan Williams wrote:
> Turn the macro into a static inline and rewrite the condition checks for
> better readability in preparation for adding another condition.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> [ross: fix logic to make conversion equivalent]
> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

This is really a nice deobfuscation! Please note this will conflict with
http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com

Trivial to resolve but I thought I should give you a heads up.

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  include/linux/huge_mm.h |   32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index a3762d49ba39..c8119e856eb1 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -85,14 +85,23 @@ extern struct kobj_attribute shmem_enabled_attr;
>  
>  extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  
> -#define transparent_hugepage_enabled(__vma)				\
> -	((transparent_hugepage_flags &					\
> -	  (1<<TRANSPARENT_HUGEPAGE_FLAG) ||				\
> -	  (transparent_hugepage_flags &					\
> -	   (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG) &&			\
> -	   ((__vma)->vm_flags & VM_HUGEPAGE))) &&			\
> -	 !((__vma)->vm_flags & VM_NOHUGEPAGE) &&			\
> -	 !is_vma_temporary_stack(__vma))
> +extern unsigned long transparent_hugepage_flags;
> +
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	if ((vma->vm_flags & VM_NOHUGEPAGE) || is_vma_temporary_stack(vma))
> +		return false;
> +
> +	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
> +		return true;
> +
> +	if (transparent_hugepage_flags &
> +				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
> +		return !!(vma->vm_flags & VM_HUGEPAGE);
> +
> +	return false;
> +}
> +
>  #define transparent_hugepage_use_zero_page()				\
>  	(transparent_hugepage_flags &					\
>  	 (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
> @@ -104,8 +113,6 @@ extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  #define transparent_hugepage_debug_cow() 0
>  #endif /* CONFIG_DEBUG_VM */
>  
> -extern unsigned long transparent_hugepage_flags;
> -
>  extern unsigned long thp_get_unmapped_area(struct file *filp,
>  		unsigned long addr, unsigned long len, unsigned long pgoff,
>  		unsigned long flags);
> @@ -223,7 +230,10 @@ void mm_put_huge_zero_page(struct mm_struct *mm);
>  
>  #define hpage_nr_pages(x) 1
>  
> -#define transparent_hugepage_enabled(__vma) 0
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	return false;
> +}
>  
>  static inline void prep_transhuge_page(struct page *page) {}
>  
> 
> --
> 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>

-- 
Michal Hocko
SUSE Labs

--
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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Vlastimil Babka]

On 06/14/2017 01:08 AM, Dan Williams wrote:
> Turn the macro into a static inline and rewrite the condition checks for
> better readability in preparation for adding another condition.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> [ross: fix logic to make conversion equivalent]
> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

vbabka@gusiac:~/wrk/cbmc> cbmc test-thp.c
CBMC version 5.3 64-bit x86_64 linux
Parsing test-thp.c
file <command-line> line 0: <command-line>:0:0: warning:
"__STDC_VERSION__" redefined
file <command-line> line 0: <built-in>: note: this is the location of
the previous definition
Converting
Type-checking test-thp
file test-thp.c line 75 function main: function `assert' is not declared
Generating GOTO Program
Adding CPROVER library
Function Pointer Removal
Partial Inlining
Generic Property Instrumentation
Starting Bounded Model Checking
size of program expression: 171 steps
simple slicing removed 3 assignments
Generated 1 VCC(s), 1 remaining after simplification
Passing problem to propositional reduction
converting SSA
Running propositional reduction
Post-processing
Solving with MiniSAT 2.2.0 with simplifier
4899 variables, 13228 clauses
SAT checker: negated claim is UNSATISFIABLE, i.e., holds
Runtime decision procedure: 0.008s
VERIFICATION SUCCESSFUL

(and yeah, the v1 version fails :)

> ---
>  include/linux/huge_mm.h |   32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index a3762d49ba39..c8119e856eb1 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -85,14 +85,23 @@ extern struct kobj_attribute shmem_enabled_attr;
>  
>  extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  
> -#define transparent_hugepage_enabled(__vma)				\
> -	((transparent_hugepage_flags &					\
> -	  (1<<TRANSPARENT_HUGEPAGE_FLAG) ||				\
> -	  (transparent_hugepage_flags &					\
> -	   (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG) &&			\
> -	   ((__vma)->vm_flags & VM_HUGEPAGE))) &&			\
> -	 !((__vma)->vm_flags & VM_NOHUGEPAGE) &&			\
> -	 !is_vma_temporary_stack(__vma))
> +extern unsigned long transparent_hugepage_flags;
> +
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	if ((vma->vm_flags & VM_NOHUGEPAGE) || is_vma_temporary_stack(vma))
> +		return false;
> +
> +	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
> +		return true;
> +
> +	if (transparent_hugepage_flags &
> +				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
> +		return !!(vma->vm_flags & VM_HUGEPAGE);
> +
> +	return false;
> +}
> +
>  #define transparent_hugepage_use_zero_page()				\
>  	(transparent_hugepage_flags &					\
>  	 (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
> @@ -104,8 +113,6 @@ extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  #define transparent_hugepage_debug_cow() 0
>  #endif /* CONFIG_DEBUG_VM */
>  
> -extern unsigned long transparent_hugepage_flags;
> -
>  extern unsigned long thp_get_unmapped_area(struct file *filp,
>  		unsigned long addr, unsigned long len, unsigned long pgoff,
>  		unsigned long flags);
> @@ -223,7 +230,10 @@ void mm_put_huge_zero_page(struct mm_struct *mm);
>  
>  #define hpage_nr_pages(x) 1
>  
> -#define transparent_hugepage_enabled(__vma) 0
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	return false;
> +}
>  
>  static inline void prep_transhuge_page(struct page *page) {}
>  
> 
> --
> 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>
> 

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Vlastimil Babka]

On 06/14/2017 01:08 AM, Dan Williams wrote:
> Turn the macro into a static inline and rewrite the condition checks for
> better readability in preparation for adding another condition.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> [ross: fix logic to make conversion equivalent]
> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

vbabka@gusiac:~/wrk/cbmc> cbmc test-thp.c
CBMC version 5.3 64-bit x86_64 linux
Parsing test-thp.c
file <command-line> line 0: <command-line>:0:0: warning:
"__STDC_VERSION__" redefined
file <command-line> line 0: <built-in>: note: this is the location of
the previous definition
Converting
Type-checking test-thp
file test-thp.c line 75 function main: function `assert' is not declared
Generating GOTO Program
Adding CPROVER library
Function Pointer Removal
Partial Inlining
Generic Property Instrumentation
Starting Bounded Model Checking
size of program expression: 171 steps
simple slicing removed 3 assignments
Generated 1 VCC(s), 1 remaining after simplification
Passing problem to propositional reduction
converting SSA
Running propositional reduction
Post-processing
Solving with MiniSAT 2.2.0 with simplifier
4899 variables, 13228 clauses
SAT checker: negated claim is UNSATISFIABLE, i.e., holds
Runtime decision procedure: 0.008s
VERIFICATION SUCCESSFUL

(and yeah, the v1 version fails :)

> ---
>  include/linux/huge_mm.h |   32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index a3762d49ba39..c8119e856eb1 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -85,14 +85,23 @@ extern struct kobj_attribute shmem_enabled_attr;
>  
>  extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  
> -#define transparent_hugepage_enabled(__vma)				\
> -	((transparent_hugepage_flags &					\
> -	  (1<<TRANSPARENT_HUGEPAGE_FLAG) ||				\
> -	  (transparent_hugepage_flags &					\
> -	   (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG) &&			\
> -	   ((__vma)->vm_flags & VM_HUGEPAGE))) &&			\
> -	 !((__vma)->vm_flags & VM_NOHUGEPAGE) &&			\
> -	 !is_vma_temporary_stack(__vma))
> +extern unsigned long transparent_hugepage_flags;
> +
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	if ((vma->vm_flags & VM_NOHUGEPAGE) || is_vma_temporary_stack(vma))
> +		return false;
> +
> +	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
> +		return true;
> +
> +	if (transparent_hugepage_flags &
> +				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
> +		return !!(vma->vm_flags & VM_HUGEPAGE);
> +
> +	return false;
> +}
> +
>  #define transparent_hugepage_use_zero_page()				\
>  	(transparent_hugepage_flags &					\
>  	 (1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
> @@ -104,8 +113,6 @@ extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
>  #define transparent_hugepage_debug_cow() 0
>  #endif /* CONFIG_DEBUG_VM */
>  
> -extern unsigned long transparent_hugepage_flags;
> -
>  extern unsigned long thp_get_unmapped_area(struct file *filp,
>  		unsigned long addr, unsigned long len, unsigned long pgoff,
>  		unsigned long flags);
> @@ -223,7 +230,10 @@ void mm_put_huge_zero_page(struct mm_struct *mm);
>  
>  #define hpage_nr_pages(x) 1
>  
> -#define transparent_hugepage_enabled(__vma) 0
> +static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +{
> +	return false;
> +}
>  
>  static inline void prep_transhuge_page(struct page *page) {}
>  
> 
> --
> 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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Dan Williams]

On Wed, Jun 14, 2017 at 9:53 AM, Vlastimil Babka <vbabka@suse.cz> wrote:
> On 06/14/2017 01:08 AM, Dan Williams wrote:
>> Turn the macro into a static inline and rewrite the condition checks for
>> better readability in preparation for adding another condition.
>>
>> Cc: Jan Kara <jack@suse.cz>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> [ross: fix logic to make conversion equivalent]
>> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>
> vbabka@gusiac:~/wrk/cbmc> cbmc test-thp.c
> CBMC version 5.3 64-bit x86_64 linux
> Parsing test-thp.c
> file <command-line> line 0: <command-line>:0:0: warning:
> "__STDC_VERSION__" redefined
> file <command-line> line 0: <built-in>: note: this is the location of
> the previous definition
> Converting
> Type-checking test-thp
> file test-thp.c line 75 function main: function `assert' is not declared
> Generating GOTO Program
> Adding CPROVER library
> Function Pointer Removal
> Partial Inlining
> Generic Property Instrumentation
> Starting Bounded Model Checking
> size of program expression: 171 steps
> simple slicing removed 3 assignments
> Generated 1 VCC(s), 1 remaining after simplification
> Passing problem to propositional reduction
> converting SSA
> Running propositional reduction
> Post-processing
> Solving with MiniSAT 2.2.0 with simplifier
> 4899 variables, 13228 clauses
> SAT checker: negated claim is UNSATISFIABLE, i.e., holds
> Runtime decision procedure: 0.008s
> VERIFICATION SUCCESSFUL
>
> (and yeah, the v1 version fails :)

Can you share the test-thp.c so I can add this to my test collection?
I'm assuming cbmc is "Bounded Model Checker for C/C++"?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Dan Williams]

On Wed, Jun 14, 2017 at 9:53 AM, Vlastimil Babka <vbabka@suse.cz> wrote:
> On 06/14/2017 01:08 AM, Dan Williams wrote:
>> Turn the macro into a static inline and rewrite the condition checks for
>> better readability in preparation for adding another condition.
>>
>> Cc: Jan Kara <jack@suse.cz>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> [ross: fix logic to make conversion equivalent]
>> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>
> vbabka@gusiac:~/wrk/cbmc> cbmc test-thp.c
> CBMC version 5.3 64-bit x86_64 linux
> Parsing test-thp.c
> file <command-line> line 0: <command-line>:0:0: warning:
> "__STDC_VERSION__" redefined
> file <command-line> line 0: <built-in>: note: this is the location of
> the previous definition
> Converting
> Type-checking test-thp
> file test-thp.c line 75 function main: function `assert' is not declared
> Generating GOTO Program
> Adding CPROVER library
> Function Pointer Removal
> Partial Inlining
> Generic Property Instrumentation
> Starting Bounded Model Checking
> size of program expression: 171 steps
> simple slicing removed 3 assignments
> Generated 1 VCC(s), 1 remaining after simplification
> Passing problem to propositional reduction
> converting SSA
> Running propositional reduction
> Post-processing
> Solving with MiniSAT 2.2.0 with simplifier
> 4899 variables, 13228 clauses
> SAT checker: negated claim is UNSATISFIABLE, i.e., holds
> Runtime decision procedure: 0.008s
> VERIFICATION SUCCESSFUL
>
> (and yeah, the v1 version fails :)

Can you share the test-thp.c so I can add this to my test collection?
I'm assuming cbmc is "Bounded Model Checker for C/C++"?

--
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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Vlastimil Babka]

On 06/14/2017 07:02 PM, Dan Williams wrote:
> On Wed, Jun 14, 2017 at 9:53 AM, Vlastimil Babka <vbabka@suse.cz> wrote:
> 
> Can you share the test-thp.c so I can add this to my test collection?

Attached.

> I'm assuming cbmc is "Bounded Model Checker for C/C++"?

Yes. This blog from Paul inspired me:
http://paulmck.livejournal.com/38997.html

Works nicely, just if it finds a bug, the counterexamples are a bit of
PITA to decipher :)

Vlastimil
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Vlastimil Babka]

[-- Attachment #1: Type: text/plain, Size: 448 bytes --]

On 06/14/2017 07:02 PM, Dan Williams wrote:
> On Wed, Jun 14, 2017 at 9:53 AM, Vlastimil Babka <vbabka@suse.cz> wrote:
> 
> Can you share the test-thp.c so I can add this to my test collection?

Attached.

> I'm assuming cbmc is "Bounded Model Checker for C/C++"?

Yes. This blog from Paul inspired me:
http://paulmck.livejournal.com/38997.html

Works nicely, just if it finds a bug, the counterexamples are a bit of
PITA to decipher :)

Vlastimil

[-- Attachment #2: test-thp.c --]
[-- Type: text/x-csrc, Size: 3072 bytes --]

#include <stdbool.h>
#include <stdio.h>

#define VM_GROWSDOWN    0x00000100      /* general info on the segment */
#define VM_HUGEPAGE     0x20000000      /* MADV_HUGEPAGE marked this vma */
#define VM_NOHUGEPAGE   0x40000000      /* MADV_NOHUGEPAGE marked this vma */
#define VM_ARCH_1       0x01000000      /* Architecture-specific flag */
#define VM_GROWSUP VM_ARCH_1
#define VM_SEQ_READ     0x00008000      /* App will access data sequentially */
#define VM_RAND_READ    0x00010000      /* App will not benefit from clustered reads */
#define VM_STACK_INCOMPLETE_SETUP       (VM_RAND_READ | VM_SEQ_READ)

enum transparent_hugepage_flag {
        TRANSPARENT_HUGEPAGE_FLAG,
        TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
        TRANSPARENT_HUGEPAGE_DEFRAG_DIRECT_FLAG,
        TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_FLAG,
        TRANSPARENT_HUGEPAGE_DEFRAG_KSWAPD_OR_MADV_FLAG,
        TRANSPARENT_HUGEPAGE_DEFRAG_REQ_MADV_FLAG,
        TRANSPARENT_HUGEPAGE_DEFRAG_KHUGEPAGED_FLAG,
        TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG,
        TRANSPARENT_HUGEPAGE_DEBUG_COW_FLAG,
};

unsigned long transparent_hugepage_flags;
struct vm_area_struct {
	unsigned long vm_flags;
};

bool is_vma_temporary_stack(struct vm_area_struct *vma)
{
        int maybe_stack = vma->vm_flags & (VM_GROWSDOWN | VM_GROWSUP);

        if (!maybe_stack)
                return false;

        if ((vma->vm_flags & VM_STACK_INCOMPLETE_SETUP) ==
                                                VM_STACK_INCOMPLETE_SETUP)
                return true;

        return false;
}

#define transparent_hugepage_enabled1(__vma)				\
	((transparent_hugepage_flags &					\
	  (1<<TRANSPARENT_HUGEPAGE_FLAG) ||				\
	  (transparent_hugepage_flags &					\
	   (1<<TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG) &&			\
	   ((__vma)->vm_flags & VM_HUGEPAGE))) &&			\
	 !((__vma)->vm_flags & VM_NOHUGEPAGE) &&			\
	 !is_vma_temporary_stack(__vma))

// v2
static inline bool transparent_hugepage_enabled2(struct vm_area_struct *vma)
{
	if ((vma->vm_flags & VM_NOHUGEPAGE) || is_vma_temporary_stack(vma))
		return false;

	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
		return true;

	if (transparent_hugepage_flags &
				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
		return !!(vma->vm_flags & VM_HUGEPAGE);

	return false;
}

// v1
static inline bool transparent_hugepage_enabled3(struct vm_area_struct *vma)
{
	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
		return true;

	if (transparent_hugepage_flags
			& (1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
		/* check vma flags */;
	else
		return false;

	if ((vma->vm_flags & (VM_HUGEPAGE | VM_NOHUGEPAGE)) == VM_HUGEPAGE
			&& !is_vma_temporary_stack(vma))
		return true;

	return false;
}

int main(int argc, char *argv[])
{
	struct vm_area_struct vma;

	vma.vm_flags = (unsigned long) argv[1];
	transparent_hugepage_flags = (unsigned long) argv[2];

//	assert(transparent_hugepage_enabled1(&vma)
//			== transparent_hugepage_enabled2(&vma));
	assert(transparent_hugepage_enabled1(&vma)
			== transparent_hugepage_enabled3(&vma));
}

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Dan Williams]

On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
> On Tue 13-06-17 16:08:26, Dan Williams wrote:
>> Turn the macro into a static inline and rewrite the condition checks for
>> better readability in preparation for adding another condition.
>>
>> Cc: Jan Kara <jack@suse.cz>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> [ross: fix logic to make conversion equivalent]
>> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> This is really a nice deobfuscation! Please note this will conflict with
> http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
>
>
> Trivial to resolve but I thought I should give you a heads up.

Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
...and while we're there should vma_is_dax() also override
VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
huge pages is to avoid mm pressure, dax exerts no such pressure.

> Acked-by: Michal Hocko <mhocko@suse.com>

Thanks for the heads up.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Dan Williams]

On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
> On Tue 13-06-17 16:08:26, Dan Williams wrote:
>> Turn the macro into a static inline and rewrite the condition checks for
>> better readability in preparation for adding another condition.
>>
>> Cc: Jan Kara <jack@suse.cz>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> [ross: fix logic to make conversion equivalent]
>> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> This is really a nice deobfuscation! Please note this will conflict with
> http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
>
>
> Trivial to resolve but I thought I should give you a heads up.

Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
...and while we're there should vma_is_dax() also override
VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
huge pages is to avoid mm pressure, dax exerts no such pressure.

> Acked-by: Michal Hocko <mhocko@suse.com>

Thanks for the heads up.

--
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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Michal Hocko]

On Wed 14-06-17 12:26:46, Dan Williams wrote:
> On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Tue 13-06-17 16:08:26, Dan Williams wrote:
> >> Turn the macro into a static inline and rewrite the condition checks for
> >> better readability in preparation for adding another condition.
> >>
> >> Cc: Jan Kara <jack@suse.cz>
> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> >> [ross: fix logic to make conversion equivalent]
> >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> >
> > This is really a nice deobfuscation! Please note this will conflict with
> > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
> >
> >
> > Trivial to resolve but I thought I should give you a heads up.
> 
> Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
> ...and while we're there should vma_is_dax() also override
> VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
> huge pages is to avoid mm pressure, dax exerts no such pressure.

As the changelog of the referenced patch says another reason is to stop
khugepaged from interfering and collapsing smaller pages into THP. If
DAX mappings are subject to khugepaged then we really need to exclude
it. Why would you want to override user's decision to disable THP
anyway? I can see why the global knob should be ignored but if the
disable is targeted for the specific VMA or the process then we should
obey that, no?

> > Acked-by: Michal Hocko <mhocko@suse.com>
> 
> Thanks for the heads up.

-- 
Michal Hocko
SUSE Labs
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Michal Hocko]

On Wed 14-06-17 12:26:46, Dan Williams wrote:
> On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Tue 13-06-17 16:08:26, Dan Williams wrote:
> >> Turn the macro into a static inline and rewrite the condition checks for
> >> better readability in preparation for adding another condition.
> >>
> >> Cc: Jan Kara <jack@suse.cz>
> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> >> [ross: fix logic to make conversion equivalent]
> >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> >
> > This is really a nice deobfuscation! Please note this will conflict with
> > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
> >
> >
> > Trivial to resolve but I thought I should give you a heads up.
> 
> Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
> ...and while we're there should vma_is_dax() also override
> VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
> huge pages is to avoid mm pressure, dax exerts no such pressure.

As the changelog of the referenced patch says another reason is to stop
khugepaged from interfering and collapsing smaller pages into THP. If
DAX mappings are subject to khugepaged then we really need to exclude
it. Why would you want to override user's decision to disable THP
anyway? I can see why the global knob should be ignored but if the
disable is targeted for the specific VMA or the process then we should
obey that, no?

> > Acked-by: Michal Hocko <mhocko@suse.com>
> 
> Thanks for the heads up.

-- 
Michal Hocko
SUSE Labs

--
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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Andrew Morton]

On Thu, 15 Jun 2017 10:07:39 +0200 Michal Hocko <mhocko@kernel.org> wrote:

> On Wed 14-06-17 12:26:46, Dan Williams wrote:
> > On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > On Tue 13-06-17 16:08:26, Dan Williams wrote:
> > >> Turn the macro into a static inline and rewrite the condition checks for
> > >> better readability in preparation for adding another condition.
> > >>
> > >> Cc: Jan Kara <jack@suse.cz>
> > >> Cc: Andrew Morton <akpm@linux-foundation.org>
> > >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > >> [ross: fix logic to make conversion equivalent]
> > >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > >
> > > This is really a nice deobfuscation! Please note this will conflict with
> > > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
> > >
> > >
> > > Trivial to resolve but I thought I should give you a heads up.
> > 
> > Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
> > ...and while we're there should vma_is_dax() also override
> > VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
> > huge pages is to avoid mm pressure, dax exerts no such pressure.
> 
> As the changelog of the referenced patch says another reason is to stop
> khugepaged from interfering and collapsing smaller pages into THP. If
> DAX mappings are subject to khugepaged then we really need to exclude
> it. Why would you want to override user's decision to disable THP
> anyway? I can see why the global knob should be ignored but if the
> disable is targeted for the specific VMA or the process then we should
> obey that, no?

So... Like this?

static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
{
	if (vma->vm_flags & VM_NOHUGEPAGE))
		return false;

	if (is_vma_temporary_stack(vma))
		return false;

	if (test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
		return false;

	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
		return true;

	if (transparent_hugepage_flags &
				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
		return !!(vma->vm_flags & VM_HUGEPAGE);

	return false;
}

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Andrew Morton]

On Thu, 15 Jun 2017 10:07:39 +0200 Michal Hocko <mhocko@kernel.org> wrote:

> On Wed 14-06-17 12:26:46, Dan Williams wrote:
> > On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > On Tue 13-06-17 16:08:26, Dan Williams wrote:
> > >> Turn the macro into a static inline and rewrite the condition checks for
> > >> better readability in preparation for adding another condition.
> > >>
> > >> Cc: Jan Kara <jack@suse.cz>
> > >> Cc: Andrew Morton <akpm@linux-foundation.org>
> > >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > >> [ross: fix logic to make conversion equivalent]
> > >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > >
> > > This is really a nice deobfuscation! Please note this will conflict with
> > > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
> > >
> > >
> > > Trivial to resolve but I thought I should give you a heads up.
> > 
> > Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
> > ...and while we're there should vma_is_dax() also override
> > VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
> > huge pages is to avoid mm pressure, dax exerts no such pressure.
> 
> As the changelog of the referenced patch says another reason is to stop
> khugepaged from interfering and collapsing smaller pages into THP. If
> DAX mappings are subject to khugepaged then we really need to exclude
> it. Why would you want to override user's decision to disable THP
> anyway? I can see why the global knob should be ignored but if the
> disable is targeted for the specific VMA or the process then we should
> obey that, no?

So... Like this?

static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
{
	if (vma->vm_flags & VM_NOHUGEPAGE))
		return false;

	if (is_vma_temporary_stack(vma))
		return false;

	if (test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
		return false;

	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
		return true;

	if (transparent_hugepage_flags &
				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
		return !!(vma->vm_flags & VM_HUGEPAGE);

	return false;
}

--
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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Dan Williams]

On Thu, Jun 15, 2017 at 1:07 AM, Michal Hocko <mhocko@kernel.org> wrote:
> On Wed 14-06-17 12:26:46, Dan Williams wrote:
>> On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> > On Tue 13-06-17 16:08:26, Dan Williams wrote:
>> >> Turn the macro into a static inline and rewrite the condition checks for
>> >> better readability in preparation for adding another condition.
>> >>
>> >> Cc: Jan Kara <jack@suse.cz>
>> >> Cc: Andrew Morton <akpm@linux-foundation.org>
>> >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> >> [ross: fix logic to make conversion equivalent]
>> >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> >
>> > This is really a nice deobfuscation! Please note this will conflict with
>> > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
>> >
>> >
>> > Trivial to resolve but I thought I should give you a heads up.
>>
>> Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
>> ...and while we're there should vma_is_dax() also override
>> VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
>> huge pages is to avoid mm pressure, dax exerts no such pressure.
>
> As the changelog of the referenced patch says another reason is to stop
> khugepaged from interfering and collapsing smaller pages into THP. If
> DAX mappings are subject to khugepaged then we really need to exclude
> it. Why would you want to override user's decision to disable THP
> anyway? I can see why the global knob should be ignored but if the
> disable is targeted for the specific VMA or the process then we should
> obey that, no?

I don't think DAX mappings have any interaction with THP machinery
outside of piggybacking on some of the paths in the fault handling and
the helpers to manage huge page table entries. Since DAX disables the
page cache, and all DAX mappings are file-backed I don't see a need
for a user to disable THP... does anybody else?

I think DAX != THP for any of the cases that
transparent_hugepage_enabled() cares about.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Dan Williams]

On Thu, Jun 15, 2017 at 1:07 AM, Michal Hocko <mhocko@kernel.org> wrote:
> On Wed 14-06-17 12:26:46, Dan Williams wrote:
>> On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> > On Tue 13-06-17 16:08:26, Dan Williams wrote:
>> >> Turn the macro into a static inline and rewrite the condition checks for
>> >> better readability in preparation for adding another condition.
>> >>
>> >> Cc: Jan Kara <jack@suse.cz>
>> >> Cc: Andrew Morton <akpm@linux-foundation.org>
>> >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> >> [ross: fix logic to make conversion equivalent]
>> >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> >
>> > This is really a nice deobfuscation! Please note this will conflict with
>> > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
>> >
>> >
>> > Trivial to resolve but I thought I should give you a heads up.
>>
>> Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
>> ...and while we're there should vma_is_dax() also override
>> VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
>> huge pages is to avoid mm pressure, dax exerts no such pressure.
>
> As the changelog of the referenced patch says another reason is to stop
> khugepaged from interfering and collapsing smaller pages into THP. If
> DAX mappings are subject to khugepaged then we really need to exclude
> it. Why would you want to override user's decision to disable THP
> anyway? I can see why the global knob should be ignored but if the
> disable is targeted for the specific VMA or the process then we should
> obey that, no?

I don't think DAX mappings have any interaction with THP machinery
outside of piggybacking on some of the paths in the fault handling and
the helpers to manage huge page table entries. Since DAX disables the
page cache, and all DAX mappings are file-backed I don't see a need
for a user to disable THP... does anybody else?

I think DAX != THP for any of the cases that
transparent_hugepage_enabled() cares about.

--
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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Michal Hocko]

On Thu 15-06-17 13:21:46, Dan Williams wrote:
> On Thu, Jun 15, 2017 at 1:07 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Wed 14-06-17 12:26:46, Dan Williams wrote:
> >> On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
> >> > On Tue 13-06-17 16:08:26, Dan Williams wrote:
> >> >> Turn the macro into a static inline and rewrite the condition checks for
> >> >> better readability in preparation for adding another condition.
> >> >>
> >> >> Cc: Jan Kara <jack@suse.cz>
> >> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >> >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> >> >> [ross: fix logic to make conversion equivalent]
> >> >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> >> >
> >> > This is really a nice deobfuscation! Please note this will conflict with
> >> > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
> >> >
> >> >
> >> > Trivial to resolve but I thought I should give you a heads up.
> >>
> >> Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
> >> ...and while we're there should vma_is_dax() also override
> >> VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
> >> huge pages is to avoid mm pressure, dax exerts no such pressure.
> >
> > As the changelog of the referenced patch says another reason is to stop
> > khugepaged from interfering and collapsing smaller pages into THP. If
> > DAX mappings are subject to khugepaged then we really need to exclude
> > it. Why would you want to override user's decision to disable THP
> > anyway? I can see why the global knob should be ignored but if the
> > disable is targeted for the specific VMA or the process then we should
> > obey that, no?
> 
> I don't think DAX mappings have any interaction with THP machinery
> outside of piggybacking on some of the paths in the fault handling and
> the helpers to manage huge page table entries. Since DAX disables the
> page cache, and all DAX mappings are file-backed I don't see a need
> for a user to disable THP... does anybody else?

So let me ask differently. If the VMA is explicitly marked to not use
THP resp. the process explicitly asked to be opted out from THP why
should we make any exception for DAX? What makes DAX so special to
ignore what an user asked for? 

-- 
Michal Hocko
SUSE Labs
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Michal Hocko]

On Thu 15-06-17 13:21:46, Dan Williams wrote:
> On Thu, Jun 15, 2017 at 1:07 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Wed 14-06-17 12:26:46, Dan Williams wrote:
> >> On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
> >> > On Tue 13-06-17 16:08:26, Dan Williams wrote:
> >> >> Turn the macro into a static inline and rewrite the condition checks for
> >> >> better readability in preparation for adding another condition.
> >> >>
> >> >> Cc: Jan Kara <jack@suse.cz>
> >> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >> >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> >> >> [ross: fix logic to make conversion equivalent]
> >> >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> >> >
> >> > This is really a nice deobfuscation! Please note this will conflict with
> >> > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
> >> >
> >> >
> >> > Trivial to resolve but I thought I should give you a heads up.
> >>
> >> Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
> >> ...and while we're there should vma_is_dax() also override
> >> VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
> >> huge pages is to avoid mm pressure, dax exerts no such pressure.
> >
> > As the changelog of the referenced patch says another reason is to stop
> > khugepaged from interfering and collapsing smaller pages into THP. If
> > DAX mappings are subject to khugepaged then we really need to exclude
> > it. Why would you want to override user's decision to disable THP
> > anyway? I can see why the global knob should be ignored but if the
> > disable is targeted for the specific VMA or the process then we should
> > obey that, no?
> 
> I don't think DAX mappings have any interaction with THP machinery
> outside of piggybacking on some of the paths in the fault handling and
> the helpers to manage huge page table entries. Since DAX disables the
> page cache, and all DAX mappings are file-backed I don't see a need
> for a user to disable THP... does anybody else?

So let me ask differently. If the VMA is explicitly marked to not use
THP resp. the process explicitly asked to be opted out from THP why
should we make any exception for DAX? What makes DAX so special to
ignore what an user asked for? 

-- 
Michal Hocko
SUSE Labs

--
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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Michal Hocko]

On Thu 15-06-17 13:06:58, Andrew Morton wrote:
> On Thu, 15 Jun 2017 10:07:39 +0200 Michal Hocko <mhocko@kernel.org> wrote:
> 
> > On Wed 14-06-17 12:26:46, Dan Williams wrote:
> > > On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > > On Tue 13-06-17 16:08:26, Dan Williams wrote:
> > > >> Turn the macro into a static inline and rewrite the condition checks for
> > > >> better readability in preparation for adding another condition.
> > > >>
> > > >> Cc: Jan Kara <jack@suse.cz>
> > > >> Cc: Andrew Morton <akpm@linux-foundation.org>
> > > >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > >> [ross: fix logic to make conversion equivalent]
> > > >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > > >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > >
> > > > This is really a nice deobfuscation! Please note this will conflict with
> > > > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
> > > >
> > > >
> > > > Trivial to resolve but I thought I should give you a heads up.
> > > 
> > > Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
> > > ...and while we're there should vma_is_dax() also override
> > > VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
> > > huge pages is to avoid mm pressure, dax exerts no such pressure.
> > 
> > As the changelog of the referenced patch says another reason is to stop
> > khugepaged from interfering and collapsing smaller pages into THP. If
> > DAX mappings are subject to khugepaged then we really need to exclude
> > it. Why would you want to override user's decision to disable THP
> > anyway? I can see why the global knob should be ignored but if the
> > disable is targeted for the specific VMA or the process then we should
> > obey that, no?
> 
> So... Like this?
> 
> static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> {
> 	if (vma->vm_flags & VM_NOHUGEPAGE))
> 		return false;
> 
> 	if (is_vma_temporary_stack(vma))
> 		return false;
> 
> 	if (test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
> 		return false;
> 
> 	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
> 		return true;
> 
> 	if (transparent_hugepage_flags &
> 				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
> 		return !!(vma->vm_flags & VM_HUGEPAGE);
> 
> 	return false;
> }

yes

-- 
Michal Hocko
SUSE Labs
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Michal Hocko]

On Thu 15-06-17 13:06:58, Andrew Morton wrote:
> On Thu, 15 Jun 2017 10:07:39 +0200 Michal Hocko <mhocko@kernel.org> wrote:
> 
> > On Wed 14-06-17 12:26:46, Dan Williams wrote:
> > > On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > > On Tue 13-06-17 16:08:26, Dan Williams wrote:
> > > >> Turn the macro into a static inline and rewrite the condition checks for
> > > >> better readability in preparation for adding another condition.
> > > >>
> > > >> Cc: Jan Kara <jack@suse.cz>
> > > >> Cc: Andrew Morton <akpm@linux-foundation.org>
> > > >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > >> [ross: fix logic to make conversion equivalent]
> > > >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > > >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > >
> > > > This is really a nice deobfuscation! Please note this will conflict with
> > > > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
> > > >
> > > >
> > > > Trivial to resolve but I thought I should give you a heads up.
> > > 
> > > Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
> > > ...and while we're there should vma_is_dax() also override
> > > VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
> > > huge pages is to avoid mm pressure, dax exerts no such pressure.
> > 
> > As the changelog of the referenced patch says another reason is to stop
> > khugepaged from interfering and collapsing smaller pages into THP. If
> > DAX mappings are subject to khugepaged then we really need to exclude
> > it. Why would you want to override user's decision to disable THP
> > anyway? I can see why the global knob should be ignored but if the
> > disable is targeted for the specific VMA or the process then we should
> > obey that, no?
> 
> So... Like this?
> 
> static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> {
> 	if (vma->vm_flags & VM_NOHUGEPAGE))
> 		return false;
> 
> 	if (is_vma_temporary_stack(vma))
> 		return false;
> 
> 	if (test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
> 		return false;
> 
> 	if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_FLAG))
> 		return true;
> 
> 	if (transparent_hugepage_flags &
> 				(1 << TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG))
> 		return !!(vma->vm_flags & VM_HUGEPAGE);
> 
> 	return false;
> }

yes

-- 
Michal Hocko
SUSE Labs

--
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>

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Dan Williams]

On Thu, Jun 15, 2017 at 3:22 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Thu 15-06-17 13:21:46, Dan Williams wrote:
>> On Thu, Jun 15, 2017 at 1:07 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> > On Wed 14-06-17 12:26:46, Dan Williams wrote:
>> >> On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> >> > On Tue 13-06-17 16:08:26, Dan Williams wrote:
>> >> >> Turn the macro into a static inline and rewrite the condition checks for
>> >> >> better readability in preparation for adding another condition.
>> >> >>
>> >> >> Cc: Jan Kara <jack@suse.cz>
>> >> >> Cc: Andrew Morton <akpm@linux-foundation.org>
>> >> >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> >> >> [ross: fix logic to make conversion equivalent]
>> >> >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> >> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> >> >
>> >> > This is really a nice deobfuscation! Please note this will conflict with
>> >> > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
>> >> >
>> >> >
>> >> > Trivial to resolve but I thought I should give you a heads up.
>> >>
>> >> Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
>> >> ...and while we're there should vma_is_dax() also override
>> >> VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
>> >> huge pages is to avoid mm pressure, dax exerts no such pressure.
>> >
>> > As the changelog of the referenced patch says another reason is to stop
>> > khugepaged from interfering and collapsing smaller pages into THP. If
>> > DAX mappings are subject to khugepaged then we really need to exclude
>> > it. Why would you want to override user's decision to disable THP
>> > anyway? I can see why the global knob should be ignored but if the
>> > disable is targeted for the specific VMA or the process then we should
>> > obey that, no?
>>
>> I don't think DAX mappings have any interaction with THP machinery
>> outside of piggybacking on some of the paths in the fault handling and
>> the helpers to manage huge page table entries. Since DAX disables the
>> page cache, and all DAX mappings are file-backed I don't see a need
>> for a user to disable THP... does anybody else?
>
> So let me ask differently. If the VMA is explicitly marked to not use
> THP resp. the process explicitly asked to be opted out from THP why
> should we make any exception for DAX? What makes DAX so special to
> ignore what an user asked for?

Hmm, the only case where this is a problem is in the device-dax case
where it tries to guarantee a given fault granularity. In the
filesystem-dax case it will fall back to 4K mappings which is
expected, device-dax will just fail which is not.

However, I think I can solve this just with better device-dax
documentation that highlights this side-effect of disabling THP when
the device is configured to support a minimum TLB size that is greater
than PAGE_SIZE.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 1/2] mm: improve readability of transparent_hugepage_enabled()

[Dan Williams]

On Thu, Jun 15, 2017 at 3:22 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Thu 15-06-17 13:21:46, Dan Williams wrote:
>> On Thu, Jun 15, 2017 at 1:07 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> > On Wed 14-06-17 12:26:46, Dan Williams wrote:
>> >> On Wed, Jun 14, 2017 at 5:45 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> >> > On Tue 13-06-17 16:08:26, Dan Williams wrote:
>> >> >> Turn the macro into a static inline and rewrite the condition checks for
>> >> >> better readability in preparation for adding another condition.
>> >> >>
>> >> >> Cc: Jan Kara <jack@suse.cz>
>> >> >> Cc: Andrew Morton <akpm@linux-foundation.org>
>> >> >> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> >> >> [ross: fix logic to make conversion equivalent]
>> >> >> Acked-by: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> >> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> >> >
>> >> > This is really a nice deobfuscation! Please note this will conflict with
>> >> > http://lkml.kernel.org/r/1496415802-30944-1-git-send-email-rppt@linux.vnet.ibm.com
>> >> >
>> >> >
>> >> > Trivial to resolve but I thought I should give you a heads up.
>> >>
>> >> Hmm, I'm assuming that vma_is_dax() should override PRCTL_THP_DISABLE?
>> >> ...and while we're there should vma_is_dax() also override
>> >> VM_NOHUGEPAGE? This is with the assumption that the reason to turn off
>> >> huge pages is to avoid mm pressure, dax exerts no such pressure.
>> >
>> > As the changelog of the referenced patch says another reason is to stop
>> > khugepaged from interfering and collapsing smaller pages into THP. If
>> > DAX mappings are subject to khugepaged then we really need to exclude
>> > it. Why would you want to override user's decision to disable THP
>> > anyway? I can see why the global knob should be ignored but if the
>> > disable is targeted for the specific VMA or the process then we should
>> > obey that, no?
>>
>> I don't think DAX mappings have any interaction with THP machinery
>> outside of piggybacking on some of the paths in the fault handling and
>> the helpers to manage huge page table entries. Since DAX disables the
>> page cache, and all DAX mappings are file-backed I don't see a need
>> for a user to disable THP... does anybody else?
>
> So let me ask differently. If the VMA is explicitly marked to not use
> THP resp. the process explicitly asked to be opted out from THP why
> should we make any exception for DAX? What makes DAX so special to
> ignore what an user asked for?

Hmm, the only case where this is a problem is in the device-dax case
where it tries to guarantee a given fault granularity. In the
filesystem-dax case it will fall back to 4K mappings which is
expected, device-dax will just fail which is not.

However, I think I can solve this just with better device-dax
documentation that highlights this side-effect of disabling THP when
the device is configured to support a minimum TLB size that is greater
than PAGE_SIZE.

--
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>