* [PATCH v3 1/5] mm/huge_memory.c: remove dedicated macro HPAGE_CACHE_INDEX_MASK
2021-05-11 13:48 [PATCH v3 0/5] Cleanup and fixup for huge_memory Miaohe Lin
@ 2021-05-11 13:48 ` Miaohe Lin
2021-05-11 13:48 ` [PATCH v3 2/5] mm/huge_memory.c: use page->deferred_list Miaohe Lin
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Miaohe Lin @ 2021-05-11 13:48 UTC (permalink / raw)
To: akpm
Cc: ziy, william.kucharski, willy, yang.shi, aneesh.kumar, rcampbell,
songliubraving, kirill.shutemov, riel, hannes, minchan, hughd,
adobriyan, linux-kernel, linux-mm, linux-fsdevel, linmiaohe
Rewrite the pgoff checking logic to remove macro HPAGE_CACHE_INDEX_MASK
which is only used here to simplify the code.
Reviewed-by: Yang Shi <shy828301@gmail.com>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
include/linux/huge_mm.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 9626fda5efce..0a526f211fec 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -152,15 +152,13 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
bool transparent_hugepage_enabled(struct vm_area_struct *vma);
-#define HPAGE_CACHE_INDEX_MASK (HPAGE_PMD_NR - 1)
-
static inline bool transhuge_vma_suitable(struct vm_area_struct *vma,
unsigned long haddr)
{
/* Don't have to check pgoff for anonymous vma */
if (!vma_is_anonymous(vma)) {
- if (((vma->vm_start >> PAGE_SHIFT) & HPAGE_CACHE_INDEX_MASK) !=
- (vma->vm_pgoff & HPAGE_CACHE_INDEX_MASK))
+ if (!IS_ALIGNED((vma->vm_start >> PAGE_SHIFT) - vma->vm_pgoff,
+ HPAGE_PMD_NR))
return false;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 2/5] mm/huge_memory.c: use page->deferred_list
2021-05-11 13:48 [PATCH v3 0/5] Cleanup and fixup for huge_memory Miaohe Lin
2021-05-11 13:48 ` [PATCH v3 1/5] mm/huge_memory.c: remove dedicated macro HPAGE_CACHE_INDEX_MASK Miaohe Lin
@ 2021-05-11 13:48 ` Miaohe Lin
2021-05-11 23:03 ` Matthew Wilcox
2021-05-11 13:48 ` [PATCH v3 3/5] mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled() Miaohe Lin
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Miaohe Lin @ 2021-05-11 13:48 UTC (permalink / raw)
To: akpm
Cc: ziy, william.kucharski, willy, yang.shi, aneesh.kumar, rcampbell,
songliubraving, kirill.shutemov, riel, hannes, minchan, hughd,
adobriyan, linux-kernel, linux-mm, linux-fsdevel, linmiaohe
Now that we can represent the location of ->deferred_list instead of
->mapping + ->index, make use of it to improve readability.
Reviewed-by: Yang Shi <shy828301@gmail.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/huge_memory.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 63ed6b25deaa..76ca1eb2a223 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2868,7 +2868,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
/* Take pin on all head pages to avoid freeing them under us */
list_for_each_safe(pos, next, &ds_queue->split_queue) {
- page = list_entry((void *)pos, struct page, mapping);
+ page = list_entry((void *)pos, struct page, deferred_list);
page = compound_head(page);
if (get_page_unless_zero(page)) {
list_move(page_deferred_list(page), &list);
@@ -2883,7 +2883,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
list_for_each_safe(pos, next, &list) {
- page = list_entry((void *)pos, struct page, mapping);
+ page = list_entry((void *)pos, struct page, deferred_list);
if (!trylock_page(page))
goto next;
/* split_huge_page() removes page from list on success */
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 2/5] mm/huge_memory.c: use page->deferred_list
2021-05-11 13:48 ` [PATCH v3 2/5] mm/huge_memory.c: use page->deferred_list Miaohe Lin
@ 2021-05-11 23:03 ` Matthew Wilcox
2021-05-12 2:02 ` Miaohe Lin
0 siblings, 1 reply; 10+ messages in thread
From: Matthew Wilcox @ 2021-05-11 23:03 UTC (permalink / raw)
To: Miaohe Lin
Cc: akpm, ziy, william.kucharski, yang.shi, aneesh.kumar, rcampbell,
songliubraving, kirill.shutemov, riel, hannes, minchan, hughd,
adobriyan, linux-kernel, linux-mm, linux-fsdevel
On Tue, May 11, 2021 at 09:48:54PM +0800, Miaohe Lin wrote:
> Now that we can represent the location of ->deferred_list instead of
> ->mapping + ->index, make use of it to improve readability.
>
> Reviewed-by: Yang Shi <shy828301@gmail.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
> mm/huge_memory.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 63ed6b25deaa..76ca1eb2a223 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2868,7 +2868,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
> spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
> /* Take pin on all head pages to avoid freeing them under us */
> list_for_each_safe(pos, next, &ds_queue->split_queue) {
> - page = list_entry((void *)pos, struct page, mapping);
> + page = list_entry((void *)pos, struct page, deferred_list);
> page = compound_head(page);
This is an equivalent transformation, but it doesn't really go far
enough. I think you want something like this:
struct page *page, *next;
list_for_each_entry_safe(page, next, &ds_queue->split_queue,
deferred_list) {
struct page *head = page - 1;
... then use head throughout ...
}
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 2/5] mm/huge_memory.c: use page->deferred_list
2021-05-11 23:03 ` Matthew Wilcox
@ 2021-05-12 2:02 ` Miaohe Lin
0 siblings, 0 replies; 10+ messages in thread
From: Miaohe Lin @ 2021-05-12 2:02 UTC (permalink / raw)
To: Matthew Wilcox
Cc: akpm, ziy, william.kucharski, yang.shi, aneesh.kumar, rcampbell,
songliubraving, kirill.shutemov, riel, hannes, minchan, hughd,
adobriyan, linux-kernel, linux-mm, linux-fsdevel
On 2021/5/12 7:03, Matthew Wilcox wrote:
> On Tue, May 11, 2021 at 09:48:54PM +0800, Miaohe Lin wrote:
>> Now that we can represent the location of ->deferred_list instead of
>> ->mapping + ->index, make use of it to improve readability.
>>
>> Reviewed-by: Yang Shi <shy828301@gmail.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>> mm/huge_memory.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 63ed6b25deaa..76ca1eb2a223 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2868,7 +2868,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
>> spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
>> /* Take pin on all head pages to avoid freeing them under us */
>> list_for_each_safe(pos, next, &ds_queue->split_queue) {
>> - page = list_entry((void *)pos, struct page, mapping);
>> + page = list_entry((void *)pos, struct page, deferred_list);
>> page = compound_head(page);
>
> This is an equivalent transformation, but it doesn't really go far
> enough. I think you want something like this:
>
> struct page *page, *next;
>
> list_for_each_entry_safe(page, next, &ds_queue->split_queue,
> deferred_list) {
> struct page *head = page - 1;
> ... then use head throughout ...
> }
>
Many thanks for your time and reminder. list_for_each_entry_safe is equivalent
to list_for_each_safe + list_entry and there is many places using list_for_each_safe
+ list_entry, so I think it's ok to keep the code as it is.
Thanks again. :)
> .
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/5] mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled()
2021-05-11 13:48 [PATCH v3 0/5] Cleanup and fixup for huge_memory Miaohe Lin
2021-05-11 13:48 ` [PATCH v3 1/5] mm/huge_memory.c: remove dedicated macro HPAGE_CACHE_INDEX_MASK Miaohe Lin
2021-05-11 13:48 ` [PATCH v3 2/5] mm/huge_memory.c: use page->deferred_list Miaohe Lin
@ 2021-05-11 13:48 ` Miaohe Lin
2021-05-13 21:30 ` Yang Shi
2021-05-11 13:48 ` [PATCH v3 4/5] mm/huge_memory.c: remove unnecessary tlb_remove_page_size() for huge zero pmd Miaohe Lin
2021-05-11 13:48 ` [PATCH v3 5/5] mm/huge_memory.c: don't discard hugepage if other processes are mapping it Miaohe Lin
4 siblings, 1 reply; 10+ messages in thread
From: Miaohe Lin @ 2021-05-11 13:48 UTC (permalink / raw)
To: akpm
Cc: ziy, william.kucharski, willy, yang.shi, aneesh.kumar, rcampbell,
songliubraving, kirill.shutemov, riel, hannes, minchan, hughd,
adobriyan, linux-kernel, linux-mm, linux-fsdevel, linmiaohe
Since commit 99cb0dbd47a1 ("mm,thp: add read-only THP support for
(non-shmem) FS"), read-only THP file mapping is supported. But it
forgot to add checking for it in transparent_hugepage_enabled().
To fix it, we add checking for read-only THP file mapping and also
introduce helper transhuge_vma_enabled() to check whether thp is
enabled for specified vma to reduce duplicated code. We rename
transparent_hugepage_enabled to transparent_hugepage_active to make
the code easier to follow as suggested by David Hildenbrand.
Fixes: 99cb0dbd47a1 ("mm,thp: add read-only THP support for (non-shmem) FS")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
fs/proc/task_mmu.c | 2 +-
include/linux/huge_mm.h | 27 ++++++++++++++++++++-------
mm/huge_memory.c | 11 ++++++++++-
mm/khugepaged.c | 4 +---
mm/shmem.c | 3 +--
5 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index fc9784544b24..7389df326edd 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -832,7 +832,7 @@ static int show_smap(struct seq_file *m, void *v)
__show_smap(m, &mss, false);
seq_printf(m, "THPeligible: %d\n",
- transparent_hugepage_enabled(vma));
+ transparent_hugepage_active(vma));
if (arch_pkeys_enabled())
seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 0a526f211fec..a35c13d1f487 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -115,9 +115,19 @@ extern struct kobj_attribute shmem_enabled_attr;
extern unsigned long transparent_hugepage_flags;
+static inline bool transhuge_vma_enabled(struct vm_area_struct *vma,
+ unsigned long vm_flags)
+{
+ /* Explicitly disabled through madvise. */
+ if ((vm_flags & VM_NOHUGEPAGE) ||
+ test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
+ return false;
+ return true;
+}
+
/*
* to be used on vmas which are known to support THP.
- * Use transparent_hugepage_enabled otherwise
+ * Use transparent_hugepage_active otherwise
*/
static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
{
@@ -128,15 +138,12 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_NEVER_DAX))
return false;
- if (vma->vm_flags & VM_NOHUGEPAGE)
+ if (!transhuge_vma_enabled(vma, vma->vm_flags))
return false;
if (vma_is_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;
@@ -150,7 +157,7 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
return false;
}
-bool transparent_hugepage_enabled(struct vm_area_struct *vma);
+bool transparent_hugepage_active(struct vm_area_struct *vma);
static inline bool transhuge_vma_suitable(struct vm_area_struct *vma,
unsigned long haddr)
@@ -351,7 +358,7 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
return false;
}
-static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
+static inline bool transparent_hugepage_active(struct vm_area_struct *vma)
{
return false;
}
@@ -362,6 +369,12 @@ static inline bool transhuge_vma_suitable(struct vm_area_struct *vma,
return false;
}
+static inline bool transhuge_vma_enabled(struct vm_area_struct *vma,
+ unsigned long vm_flags)
+{
+ return false;
+}
+
static inline void prep_transhuge_page(struct page *page) {}
static inline bool is_transparent_hugepage(struct page *page)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 76ca1eb2a223..4f37867eed12 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -63,7 +63,14 @@ static struct shrinker deferred_split_shrinker;
static atomic_t huge_zero_refcount;
struct page *huge_zero_page __read_mostly;
-bool transparent_hugepage_enabled(struct vm_area_struct *vma)
+static inline bool file_thp_enabled(struct vm_area_struct *vma)
+{
+ return transhuge_vma_enabled(vma, vma->vm_flags) && vma->vm_file &&
+ !inode_is_open_for_write(vma->vm_file->f_inode) &&
+ (vma->vm_flags & VM_EXEC);
+}
+
+bool transparent_hugepage_active(struct vm_area_struct *vma)
{
/* The addr is used to check if the vma size fits */
unsigned long addr = (vma->vm_end & HPAGE_PMD_MASK) - HPAGE_PMD_SIZE;
@@ -74,6 +81,8 @@ bool transparent_hugepage_enabled(struct vm_area_struct *vma)
return __transparent_hugepage_enabled(vma);
if (vma_is_shmem(vma))
return shmem_huge_enabled(vma);
+ if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS))
+ return file_thp_enabled(vma);
return false;
}
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 6c0185fdd815..d97b20fad6e8 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -442,9 +442,7 @@ static inline int khugepaged_test_exit(struct mm_struct *mm)
static bool hugepage_vma_check(struct vm_area_struct *vma,
unsigned long vm_flags)
{
- /* Explicitly disabled through madvise. */
- if ((vm_flags & VM_NOHUGEPAGE) ||
- test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
+ if (!transhuge_vma_enabled(vma, vm_flags))
return false;
/* Enabled via shmem mount options or sysfs settings. */
diff --git a/mm/shmem.c b/mm/shmem.c
index a08cedefbfaa..1dcbec313c70 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -4032,8 +4032,7 @@ bool shmem_huge_enabled(struct vm_area_struct *vma)
loff_t i_size;
pgoff_t off;
- if ((vma->vm_flags & VM_NOHUGEPAGE) ||
- test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
+ if (!transhuge_vma_enabled(vma, vma->vm_flags))
return false;
if (shmem_huge == SHMEM_HUGE_FORCE)
return true;
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 3/5] mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled()
2021-05-11 13:48 ` [PATCH v3 3/5] mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled() Miaohe Lin
@ 2021-05-13 21:30 ` Yang Shi
2021-05-14 1:53 ` Miaohe Lin
0 siblings, 1 reply; 10+ messages in thread
From: Yang Shi @ 2021-05-13 21:30 UTC (permalink / raw)
To: Miaohe Lin
Cc: Andrew Morton, Zi Yan, william.kucharski, Matthew Wilcox,
Yang Shi, aneesh.kumar, Ralph Campbell, Song Liu,
Kirill A. Shutemov, Rik van Riel, Johannes Weiner, Minchan Kim,
Hugh Dickins, adobriyan, Linux Kernel Mailing List, Linux MM,
Linux FS-devel Mailing List
On Tue, May 11, 2021 at 6:49 AM Miaohe Lin <linmiaohe@huawei.com> wrote:
>
> Since commit 99cb0dbd47a1 ("mm,thp: add read-only THP support for
> (non-shmem) FS"), read-only THP file mapping is supported. But it
> forgot to add checking for it in transparent_hugepage_enabled().
> To fix it, we add checking for read-only THP file mapping and also
> introduce helper transhuge_vma_enabled() to check whether thp is
> enabled for specified vma to reduce duplicated code. We rename
> transparent_hugepage_enabled to transparent_hugepage_active to make
> the code easier to follow as suggested by David Hildenbrand.
>
> Fixes: 99cb0dbd47a1 ("mm,thp: add read-only THP support for (non-shmem) FS")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Looks correct to me. Reviewed-by: Yang Shi <shy828301@gmail.com>
Just a nit below:
> ---
> fs/proc/task_mmu.c | 2 +-
> include/linux/huge_mm.h | 27 ++++++++++++++++++++-------
> mm/huge_memory.c | 11 ++++++++++-
> mm/khugepaged.c | 4 +---
> mm/shmem.c | 3 +--
> 5 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index fc9784544b24..7389df326edd 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -832,7 +832,7 @@ static int show_smap(struct seq_file *m, void *v)
> __show_smap(m, &mss, false);
>
> seq_printf(m, "THPeligible: %d\n",
> - transparent_hugepage_enabled(vma));
> + transparent_hugepage_active(vma));
>
> if (arch_pkeys_enabled())
> seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 0a526f211fec..a35c13d1f487 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -115,9 +115,19 @@ extern struct kobj_attribute shmem_enabled_attr;
>
> extern unsigned long transparent_hugepage_flags;
>
> +static inline bool transhuge_vma_enabled(struct vm_area_struct *vma,
I'd like to have this function defined next to transhuge_vma_suitable().
> + unsigned long vm_flags)
> +{
> + /* Explicitly disabled through madvise. */
> + if ((vm_flags & VM_NOHUGEPAGE) ||
> + test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
> + return false;
> + return true;
> +}
> +
> /*
> * to be used on vmas which are known to support THP.
> - * Use transparent_hugepage_enabled otherwise
> + * Use transparent_hugepage_active otherwise
> */
> static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
> {
> @@ -128,15 +138,12 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
> if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_NEVER_DAX))
> return false;
>
> - if (vma->vm_flags & VM_NOHUGEPAGE)
> + if (!transhuge_vma_enabled(vma, vma->vm_flags))
> return false;
>
> if (vma_is_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;
>
> @@ -150,7 +157,7 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
> return false;
> }
>
> -bool transparent_hugepage_enabled(struct vm_area_struct *vma);
> +bool transparent_hugepage_active(struct vm_area_struct *vma);
>
> static inline bool transhuge_vma_suitable(struct vm_area_struct *vma,
> unsigned long haddr)
> @@ -351,7 +358,7 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
> return false;
> }
>
> -static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +static inline bool transparent_hugepage_active(struct vm_area_struct *vma)
> {
> return false;
> }
> @@ -362,6 +369,12 @@ static inline bool transhuge_vma_suitable(struct vm_area_struct *vma,
> return false;
> }
>
> +static inline bool transhuge_vma_enabled(struct vm_area_struct *vma,
> + unsigned long vm_flags)
> +{
> + return false;
> +}
> +
> static inline void prep_transhuge_page(struct page *page) {}
>
> static inline bool is_transparent_hugepage(struct page *page)
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 76ca1eb2a223..4f37867eed12 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -63,7 +63,14 @@ static struct shrinker deferred_split_shrinker;
> static atomic_t huge_zero_refcount;
> struct page *huge_zero_page __read_mostly;
>
> -bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> +static inline bool file_thp_enabled(struct vm_area_struct *vma)
> +{
> + return transhuge_vma_enabled(vma, vma->vm_flags) && vma->vm_file &&
> + !inode_is_open_for_write(vma->vm_file->f_inode) &&
> + (vma->vm_flags & VM_EXEC);
> +}
> +
> +bool transparent_hugepage_active(struct vm_area_struct *vma)
> {
> /* The addr is used to check if the vma size fits */
> unsigned long addr = (vma->vm_end & HPAGE_PMD_MASK) - HPAGE_PMD_SIZE;
> @@ -74,6 +81,8 @@ bool transparent_hugepage_enabled(struct vm_area_struct *vma)
> return __transparent_hugepage_enabled(vma);
> if (vma_is_shmem(vma))
> return shmem_huge_enabled(vma);
> + if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS))
> + return file_thp_enabled(vma);
>
> return false;
> }
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 6c0185fdd815..d97b20fad6e8 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -442,9 +442,7 @@ static inline int khugepaged_test_exit(struct mm_struct *mm)
> static bool hugepage_vma_check(struct vm_area_struct *vma,
> unsigned long vm_flags)
> {
> - /* Explicitly disabled through madvise. */
> - if ((vm_flags & VM_NOHUGEPAGE) ||
> - test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
> + if (!transhuge_vma_enabled(vma, vm_flags))
> return false;
>
> /* Enabled via shmem mount options or sysfs settings. */
> diff --git a/mm/shmem.c b/mm/shmem.c
> index a08cedefbfaa..1dcbec313c70 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -4032,8 +4032,7 @@ bool shmem_huge_enabled(struct vm_area_struct *vma)
> loff_t i_size;
> pgoff_t off;
>
> - if ((vma->vm_flags & VM_NOHUGEPAGE) ||
> - test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
> + if (!transhuge_vma_enabled(vma, vma->vm_flags))
> return false;
> if (shmem_huge == SHMEM_HUGE_FORCE)
> return true;
> --
> 2.23.0
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 3/5] mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled()
2021-05-13 21:30 ` Yang Shi
@ 2021-05-14 1:53 ` Miaohe Lin
0 siblings, 0 replies; 10+ messages in thread
From: Miaohe Lin @ 2021-05-14 1:53 UTC (permalink / raw)
To: Yang Shi
Cc: Andrew Morton, Zi Yan, william.kucharski, Matthew Wilcox,
Yang Shi, aneesh.kumar, Ralph Campbell, Song Liu,
Kirill A. Shutemov, Rik van Riel, Johannes Weiner, Minchan Kim,
Hugh Dickins, adobriyan, Linux Kernel Mailing List, Linux MM,
Linux FS-devel Mailing List
On 2021/5/14 5:30, Yang Shi wrote:
> On Tue, May 11, 2021 at 6:49 AM Miaohe Lin <linmiaohe@huawei.com> wrote:
>>
>> Since commit 99cb0dbd47a1 ("mm,thp: add read-only THP support for
>> (non-shmem) FS"), read-only THP file mapping is supported. But it
>> forgot to add checking for it in transparent_hugepage_enabled().
>> To fix it, we add checking for read-only THP file mapping and also
>> introduce helper transhuge_vma_enabled() to check whether thp is
>> enabled for specified vma to reduce duplicated code. We rename
>> transparent_hugepage_enabled to transparent_hugepage_active to make
>> the code easier to follow as suggested by David Hildenbrand.
>>
>> Fixes: 99cb0dbd47a1 ("mm,thp: add read-only THP support for (non-shmem) FS")
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>
> Looks correct to me. Reviewed-by: Yang Shi <shy828301@gmail.com>
>
Many thanks for your Reviewed-by tag.
> Just a nit below:
>
>> ---
>> fs/proc/task_mmu.c | 2 +-
>> include/linux/huge_mm.h | 27 ++++++++++++++++++++-------
>> mm/huge_memory.c | 11 ++++++++++-
>> mm/khugepaged.c | 4 +---
>> mm/shmem.c | 3 +--
>> 5 files changed, 33 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>> index fc9784544b24..7389df326edd 100644
>> --- a/fs/proc/task_mmu.c
>> +++ b/fs/proc/task_mmu.c
>> @@ -832,7 +832,7 @@ static int show_smap(struct seq_file *m, void *v)
>> __show_smap(m, &mss, false);
>>
>> seq_printf(m, "THPeligible: %d\n",
>> - transparent_hugepage_enabled(vma));
>> + transparent_hugepage_active(vma));
>>
>> if (arch_pkeys_enabled())
>> seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index 0a526f211fec..a35c13d1f487 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -115,9 +115,19 @@ extern struct kobj_attribute shmem_enabled_attr;
>>
>> extern unsigned long transparent_hugepage_flags;
>>
>> +static inline bool transhuge_vma_enabled(struct vm_area_struct *vma,
>
> I'd like to have this function defined next to transhuge_vma_suitable().
>
Sounds reasonable. Will do. Thanks!
>> + unsigned long vm_flags)
>> +{
>> + /* Explicitly disabled through madvise. */
>> + if ((vm_flags & VM_NOHUGEPAGE) ||
>> + test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
>> + return false;
>> + return true;
>> +}
>> +
>> /*
>> * to be used on vmas which are known to support THP.
>> - * Use transparent_hugepage_enabled otherwise
>> + * Use transparent_hugepage_active otherwise
>> */
>> static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
>> {
>> @@ -128,15 +138,12 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
>> if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_NEVER_DAX))
>> return false;
>>
>> - if (vma->vm_flags & VM_NOHUGEPAGE)
>> + if (!transhuge_vma_enabled(vma, vma->vm_flags))
>> return false;
>>
>> if (vma_is_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;
>>
>> @@ -150,7 +157,7 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
>> return false;
>> }
>>
>> -bool transparent_hugepage_enabled(struct vm_area_struct *vma);
>> +bool transparent_hugepage_active(struct vm_area_struct *vma);
>>
>> static inline bool transhuge_vma_suitable(struct vm_area_struct *vma,
>> unsigned long haddr)
>> @@ -351,7 +358,7 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
>> return false;
>> }
>>
>> -static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
>> +static inline bool transparent_hugepage_active(struct vm_area_struct *vma)
>> {
>> return false;
>> }
>> @@ -362,6 +369,12 @@ static inline bool transhuge_vma_suitable(struct vm_area_struct *vma,
>> return false;
>> }
>>
>> +static inline bool transhuge_vma_enabled(struct vm_area_struct *vma,
>> + unsigned long vm_flags)
>> +{
>> + return false;
>> +}
>> +
>> static inline void prep_transhuge_page(struct page *page) {}
>>
>> static inline bool is_transparent_hugepage(struct page *page)
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 76ca1eb2a223..4f37867eed12 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -63,7 +63,14 @@ static struct shrinker deferred_split_shrinker;
>> static atomic_t huge_zero_refcount;
>> struct page *huge_zero_page __read_mostly;
>>
>> -bool transparent_hugepage_enabled(struct vm_area_struct *vma)
>> +static inline bool file_thp_enabled(struct vm_area_struct *vma)
>> +{
>> + return transhuge_vma_enabled(vma, vma->vm_flags) && vma->vm_file &&
>> + !inode_is_open_for_write(vma->vm_file->f_inode) &&
>> + (vma->vm_flags & VM_EXEC);
>> +}
>> +
>> +bool transparent_hugepage_active(struct vm_area_struct *vma)
>> {
>> /* The addr is used to check if the vma size fits */
>> unsigned long addr = (vma->vm_end & HPAGE_PMD_MASK) - HPAGE_PMD_SIZE;
>> @@ -74,6 +81,8 @@ bool transparent_hugepage_enabled(struct vm_area_struct *vma)
>> return __transparent_hugepage_enabled(vma);
>> if (vma_is_shmem(vma))
>> return shmem_huge_enabled(vma);
>> + if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS))
>> + return file_thp_enabled(vma);
>>
>> return false;
>> }
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 6c0185fdd815..d97b20fad6e8 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -442,9 +442,7 @@ static inline int khugepaged_test_exit(struct mm_struct *mm)
>> static bool hugepage_vma_check(struct vm_area_struct *vma,
>> unsigned long vm_flags)
>> {
>> - /* Explicitly disabled through madvise. */
>> - if ((vm_flags & VM_NOHUGEPAGE) ||
>> - test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
>> + if (!transhuge_vma_enabled(vma, vm_flags))
>> return false;
>>
>> /* Enabled via shmem mount options or sysfs settings. */
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index a08cedefbfaa..1dcbec313c70 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -4032,8 +4032,7 @@ bool shmem_huge_enabled(struct vm_area_struct *vma)
>> loff_t i_size;
>> pgoff_t off;
>>
>> - if ((vma->vm_flags & VM_NOHUGEPAGE) ||
>> - test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags))
>> + if (!transhuge_vma_enabled(vma, vma->vm_flags))
>> return false;
>> if (shmem_huge == SHMEM_HUGE_FORCE)
>> return true;
>> --
>> 2.23.0
>>
>>
> .
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 4/5] mm/huge_memory.c: remove unnecessary tlb_remove_page_size() for huge zero pmd
2021-05-11 13:48 [PATCH v3 0/5] Cleanup and fixup for huge_memory Miaohe Lin
` (2 preceding siblings ...)
2021-05-11 13:48 ` [PATCH v3 3/5] mm/huge_memory.c: add missing read-only THP checking in transparent_hugepage_enabled() Miaohe Lin
@ 2021-05-11 13:48 ` Miaohe Lin
2021-05-11 13:48 ` [PATCH v3 5/5] mm/huge_memory.c: don't discard hugepage if other processes are mapping it Miaohe Lin
4 siblings, 0 replies; 10+ messages in thread
From: Miaohe Lin @ 2021-05-11 13:48 UTC (permalink / raw)
To: akpm
Cc: ziy, william.kucharski, willy, yang.shi, aneesh.kumar, rcampbell,
songliubraving, kirill.shutemov, riel, hannes, minchan, hughd,
adobriyan, linux-kernel, linux-mm, linux-fsdevel, linmiaohe
Commit aa88b68c3b1d ("thp: keep huge zero page pinned until tlb flush")
introduced tlb_remove_page() for huge zero page to keep it pinned until
flush is complete and prevents the page from being split under us. But
huge zero page is kept pinned until all relevant mm_users reach zero since
the commit 6fcb52a56ff6 ("thp: reduce usage of huge zero page's atomic
counter"). So tlb_remove_page_size() for huge zero pmd is unnecessary now.
Reviewed-by: Yang Shi <shy828301@gmail.com>
Acked-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/huge_memory.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 4f37867eed12..b8e67332806f 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1683,12 +1683,9 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
if (arch_needs_pgtable_deposit())
zap_deposited_table(tlb->mm, pmd);
spin_unlock(ptl);
- if (is_huge_zero_pmd(orig_pmd))
- tlb_remove_page_size(tlb, pmd_page(orig_pmd), HPAGE_PMD_SIZE);
} else if (is_huge_zero_pmd(orig_pmd)) {
zap_deposited_table(tlb->mm, pmd);
spin_unlock(ptl);
- tlb_remove_page_size(tlb, pmd_page(orig_pmd), HPAGE_PMD_SIZE);
} else {
struct page *page = NULL;
int flush_needed = 1;
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 5/5] mm/huge_memory.c: don't discard hugepage if other processes are mapping it
2021-05-11 13:48 [PATCH v3 0/5] Cleanup and fixup for huge_memory Miaohe Lin
` (3 preceding siblings ...)
2021-05-11 13:48 ` [PATCH v3 4/5] mm/huge_memory.c: remove unnecessary tlb_remove_page_size() for huge zero pmd Miaohe Lin
@ 2021-05-11 13:48 ` Miaohe Lin
4 siblings, 0 replies; 10+ messages in thread
From: Miaohe Lin @ 2021-05-11 13:48 UTC (permalink / raw)
To: akpm
Cc: ziy, william.kucharski, willy, yang.shi, aneesh.kumar, rcampbell,
songliubraving, kirill.shutemov, riel, hannes, minchan, hughd,
adobriyan, linux-kernel, linux-mm, linux-fsdevel, linmiaohe
If other processes are mapping any other subpages of the hugepage, i.e. in
pte-mapped thp case, page_mapcount() will return 1 incorrectly. Then we
would discard the page while other processes are still mapping it. Fix it
by using total_mapcount() which can tell whether other processes are still
mapping it.
Fixes: b8d3c4c3009d ("mm/huge_memory.c: don't split THP page when MADV_FREE syscall is called")
Reviewed-by: Yang Shi <shy828301@gmail.com>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
mm/huge_memory.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index b8e67332806f..24647fe076b8 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1610,7 +1610,7 @@ bool madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
* If other processes are mapping this page, we couldn't discard
* the page unless they all do MADV_FREE so let's skip the page.
*/
- if (page_mapcount(page) != 1)
+ if (total_mapcount(page) != 1)
goto out;
if (!trylock_page(page))
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread