Re: [PATCH] mm: khugepaged: fix call hpage_collapse_scan_file() for anonymous vma

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






On 1/9/25 8:31 PM, Matthew Wilcox wrote:
On Thu, Jan 09, 2025 at 09:00:24AM -0800, Yang Shi wrote:
Thanks for catching this. It sounds a little bit weird to have vm_file for
an anonymous VMA. I'm not sure why we should keep such special case. It
seems shared mapping is treated as shmem file mapping. So can we set vm_file
to NULL when mmap'ing /dev/zero for private mapping? Something like:

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 169eed162a7f..fc332efc5c11 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -527,6 +527,7 @@ static int mmap_zero(struct file *file, struct
vm_area_struct *vma)
         if (vma->vm_flags & VM_SHARED)
                 return shmem_zero_setup(vma);
         vma_set_anonymous(vma);
+       vma->vm_file = NULL;
         return 0;
  }
I'm wary this might cause other bugs somewhere.  rc6 is a bit late to be
introducing such a subtle change.

Thanks for the extra caution. Applying the proposed fix in khugepaged code is fine to me either. We can try to kill the special case later.

Looking at the code further, I think we should do more to make private /dev/zero mapping an anonymous mapping:

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 169eed162a7f..98cfac2bb01f 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -527,6 +527,10 @@ static int mmap_zero(struct file *file, struct vm_area_struct *vma)
        if (vma->vm_flags & VM_SHARED)
                return shmem_zero_setup(vma);
        vma_set_anonymous(vma);
+       fput(vma->vm_file);
+       vma->vm_file = NULL;
+       vma->vm_pgoff = vma->vm_start >> PAGE_SHIFT;
+
        return 0;
 }


AFAICT, the user visible effect is we will have different entry in smaps/maps.

Before the change:
ffffb7190000-ffffb7590000 rw-p 00001000 00:06 8                          /dev/zero
Size:               4096 kB
KernelPageSize:        4 kB
MMUPageSize:           4 kB
Rss:                   4 kB
Pss:                   4 kB
Pss_Dirty:             4 kB
Shared_Clean:          0 kB
Shared_Dirty:          0 kB
Private_Clean:         0 kB
Private_Dirty:         4 kB
Referenced:            4 kB
Anonymous:             4 kB
KSM:                   0 kB
LazyFree:              0 kB
AnonHugePages:         0 kB
ShmemPmdMapped:        0 kB
FilePmdMapped:         0 kB
Shared_Hugetlb:        0 kB
Private_Hugetlb:       0 kB
Swap:                  0 kB
SwapPss:               0 kB
Locked:                0 kB
THPeligible:           0
VmFlags: rd wr mr mw me ac

After the change:
ffffb6130000-ffffb6530000 rw-p 00000000 00:00 0
Size:               4096 kB
KernelPageSize:        4 kB
MMUPageSize:           4 kB
Rss:                   4 kB
Pss:                   4 kB
Pss_Dirty:             4 kB
Shared_Clean:          0 kB
Shared_Dirty:          0 kB
Private_Clean:         0 kB
Private_Dirty:         4 kB
Referenced:            4 kB
Anonymous:             4 kB
KSM:                   0 kB
LazyFree:              0 kB
AnonHugePages:         0 kB
ShmemPmdMapped:        0 kB
FilePmdMapped:         0 kB
Shared_Hugetlb:        0 kB
Private_Hugetlb:       0 kB
Swap:                  0 kB
SwapPss:               0 kB
Locked:                0 kB
THPeligible:           0
VmFlags: rd wr mr mw me ac

I'm not sure who really cares about the difference.






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux