* [PATCH v2] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma
@ 2025-01-11 3:45 Liu Shixin
2025-01-13 18:52 ` Yang Shi
2025-01-14 16:56 ` David Hildenbrand
0 siblings, 2 replies; 6+ messages in thread
From: Liu Shixin @ 2025-01-11 3:45 UTC (permalink / raw)
To: Andrew Morton, Chengming Zhou, Matthew Wilcox, Kefeng Wang,
Nanyong Sun, Muchun Song, Qi Zheng, Johannes Weiner, Yang Shi
Cc: linux-mm, linux-kernel, Liu Shixin
syzkaller reported such a BUG_ON():
------------[ cut here ]------------
kernel BUG at mm/khugepaged.c:1835!
Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
...
CPU: 6 UID: 0 PID: 8009 Comm: syz.15.106 Kdump: loaded Tainted: G W 6.13.0-rc6 #22
Tainted: [W]=WARN
Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : collapse_file+0xa44/0x1400
lr : collapse_file+0x88/0x1400
sp : ffff80008afe3a60
...
Call trace:
collapse_file+0xa44/0x1400 (P)
hpage_collapse_scan_file+0x278/0x400
madvise_collapse+0x1bc/0x678
madvise_vma_behavior+0x32c/0x448
madvise_walk_vmas.constprop.0+0xbc/0x140
do_madvise.part.0+0xdc/0x2c8
__arm64_sys_madvise+0x68/0x88
invoke_syscall+0x50/0x120
el0_svc_common.constprop.0+0xc8/0xf0
do_el0_svc+0x24/0x38
el0_svc+0x34/0x128
el0t_64_sync_handler+0xc8/0xd0
el0t_64_sync+0x190/0x198
This indicates that the pgoff is unaligned. After analysis, I confirm
the vma is mapped to /dev/zero. Such a vma certainly has vm_file, but
it is set to anonymous by mmap_zero(). So even if it's mmapped by
2m-unaligned, it can pass the check in thp_vma_allowable_order() as it
is an anonymous-mmap, but then be collapsed as a file-mmap.
It seems the problem has existed for a long time, but actually, since
we have khugepaged_max_ptes_none check before, we will skip collapse it
as it is /dev/zero and so has no present page. But commit d8ea7cc8547c
limit the check for only khugepaged, so the BUG_ON() can be triggered
by madvise_collapse().
Add vma_is_anonymous() check to make such vma be processed by
hpage_collapse_scan_pmd().
Fixes: d8ea7cc8547c ("mm/khugepaged: add flag to predicate khugepaged-only behavior")
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
v1->v2: Remove the redundant vm_file check, suggested by Matthew Wilcox.
mm/khugepaged.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 653dbb1ff05c..bad1e130eda8 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2422,7 +2422,7 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
VM_BUG_ON(khugepaged_scan.address < hstart ||
khugepaged_scan.address + HPAGE_PMD_SIZE >
hend);
- if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
+ if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
struct file *file = get_file(vma->vm_file);
pgoff_t pgoff = linear_page_index(vma,
khugepaged_scan.address);
@@ -2768,7 +2768,7 @@ int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev,
mmap_assert_locked(mm);
memset(cc->node_load, 0, sizeof(cc->node_load));
nodes_clear(cc->alloc_nmask);
- if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
+ if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
struct file *file = get_file(vma->vm_file);
pgoff_t pgoff = linear_page_index(vma, addr);
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma
2025-01-11 3:45 [PATCH v2] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma Liu Shixin
@ 2025-01-13 18:52 ` Yang Shi
2025-01-14 16:56 ` David Hildenbrand
1 sibling, 0 replies; 6+ messages in thread
From: Yang Shi @ 2025-01-13 18:52 UTC (permalink / raw)
To: Liu Shixin, Andrew Morton, Chengming Zhou, Matthew Wilcox,
Kefeng Wang, Nanyong Sun, Muchun Song, Qi Zheng, Johannes Weiner
Cc: linux-mm, linux-kernel
On 1/10/25 7:45 PM, Liu Shixin wrote:
> syzkaller reported such a BUG_ON():
>
> ------------[ cut here ]------------
> kernel BUG at mm/khugepaged.c:1835!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> ...
> CPU: 6 UID: 0 PID: 8009 Comm: syz.15.106 Kdump: loaded Tainted: G W 6.13.0-rc6 #22
> Tainted: [W]=WARN
> Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
> pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : collapse_file+0xa44/0x1400
> lr : collapse_file+0x88/0x1400
> sp : ffff80008afe3a60
> ...
> Call trace:
> collapse_file+0xa44/0x1400 (P)
> hpage_collapse_scan_file+0x278/0x400
> madvise_collapse+0x1bc/0x678
> madvise_vma_behavior+0x32c/0x448
> madvise_walk_vmas.constprop.0+0xbc/0x140
> do_madvise.part.0+0xdc/0x2c8
> __arm64_sys_madvise+0x68/0x88
> invoke_syscall+0x50/0x120
> el0_svc_common.constprop.0+0xc8/0xf0
> do_el0_svc+0x24/0x38
> el0_svc+0x34/0x128
> el0t_64_sync_handler+0xc8/0xd0
> el0t_64_sync+0x190/0x198
>
> This indicates that the pgoff is unaligned. After analysis, I confirm
> the vma is mapped to /dev/zero. Such a vma certainly has vm_file, but
> it is set to anonymous by mmap_zero(). So even if it's mmapped by
> 2m-unaligned, it can pass the check in thp_vma_allowable_order() as it
> is an anonymous-mmap, but then be collapsed as a file-mmap.
>
> It seems the problem has existed for a long time, but actually, since
> we have khugepaged_max_ptes_none check before, we will skip collapse it
> as it is /dev/zero and so has no present page. But commit d8ea7cc8547c
> limit the check for only khugepaged, so the BUG_ON() can be triggered
> by madvise_collapse().
>
> Add vma_is_anonymous() check to make such vma be processed by
> hpage_collapse_scan_pmd().
>
> Fixes: d8ea7cc8547c ("mm/khugepaged: add flag to predicate khugepaged-only behavior")
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> ---
> v1->v2: Remove the redundant vm_file check, suggested by Matthew Wilcox.
Reviewed-by: Yang Shi <yang@os.amperecomputing.com>
>
> mm/khugepaged.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 653dbb1ff05c..bad1e130eda8 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2422,7 +2422,7 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
> VM_BUG_ON(khugepaged_scan.address < hstart ||
> khugepaged_scan.address + HPAGE_PMD_SIZE >
> hend);
> - if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
> + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
> struct file *file = get_file(vma->vm_file);
> pgoff_t pgoff = linear_page_index(vma,
> khugepaged_scan.address);
> @@ -2768,7 +2768,7 @@ int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev,
> mmap_assert_locked(mm);
> memset(cc->node_load, 0, sizeof(cc->node_load));
> nodes_clear(cc->alloc_nmask);
> - if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
> + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
> struct file *file = get_file(vma->vm_file);
> pgoff_t pgoff = linear_page_index(vma, addr);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma
2025-01-11 3:45 [PATCH v2] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma Liu Shixin
2025-01-13 18:52 ` Yang Shi
@ 2025-01-14 16:56 ` David Hildenbrand
2025-01-14 17:56 ` Yang Shi
2025-01-14 18:00 ` Yang Shi
1 sibling, 2 replies; 6+ messages in thread
From: David Hildenbrand @ 2025-01-14 16:56 UTC (permalink / raw)
To: Liu Shixin, Andrew Morton, Chengming Zhou, Matthew Wilcox,
Kefeng Wang, Nanyong Sun, Muchun Song, Qi Zheng, Johannes Weiner,
Yang Shi
Cc: linux-mm, linux-kernel
On 11.01.25 04:45, Liu Shixin wrote:
> syzkaller reported such a BUG_ON():
>
> ------------[ cut here ]------------
> kernel BUG at mm/khugepaged.c:1835!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> ...
> CPU: 6 UID: 0 PID: 8009 Comm: syz.15.106 Kdump: loaded Tainted: G W 6.13.0-rc6 #22
> Tainted: [W]=WARN
> Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
> pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : collapse_file+0xa44/0x1400
> lr : collapse_file+0x88/0x1400
> sp : ffff80008afe3a60
> ...
> Call trace:
> collapse_file+0xa44/0x1400 (P)
> hpage_collapse_scan_file+0x278/0x400
> madvise_collapse+0x1bc/0x678
> madvise_vma_behavior+0x32c/0x448
> madvise_walk_vmas.constprop.0+0xbc/0x140
> do_madvise.part.0+0xdc/0x2c8
> __arm64_sys_madvise+0x68/0x88
> invoke_syscall+0x50/0x120
> el0_svc_common.constprop.0+0xc8/0xf0
> do_el0_svc+0x24/0x38
> el0_svc+0x34/0x128
> el0t_64_sync_handler+0xc8/0xd0
> el0t_64_sync+0x190/0x198
>
> This indicates that the pgoff is unaligned. After analysis, I confirm
> the vma is mapped to /dev/zero. Such a vma certainly has vm_file, but
> it is set to anonymous by mmap_zero(). So even if it's mmapped by
> 2m-unaligned, it can pass the check in thp_vma_allowable_order() as it
> is an anonymous-mmap, but then be collapsed as a file-mmap.
>
> It seems the problem has existed for a long time, but actually, since
> we have khugepaged_max_ptes_none check before, we will skip collapse it
> as it is /dev/zero and so has no present page. But commit d8ea7cc8547c
> limit the check for only khugepaged, so the BUG_ON() can be triggered
> by madvise_collapse().
>
> Add vma_is_anonymous() check to make such vma be processed by
> hpage_collapse_scan_pmd().
>
> Fixes: d8ea7cc8547c ("mm/khugepaged: add flag to predicate khugepaged-only behavior")
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> ---
> v1->v2: Remove the redundant vm_file check, suggested by Matthew Wilcox.
>
> mm/khugepaged.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 653dbb1ff05c..bad1e130eda8 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2422,7 +2422,7 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
> VM_BUG_ON(khugepaged_scan.address < hstart ||
> khugepaged_scan.address + HPAGE_PMD_SIZE >
> hend);
> - if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
> + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
> struct file *file = get_file(vma->vm_file);
> pgoff_t pgoff = linear_page_index(vma,
> khugepaged_scan.address);
> @@ -2768,7 +2768,7 @@ int madvise_collapse(struct vm_area_struct *vma, struct vm_area_struct **prev,
> mmap_assert_locked(mm);
> memset(cc->node_load, 0, sizeof(cc->node_load));
> nodes_clear(cc->alloc_nmask);
> - if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
> + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
> struct file *file = get_file(vma->vm_file);
> pgoff_t pgoff = linear_page_index(vma, addr);
>
Acked-by: David Hildenbrand <david@redhat.com>
The whole CONFIG_SHMEM is all weird and needs to be cleaned up ... at
some point.
BTW, do we correctly handle MAP_PRIVATE of these files, where we would
have vma->vm_file and !vma_is_anonymous(vma), but could end up having
anonymous pages in there?
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma
2025-01-14 16:56 ` David Hildenbrand
@ 2025-01-14 17:56 ` Yang Shi
2025-01-14 18:00 ` Yang Shi
1 sibling, 0 replies; 6+ messages in thread
From: Yang Shi @ 2025-01-14 17:56 UTC (permalink / raw)
To: David Hildenbrand, Liu Shixin, Andrew Morton, Chengming Zhou,
Matthew Wilcox, Kefeng Wang, Nanyong Sun, Muchun Song, Qi Zheng,
Johannes Weiner
Cc: linux-mm, linux-kernel
On 1/14/25 8:56 AM, David Hildenbrand wrote:
> On 11.01.25 04:45, Liu Shixin wrote:
>> syzkaller reported such a BUG_ON():
>>
>> ------------[ cut here ]------------
>> kernel BUG at mm/khugepaged.c:1835!
>> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
>> ...
>> CPU: 6 UID: 0 PID: 8009 Comm: syz.15.106 Kdump: loaded Tainted:
>> G W 6.13.0-rc6 #22
>> Tainted: [W]=WARN
>> Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
>> pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : collapse_file+0xa44/0x1400
>> lr : collapse_file+0x88/0x1400
>> sp : ffff80008afe3a60
>> ...
>> Call trace:
>> collapse_file+0xa44/0x1400 (P)
>> hpage_collapse_scan_file+0x278/0x400
>> madvise_collapse+0x1bc/0x678
>> madvise_vma_behavior+0x32c/0x448
>> madvise_walk_vmas.constprop.0+0xbc/0x140
>> do_madvise.part.0+0xdc/0x2c8
>> __arm64_sys_madvise+0x68/0x88
>> invoke_syscall+0x50/0x120
>> el0_svc_common.constprop.0+0xc8/0xf0
>> do_el0_svc+0x24/0x38
>> el0_svc+0x34/0x128
>> el0t_64_sync_handler+0xc8/0xd0
>> el0t_64_sync+0x190/0x198
>>
>> This indicates that the pgoff is unaligned. After analysis, I confirm
>> the vma is mapped to /dev/zero. Such a vma certainly has vm_file, but
>> it is set to anonymous by mmap_zero(). So even if it's mmapped by
>> 2m-unaligned, it can pass the check in thp_vma_allowable_order() as it
>> is an anonymous-mmap, but then be collapsed as a file-mmap.
>>
>> It seems the problem has existed for a long time, but actually, since
>> we have khugepaged_max_ptes_none check before, we will skip collapse it
>> as it is /dev/zero and so has no present page. But commit d8ea7cc8547c
>> limit the check for only khugepaged, so the BUG_ON() can be triggered
>> by madvise_collapse().
>>
>> Add vma_is_anonymous() check to make such vma be processed by
>> hpage_collapse_scan_pmd().
>>
>> Fixes: d8ea7cc8547c ("mm/khugepaged: add flag to predicate
>> khugepaged-only behavior")
>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>> ---
>> v1->v2: Remove the redundant vm_file check, suggested by Matthew Wilcox.
>>
>> mm/khugepaged.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 653dbb1ff05c..bad1e130eda8 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -2422,7 +2422,7 @@ static unsigned int
>> khugepaged_scan_mm_slot(unsigned int pages, int *result,
>> VM_BUG_ON(khugepaged_scan.address < hstart ||
>> khugepaged_scan.address + HPAGE_PMD_SIZE >
>> hend);
>> - if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
>> + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
>> struct file *file = get_file(vma->vm_file);
>> pgoff_t pgoff = linear_page_index(vma,
>> khugepaged_scan.address);
>> @@ -2768,7 +2768,7 @@ int madvise_collapse(struct vm_area_struct
>> *vma, struct vm_area_struct **prev,
>> mmap_assert_locked(mm);
>> memset(cc->node_load, 0, sizeof(cc->node_load));
>> nodes_clear(cc->alloc_nmask);
>> - if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
>> + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
>> struct file *file = get_file(vma->vm_file);
>> pgoff_t pgoff = linear_page_index(vma, addr);
>
> Acked-by: David Hildenbrand <david@redhat.com>
>
> The whole CONFIG_SHMEM is all weird and needs to be cleaned up ... at
> some point.
>
> BTW, do we correctly handle MAP_PRIVATE of these files, where we would
> have vma->vm_file and !vma_is_anonymous(vma), but could end up having
> anonymous pages in there?
I think so. There are a couple of cases if I don't miss something:
1. non-shmem: khugepaged just supports collapse read-only opened regular
file at this point, there should be no anonymous pages in this case
2. Shmem:
2.1. small folios in page cache: it should be fine to collapse page
cache itself even though there are anonymous pages
2.2. large folios in page cache: khugepaged should bail out from
collapse_pte_mapped_thp() if CoW'ed anonymous folio is met
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma
2025-01-14 16:56 ` David Hildenbrand
2025-01-14 17:56 ` Yang Shi
@ 2025-01-14 18:00 ` Yang Shi
2025-01-20 14:02 ` David Hildenbrand
1 sibling, 1 reply; 6+ messages in thread
From: Yang Shi @ 2025-01-14 18:00 UTC (permalink / raw)
To: David Hildenbrand, Liu Shixin, Andrew Morton, Chengming Zhou,
Matthew Wilcox, Kefeng Wang, Nanyong Sun, Muchun Song, Qi Zheng,
Johannes Weiner
Cc: linux-mm, linux-kernel
On 1/14/25 8:56 AM, David Hildenbrand wrote:
> On 11.01.25 04:45, Liu Shixin wrote:
>> syzkaller reported such a BUG_ON():
>>
>> ------------[ cut here ]------------
>> kernel BUG at mm/khugepaged.c:1835!
>> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
>> ...
>> CPU: 6 UID: 0 PID: 8009 Comm: syz.15.106 Kdump: loaded Tainted:
>> G W 6.13.0-rc6 #22
>> Tainted: [W]=WARN
>> Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
>> pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : collapse_file+0xa44/0x1400
>> lr : collapse_file+0x88/0x1400
>> sp : ffff80008afe3a60
>> ...
>> Call trace:
>> collapse_file+0xa44/0x1400 (P)
>> hpage_collapse_scan_file+0x278/0x400
>> madvise_collapse+0x1bc/0x678
>> madvise_vma_behavior+0x32c/0x448
>> madvise_walk_vmas.constprop.0+0xbc/0x140
>> do_madvise.part.0+0xdc/0x2c8
>> __arm64_sys_madvise+0x68/0x88
>> invoke_syscall+0x50/0x120
>> el0_svc_common.constprop.0+0xc8/0xf0
>> do_el0_svc+0x24/0x38
>> el0_svc+0x34/0x128
>> el0t_64_sync_handler+0xc8/0xd0
>> el0t_64_sync+0x190/0x198
>>
>> This indicates that the pgoff is unaligned. After analysis, I confirm
>> the vma is mapped to /dev/zero. Such a vma certainly has vm_file, but
>> it is set to anonymous by mmap_zero(). So even if it's mmapped by
>> 2m-unaligned, it can pass the check in thp_vma_allowable_order() as it
>> is an anonymous-mmap, but then be collapsed as a file-mmap.
>>
>> It seems the problem has existed for a long time, but actually, since
>> we have khugepaged_max_ptes_none check before, we will skip collapse it
>> as it is /dev/zero and so has no present page. But commit d8ea7cc8547c
>> limit the check for only khugepaged, so the BUG_ON() can be triggered
>> by madvise_collapse().
>>
>> Add vma_is_anonymous() check to make such vma be processed by
>> hpage_collapse_scan_pmd().
>>
>> Fixes: d8ea7cc8547c ("mm/khugepaged: add flag to predicate
>> khugepaged-only behavior")
>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>> ---
>> v1->v2: Remove the redundant vm_file check, suggested by Matthew Wilcox.
>>
>> mm/khugepaged.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 653dbb1ff05c..bad1e130eda8 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -2422,7 +2422,7 @@ static unsigned int
>> khugepaged_scan_mm_slot(unsigned int pages, int *result,
>> VM_BUG_ON(khugepaged_scan.address < hstart ||
>> khugepaged_scan.address + HPAGE_PMD_SIZE >
>> hend);
>> - if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
>> + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
>> struct file *file = get_file(vma->vm_file);
>> pgoff_t pgoff = linear_page_index(vma,
>> khugepaged_scan.address);
>> @@ -2768,7 +2768,7 @@ int madvise_collapse(struct vm_area_struct
>> *vma, struct vm_area_struct **prev,
>> mmap_assert_locked(mm);
>> memset(cc->node_load, 0, sizeof(cc->node_load));
>> nodes_clear(cc->alloc_nmask);
>> - if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
>> + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
>> struct file *file = get_file(vma->vm_file);
>> pgoff_t pgoff = linear_page_index(vma, addr);
>
> Acked-by: David Hildenbrand <david@redhat.com>
>
> The whole CONFIG_SHMEM is all weird and needs to be cleaned up ... at
> some point.
>
> BTW, do we correctly handle MAP_PRIVATE of these files, where we would
> have vma->vm_file and !vma_is_anonymous(vma), but could end up having
> anonymous pages in there?
I think so. There are a couple of cases if I don't miss something:
1. non-shmem: khugepaged just supports collapse read-only opened regular
file at this point, there should be no anonymous pages in this case
2. Shmem:
2.1. small folios in page cache: it should be fine to collapse page
cache itself even though there are anonymous pages
2.2. large folios in page cache: khugepaged should bail out from
collapse_pte_mapped_thp() if CoW'ed anonymous folio is met
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma
2025-01-14 18:00 ` Yang Shi
@ 2025-01-20 14:02 ` David Hildenbrand
0 siblings, 0 replies; 6+ messages in thread
From: David Hildenbrand @ 2025-01-20 14:02 UTC (permalink / raw)
To: Yang Shi, Liu Shixin, Andrew Morton, Chengming Zhou,
Matthew Wilcox, Kefeng Wang, Nanyong Sun, Muchun Song, Qi Zheng,
Johannes Weiner
Cc: linux-mm, linux-kernel
On 14.01.25 19:00, Yang Shi wrote:
>
>
>
> On 1/14/25 8:56 AM, David Hildenbrand wrote:
>> On 11.01.25 04:45, Liu Shixin wrote:
>>> syzkaller reported such a BUG_ON():
>>>
>>> ------------[ cut here ]------------
>>> kernel BUG at mm/khugepaged.c:1835!
>>> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
>>> ...
>>> CPU: 6 UID: 0 PID: 8009 Comm: syz.15.106 Kdump: loaded Tainted:
>>> G W 6.13.0-rc6 #22
>>> Tainted: [W]=WARN
>>> Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
>>> pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>> pc : collapse_file+0xa44/0x1400
>>> lr : collapse_file+0x88/0x1400
>>> sp : ffff80008afe3a60
>>> ...
>>> Call trace:
>>> collapse_file+0xa44/0x1400 (P)
>>> hpage_collapse_scan_file+0x278/0x400
>>> madvise_collapse+0x1bc/0x678
>>> madvise_vma_behavior+0x32c/0x448
>>> madvise_walk_vmas.constprop.0+0xbc/0x140
>>> do_madvise.part.0+0xdc/0x2c8
>>> __arm64_sys_madvise+0x68/0x88
>>> invoke_syscall+0x50/0x120
>>> el0_svc_common.constprop.0+0xc8/0xf0
>>> do_el0_svc+0x24/0x38
>>> el0_svc+0x34/0x128
>>> el0t_64_sync_handler+0xc8/0xd0
>>> el0t_64_sync+0x190/0x198
>>>
>>> This indicates that the pgoff is unaligned. After analysis, I confirm
>>> the vma is mapped to /dev/zero. Such a vma certainly has vm_file, but
>>> it is set to anonymous by mmap_zero(). So even if it's mmapped by
>>> 2m-unaligned, it can pass the check in thp_vma_allowable_order() as it
>>> is an anonymous-mmap, but then be collapsed as a file-mmap.
>>>
>>> It seems the problem has existed for a long time, but actually, since
>>> we have khugepaged_max_ptes_none check before, we will skip collapse it
>>> as it is /dev/zero and so has no present page. But commit d8ea7cc8547c
>>> limit the check for only khugepaged, so the BUG_ON() can be triggered
>>> by madvise_collapse().
>>>
>>> Add vma_is_anonymous() check to make such vma be processed by
>>> hpage_collapse_scan_pmd().
>>>
>>> Fixes: d8ea7cc8547c ("mm/khugepaged: add flag to predicate
>>> khugepaged-only behavior")
>>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>>> ---
>>> v1->v2: Remove the redundant vm_file check, suggested by Matthew Wilcox.
>>>
>>> mm/khugepaged.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index 653dbb1ff05c..bad1e130eda8 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -2422,7 +2422,7 @@ static unsigned int
>>> khugepaged_scan_mm_slot(unsigned int pages, int *result,
>>> VM_BUG_ON(khugepaged_scan.address < hstart ||
>>> khugepaged_scan.address + HPAGE_PMD_SIZE >
>>> hend);
>>> - if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
>>> + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
>>> struct file *file = get_file(vma->vm_file);
>>> pgoff_t pgoff = linear_page_index(vma,
>>> khugepaged_scan.address);
>>> @@ -2768,7 +2768,7 @@ int madvise_collapse(struct vm_area_struct
>>> *vma, struct vm_area_struct **prev,
>>> mmap_assert_locked(mm);
>>> memset(cc->node_load, 0, sizeof(cc->node_load));
>>> nodes_clear(cc->alloc_nmask);
>>> - if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
>>> + if (IS_ENABLED(CONFIG_SHMEM) && !vma_is_anonymous(vma)) {
>>> struct file *file = get_file(vma->vm_file);
>>> pgoff_t pgoff = linear_page_index(vma, addr);
>>
>> Acked-by: David Hildenbrand <david@redhat.com>
>>
>> The whole CONFIG_SHMEM is all weird and needs to be cleaned up ... at
>> some point.
>>
>> BTW, do we correctly handle MAP_PRIVATE of these files, where we would
>> have vma->vm_file and !vma_is_anonymous(vma), but could end up having
>> anonymous pages in there?
>
>
> I think so. There are a couple of cases if I don't miss something:
>
> 1. non-shmem: khugepaged just supports collapse read-only opened regular
> file at this point, there should be no anonymous pages in this case
> 2. Shmem:
> 2.1. small folios in page cache: it should be fine to collapse page
> cache itself even though there are anonymous pages
> 2.2. large folios in page cache: khugepaged should bail out from
> collapse_pte_mapped_thp() if CoW'ed anonymous folio is met
>
Great, thanks!
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-01-20 14:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-11 3:45 [PATCH v2] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma Liu Shixin
2025-01-13 18:52 ` Yang Shi
2025-01-14 16:56 ` David Hildenbrand
2025-01-14 17:56 ` Yang Shi
2025-01-14 18:00 ` Yang Shi
2025-01-20 14:02 ` David Hildenbrand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox