Re: [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring

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

 



Hi SeongJae,

I think there are potential memory leaks in the following execution paths

+static bool damon_page_mkold(struct page *page, struct vm_area_struct *vma,
+               unsigned long addr, void *arg)
+{
+       damon_mkold(vma->vm_mm, addr);
+       return true;
+}
+
+static void damon_phys_mkold(unsigned long paddr)
+{
+       struct page *page = damon_phys_get_page(PHYS_PFN(paddr));
get_page() here 
+       struct rmap_walk_control rwc = {
+               .rmap_one = damon_page_mkold,
+               .anon_lock = page_lock_anon_vma_read,
+       };
+       bool need_lock;
+
+       if (!page)
+               return;
+
+       if (!page_mapped(page) || !page_rmapping(page))
+               return;
return without put_page() 
+
+       need_lock = !PageAnon(page) || PageKsm(page);
+       if (need_lock && !trylock_page(page))
+               return;
+
+       rmap_walk(page, &rwc);
+
+       if (need_lock)
+               unlock_page(page);
+       put_page(page);
+}
+
+static bool damon_phys_young(unsigned long paddr, unsigned long *page_sz)
+{
+       struct page *page = damon_phys_get_page(PHYS_PFN(paddr));
get_page() here 
+       struct damon_phys_access_chk_result result = {
+               .page_sz = PAGE_SIZE,
+               .accessed = false,
+       };
+       struct rmap_walk_control rwc = {
+               .arg = &result,
+               .rmap_one = damon_page_accessed,
+               .anon_lock = page_lock_anon_vma_read,
+       };
+       bool need_lock;
+
+       if (!page)
+               return false;
+
+       if (!page_mapped(page) || !page_rmapping(page))
+               return false;
return without put_page()  
+
+       need_lock = !PageAnon(page) || PageKsm(page);
+       if (need_lock && !trylock_page(page))
+               return false;
+
+       rmap_walk(page, &rwc);
+
+       if (need_lock)
+               unlock_page(page);
+       put_page(page);
+
+       *page_sz = result.page_sz;
+       return result.accessed;
+}
I observed the memory leak problem by running your userprog [1] in a kvm vm. Compare /proc/meminfo before and after running damon + userprog for about 30min then I observe a noticeable amount of DRAM is not freed even if userprog exits.
I tried to add two put_page() and the problem went away. I am not exactly sure whether adding a put_page before return is the desired behavior, nor can I think of a case to explain why put_page is necessary here for "unmapped" pages.

[1] https://github.com/sjp38/idle_page_tracking/blob/master/userprog.c

[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