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.
