* Re: [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring
[not found] <CAJFQjcOCDFGq7pNgYWpKbrhBM5FQo76d274H8UU1ma_8TWjvuQ@mail.gmail.com>
@ 2020-08-28 9:53 ` SeongJae Park
2020-08-28 11:39 ` SeongJae Park
0 siblings, 1 reply; 7+ messages in thread
From: SeongJae Park @ 2020-08-28 9:53 UTC (permalink / raw)
To: Alkaid
Cc: SeongJae Park, akpm, SeongJae Park, Jonathan.Cameron, aarcange,
acme, alexander.shishkin, amit, benh, brendan.d.gregg,
brendanhiggins, cai, colin.king, corbet, david, dwmw, fan.du,
foersleo, gthelen, irogers, jolsa, kirill, mark.rutland, mgorman,
minchan, mingo, namhyung, peterz, rdunlap, riel, rientjes,
rostedt, rppt, sblbir, shakeelb, shuah, sj38.park, snu, vbabka,
vdavydov.dev, yang.shi, ying.huang, linux-damon, linux-mm,
linux-doc, linux-kernel
On Fri, 28 Aug 2020 04:11:56 -0400 Alkaid <zgf574564920@gmail.com> wrote:
>
> [-- Attachment #1: Type: text/plain, Size: 2677 bytes --]
>
> Hi SeongJae,
>
> I think there are potential memory leaks in the following execution paths
Agreed, definitely memory leaks exists. Thank you for let me know this! I
will post a patch for this soon.
Thanks,
SeongJae Park
[...]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring
2020-08-28 9:53 ` [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring SeongJae Park
@ 2020-08-28 11:39 ` SeongJae Park
0 siblings, 0 replies; 7+ messages in thread
From: SeongJae Park @ 2020-08-28 11:39 UTC (permalink / raw)
To: Alkaid
Cc: akpm, SeongJae Park, Jonathan.Cameron, aarcange, acme,
alexander.shishkin, amit, benh, brendan.d.gregg, brendanhiggins,
cai, colin.king, corbet, david, dwmw, fan.du, foersleo, gthelen,
irogers, jolsa, kirill, mark.rutland, mgorman, minchan, mingo,
namhyung, peterz, rdunlap, riel, rientjes, rostedt, rppt, sblbir,
shakeelb, shuah, sj38.park, snu, vbabka, vdavydov.dev, yang.shi,
ying.huang, linux-damon, linux-mm, linux-doc, linux-kernel,
SeongJae Park
On Fri, 28 Aug 2020 11:53:15 +0200 SeongJae Park <sjpark@amazon.com> wrote:
> On Fri, 28 Aug 2020 04:11:56 -0400 Alkaid <zgf574564920@gmail.com> wrote:
>
> >
> > [-- Attachment #1: Type: text/plain, Size: 2677 bytes --]
> >
> > Hi SeongJae,
> >
> > I think there are potential memory leaks in the following execution paths
>
> Agreed, definitely memory leaks exists. Thank you for let me know this! I
> will post a patch for this soon.
And, below is the patch. The complete tree is available at:
https://github.com/sjp38/linux/tree/damon/next
Thanks,
SeongJae Park
==================================== >8 =======================================
From 8f605d807c55b536ab5b0f87306ac78033dc4499 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sjpark@amazon.de>
Date: Fri, 28 Aug 2020 11:29:30 +0000
Subject: [PATCH] mm/damon/paddr: Add missed 'put_page()' calls
Exceptional cases handlings in 'damon_phys_mkold()' and
'damon_phys_young()' doesn't properly put pages. This commit fixes the
problem by adding the 'put_page()' call.
Reported-by: Alkaid <zgf574564920@gmail.com>
Signed-off-by: SeongJae Park <sjpark@amazon.de>
---
mm/damon.c | 53 ++++++++++++++++++++++++++++-------------------------
1 file changed, 28 insertions(+), 25 deletions(-)
diff --git a/mm/damon.c b/mm/damon.c
index d0d55656553b..74a10ea54958 100644
--- a/mm/damon.c
+++ b/mm/damon.c
@@ -836,12 +836,16 @@ unsigned int kdamond_check_vm_accesses(struct damon_ctx *ctx)
/* access check functions for physical address based regions */
/*
- * Get a page by pfn if it is in the LRU list. Otherwise, returns NULL.
+ * Get a page by @pfn if it is in the LRU list and mapped. If the page needs
+ * locked, do the lock and save the result in @locked. Otherwise, returns
+ * NULL.
*
- * The body of this function is stollen from the 'page_idle_get_page()'. We
- * steal rather than reuse it because the code is quite simple .
+ * The body of this function is mostly stollen from the 'page_idle_get_page()'
+ * and 'page_idle_clear_pte_refs()'. We steal rather than reuse it not because
+ * we are great artists but the code is quite simple and we need to unify parts
+ * of the two functions.
*/
-static struct page *damon_phys_get_page(unsigned long pfn)
+static struct page *damon_phys_get_page(unsigned long pfn, bool *locked)
{
struct page *page = pfn_to_online_page(pfn);
pg_data_t *pgdat;
@@ -854,9 +858,22 @@ static struct page *damon_phys_get_page(unsigned long pfn)
spin_lock_irq(&pgdat->lru_lock);
if (unlikely(!PageLRU(page))) {
put_page(page);
- page = NULL;
+ spin_unlock_irq(&pgdat->lru_lock);
+ return NULL;
}
spin_unlock_irq(&pgdat->lru_lock);
+
+ if (!page_mapped(page) || !page_rmapping(page)) {
+ put_page(page);
+ return NULL;
+ }
+
+ *locked = !PageAnon(page) || PageKsm(page);
+ if (*locked && !trylock_page(page)) {
+ put_page(page);
+ return NULL;
+ }
+
return page;
}
@@ -869,26 +886,19 @@ static bool damon_page_mkold(struct page *page, struct vm_area_struct *vma,
static void damon_phys_mkold(unsigned long paddr)
{
- struct page *page = damon_phys_get_page(PHYS_PFN(paddr));
+ bool locked;
+ struct page *page = damon_phys_get_page(PHYS_PFN(paddr), &locked);
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;
-
- need_lock = !PageAnon(page) || PageKsm(page);
- if (need_lock && !trylock_page(page))
- return;
-
rmap_walk(page, &rwc);
- if (need_lock)
+ if (locked)
unlock_page(page);
put_page(page);
}
@@ -930,7 +940,8 @@ static bool damon_page_accessed(struct page *page, struct vm_area_struct *vma,
static bool damon_phys_young(unsigned long paddr, unsigned long *page_sz)
{
- struct page *page = damon_phys_get_page(PHYS_PFN(paddr));
+ bool locked;
+ struct page *page = damon_phys_get_page(PHYS_PFN(paddr), &locked);
struct damon_phys_access_chk_result result = {
.page_sz = PAGE_SIZE,
.accessed = false,
@@ -940,21 +951,13 @@ static bool damon_phys_young(unsigned long paddr, unsigned long *page_sz)
.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;
-
- need_lock = !PageAnon(page) || PageKsm(page);
- if (need_lock && !trylock_page(page))
- return false;
-
rmap_walk(page, &rwc);
- if (need_lock)
+ if (locked)
unlock_page(page);
put_page(page);
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring
2020-08-20 13:26 ` Shakeel Butt
@ 2020-08-20 14:28 ` SeongJae Park
0 siblings, 0 replies; 7+ messages in thread
From: SeongJae Park @ 2020-08-20 14:28 UTC (permalink / raw)
To: Shakeel Butt
Cc: SeongJae Park, Jonathan.Cameron, Andrea Arcangeli, acme,
alexander.shishkin, amit, benh, brendan.d.gregg, Brendan Higgins,
Qian Cai, Colin Ian King, Jonathan Corbet, David Hildenbrand,
dwmw, Du, Fan, foersleo, Greg Thelen, Ian Rogers, jolsa,
Kirill A. Shutemov, mark.rutland, Mel Gorman, Minchan Kim,
Ingo Molnar, namhyung, Peter Zijlstra (Intel),
Randy Dunlap, Rik van Riel, David Rientjes, Steven Rostedt, rppt,
sblbir, shuah, sj38.park, snu, Vlastimil Babka, Vladimir Davydov,
Yang Shi, Huang Ying, zgf574564920, linux-damon, Linux MM,
linux-doc, LKML
On Thu, 20 Aug 2020 06:26:49 -0700 Shakeel Butt <shakeelb@google.com> wrote:
> On Thu, Aug 20, 2020 at 12:17 AM SeongJae Park <sjpark@amazon.com> wrote:
> >
> > On Wed, 19 Aug 2020 17:26:15 -0700 Shakeel Butt <shakeelb@google.com> wrote:
> >
> > > On Tue, Aug 18, 2020 at 12:27 AM SeongJae Park <sjpark@amazon.com> wrote:
> > > >
> > > > From: SeongJae Park <sjpark@amazon.de>
> > > >
> > > > This commit implements the four callbacks (->init_target_regions,
> > > > ->update_target_regions, ->prepare_access_check, and ->check_accesses)
> > > > for the basic access monitoring of the physical memory address space.
> > > > By setting the callback pointers to point those, users can easily
> > > > monitor the accesses to the physical memory.
> > > >
> > > > Internally, it uses the PTE Accessed bit, as similar to that of the
> > > > virtual memory support. Also, it supports only user memory pages, as
> > > > idle page tracking also does, for the same reason. If the monitoring
> > > > target physical memory address range contains non-user memory pages,
> > > > access check of the pages will do nothing but simply treat the pages as
> > > > not accessed.
> > > >
> > > > Users who want to use other access check primitives and/or monitor the
> > > > non-user memory regions could implement and use their own callbacks.
> > > >
> > > > Signed-off-by: SeongJae Park <sjpark@amazon.de>
> > > [snip]
> > > > +static void damon_phys_mkold(unsigned long paddr)
> > > > +{
> > > > + struct page *page = damon_phys_get_page(PHYS_PFN(paddr));
> > > > + 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;
> > >
> > > I don't think you want to skip the unmapped pages. The point of
> > > physical address space monitoring was to include the monitoring of
> > > unmapped pages, so, skipping them invalidates the underlying
> > > motivation.
> >
> > I think my answer to your other mail[1] could be an answer to this. Let me
> > quote some from it:
> >
> > ```
> > Technically speaking, this patchset introduces an implementation of DAMON's low
> > level primitives for physical address space of LRU-listed pages. In other
> > words, it is not designed for cgroups case. Also, please note that this
> > patchset is only RFC, because it aims to only show the future plan of DAMON and
> > get opinions about the concept before being serious. It will be serious only
> > after the DAMON patchset is merged. Maybe I didn' made this point clear in the
> > CV, sorry. I will state this clearly in the next spin.
> > ```
>
> The unmapped pages are also LRU pages.
Sorry, I missed the detail. So, the description should be updated to:
This patchset introduces an implementation of DAMON's low level primitives
for physical addressspace of _mapped_ LRU pages.
> Let's forget about the cgroups
> support for a moment, the only reason to use DAMON's physical address
> space monitoring is also to track the accesses of unmapped pages
> otherwise virtual address space monitoring already does the monitoring
> for mapped pages.
Well, I didn't intended the use case... I just wanted to let people see the
data accesses on physical address space without tracking every mappings of the
pages.
Anyway, ok, we could consider supporting unmapped pages, but I'm unsure why and
how much it is necessary. After all, who could access unmapped pages? Could
you give me more details on the needs for access monitoring of the unmapped
pages?
Thanks,
SeongJae Park
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring
2020-08-20 7:16 ` SeongJae Park
@ 2020-08-20 13:26 ` Shakeel Butt
2020-08-20 14:28 ` SeongJae Park
0 siblings, 1 reply; 7+ messages in thread
From: Shakeel Butt @ 2020-08-20 13:26 UTC (permalink / raw)
To: SeongJae Park
Cc: SeongJae Park, Jonathan.Cameron, Andrea Arcangeli, acme,
alexander.shishkin, amit, benh, brendan.d.gregg, Brendan Higgins,
Qian Cai, Colin Ian King, Jonathan Corbet, David Hildenbrand,
dwmw, Du, Fan, foersleo, Greg Thelen, Ian Rogers, jolsa,
Kirill A. Shutemov, mark.rutland, Mel Gorman, Minchan Kim,
Ingo Molnar, namhyung, Peter Zijlstra (Intel),
Randy Dunlap, Rik van Riel, David Rientjes, Steven Rostedt, rppt,
sblbir, shuah, sj38.park, snu, Vlastimil Babka, Vladimir Davydov,
Yang Shi, Huang Ying, zgf574564920, linux-damon, Linux MM,
linux-doc, LKML
On Thu, Aug 20, 2020 at 12:17 AM SeongJae Park <sjpark@amazon.com> wrote:
>
> On Wed, 19 Aug 2020 17:26:15 -0700 Shakeel Butt <shakeelb@google.com> wrote:
>
> > On Tue, Aug 18, 2020 at 12:27 AM SeongJae Park <sjpark@amazon.com> wrote:
> > >
> > > From: SeongJae Park <sjpark@amazon.de>
> > >
> > > This commit implements the four callbacks (->init_target_regions,
> > > ->update_target_regions, ->prepare_access_check, and ->check_accesses)
> > > for the basic access monitoring of the physical memory address space.
> > > By setting the callback pointers to point those, users can easily
> > > monitor the accesses to the physical memory.
> > >
> > > Internally, it uses the PTE Accessed bit, as similar to that of the
> > > virtual memory support. Also, it supports only user memory pages, as
> > > idle page tracking also does, for the same reason. If the monitoring
> > > target physical memory address range contains non-user memory pages,
> > > access check of the pages will do nothing but simply treat the pages as
> > > not accessed.
> > >
> > > Users who want to use other access check primitives and/or monitor the
> > > non-user memory regions could implement and use their own callbacks.
> > >
> > > Signed-off-by: SeongJae Park <sjpark@amazon.de>
> > [snip]
> > > +static void damon_phys_mkold(unsigned long paddr)
> > > +{
> > > + struct page *page = damon_phys_get_page(PHYS_PFN(paddr));
> > > + 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;
> >
> > I don't think you want to skip the unmapped pages. The point of
> > physical address space monitoring was to include the monitoring of
> > unmapped pages, so, skipping them invalidates the underlying
> > motivation.
>
> I think my answer to your other mail[1] could be an answer to this. Let me
> quote some from it:
>
> ```
> Technically speaking, this patchset introduces an implementation of DAMON's low
> level primitives for physical address space of LRU-listed pages. In other
> words, it is not designed for cgroups case. Also, please note that this
> patchset is only RFC, because it aims to only show the future plan of DAMON and
> get opinions about the concept before being serious. It will be serious only
> after the DAMON patchset is merged. Maybe I didn' made this point clear in the
> CV, sorry. I will state this clearly in the next spin.
> ```
The unmapped pages are also LRU pages. Let's forget about the cgroups
support for a moment, the only reason to use DAMON's physical address
space monitoring is also to track the accesses of unmapped pages
otherwise virtual address space monitoring already does the monitoring
for mapped pages.
>
> ```
> So, DAMON is a framework rather than a tool. Though it comes with basic
> applications using DAMON as a framework (e.g., the virtual address space low
> primitives implementation, DAMON debugfs interface, and the DAMON user space
> tool) that could be useful in simple use cases, you need to code your
> application on it if your use cases are out of the simple cases. I will also
> develop more of such applications for more use-cases, but it will be only after
> the framework is complete enough to be merged in the mainline.
> ```
>
> Of course, we could prioritize the cgroup support if strongly required, though
> I still prefer focusing on the framework itself for now.
>
> [1] https://lore.kernel.org/linux-mm/20200820071052.24271-1-sjpark@amazon.com/
>
>
> Thanks,
> SeongJae Park
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring
2020-08-20 0:26 ` Shakeel Butt
@ 2020-08-20 7:16 ` SeongJae Park
2020-08-20 13:26 ` Shakeel Butt
0 siblings, 1 reply; 7+ messages in thread
From: SeongJae Park @ 2020-08-20 7:16 UTC (permalink / raw)
To: Shakeel Butt
Cc: SeongJae Park, SeongJae Park, Jonathan.Cameron, Andrea Arcangeli,
acme, alexander.shishkin, amit, benh, brendan.d.gregg,
Brendan Higgins, Qian Cai, Colin Ian King, Jonathan Corbet,
David Hildenbrand, dwmw, Du, Fan, foersleo, Greg Thelen,
Ian Rogers, jolsa, Kirill A. Shutemov, mark.rutland, Mel Gorman,
Minchan Kim, Ingo Molnar, namhyung, Peter Zijlstra (Intel),
Randy Dunlap, Rik van Riel, David Rientjes, Steven Rostedt, rppt,
sblbir, shuah, sj38.park, snu, Vlastimil Babka, Vladimir Davydov,
Yang Shi, Huang Ying, zgf574564920, linux-damon, Linux MM,
linux-doc, LKML
On Wed, 19 Aug 2020 17:26:15 -0700 Shakeel Butt <shakeelb@google.com> wrote:
> On Tue, Aug 18, 2020 at 12:27 AM SeongJae Park <sjpark@amazon.com> wrote:
> >
> > From: SeongJae Park <sjpark@amazon.de>
> >
> > This commit implements the four callbacks (->init_target_regions,
> > ->update_target_regions, ->prepare_access_check, and ->check_accesses)
> > for the basic access monitoring of the physical memory address space.
> > By setting the callback pointers to point those, users can easily
> > monitor the accesses to the physical memory.
> >
> > Internally, it uses the PTE Accessed bit, as similar to that of the
> > virtual memory support. Also, it supports only user memory pages, as
> > idle page tracking also does, for the same reason. If the monitoring
> > target physical memory address range contains non-user memory pages,
> > access check of the pages will do nothing but simply treat the pages as
> > not accessed.
> >
> > Users who want to use other access check primitives and/or monitor the
> > non-user memory regions could implement and use their own callbacks.
> >
> > Signed-off-by: SeongJae Park <sjpark@amazon.de>
> [snip]
> > +static void damon_phys_mkold(unsigned long paddr)
> > +{
> > + struct page *page = damon_phys_get_page(PHYS_PFN(paddr));
> > + 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;
>
> I don't think you want to skip the unmapped pages. The point of
> physical address space monitoring was to include the monitoring of
> unmapped pages, so, skipping them invalidates the underlying
> motivation.
I think my answer to your other mail[1] could be an answer to this. Let me
quote some from it:
```
Technically speaking, this patchset introduces an implementation of DAMON's low
level primitives for physical address space of LRU-listed pages. In other
words, it is not designed for cgroups case. Also, please note that this
patchset is only RFC, because it aims to only show the future plan of DAMON and
get opinions about the concept before being serious. It will be serious only
after the DAMON patchset is merged. Maybe I didn' made this point clear in the
CV, sorry. I will state this clearly in the next spin.
```
```
So, DAMON is a framework rather than a tool. Though it comes with basic
applications using DAMON as a framework (e.g., the virtual address space low
primitives implementation, DAMON debugfs interface, and the DAMON user space
tool) that could be useful in simple use cases, you need to code your
application on it if your use cases are out of the simple cases. I will also
develop more of such applications for more use-cases, but it will be only after
the framework is complete enough to be merged in the mainline.
```
Of course, we could prioritize the cgroup support if strongly required, though
I still prefer focusing on the framework itself for now.
[1] https://lore.kernel.org/linux-mm/20200820071052.24271-1-sjpark@amazon.com/
Thanks,
SeongJae Park
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring
2020-08-18 7:24 ` [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring SeongJae Park
@ 2020-08-20 0:26 ` Shakeel Butt
2020-08-20 7:16 ` SeongJae Park
0 siblings, 1 reply; 7+ messages in thread
From: Shakeel Butt @ 2020-08-20 0:26 UTC (permalink / raw)
To: SeongJae Park
Cc: Andrew Morton, SeongJae Park, Jonathan.Cameron, Andrea Arcangeli,
acme, alexander.shishkin, amit, benh, brendan.d.gregg,
Brendan Higgins, Qian Cai, Colin Ian King, Jonathan Corbet,
David Hildenbrand, dwmw, Du, Fan, foersleo, Greg Thelen,
Ian Rogers, jolsa, Kirill A. Shutemov, mark.rutland, Mel Gorman,
Minchan Kim, Ingo Molnar, namhyung, Peter Zijlstra (Intel),
Randy Dunlap, Rik van Riel, David Rientjes, Steven Rostedt, rppt,
sblbir, shuah, sj38.park, snu, Vlastimil Babka, Vladimir Davydov,
Yang Shi, Huang Ying, zgf574564920, linux-damon, Linux MM,
linux-doc, LKML
On Tue, Aug 18, 2020 at 12:27 AM SeongJae Park <sjpark@amazon.com> wrote:
>
> From: SeongJae Park <sjpark@amazon.de>
>
> This commit implements the four callbacks (->init_target_regions,
> ->update_target_regions, ->prepare_access_check, and ->check_accesses)
> for the basic access monitoring of the physical memory address space.
> By setting the callback pointers to point those, users can easily
> monitor the accesses to the physical memory.
>
> Internally, it uses the PTE Accessed bit, as similar to that of the
> virtual memory support. Also, it supports only user memory pages, as
> idle page tracking also does, for the same reason. If the monitoring
> target physical memory address range contains non-user memory pages,
> access check of the pages will do nothing but simply treat the pages as
> not accessed.
>
> Users who want to use other access check primitives and/or monitor the
> non-user memory regions could implement and use their own callbacks.
>
> Signed-off-by: SeongJae Park <sjpark@amazon.de>
[snip]
> +static void damon_phys_mkold(unsigned long paddr)
> +{
> + struct page *page = damon_phys_get_page(PHYS_PFN(paddr));
> + 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;
I don't think you want to skip the unmapped pages. The point of
physical address space monitoring was to include the monitoring of
unmapped pages, so, skipping them invalidates the underlying
motivation.
> +
> + 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);
> +}
> +
[snip]
> +
> +static bool damon_phys_young(unsigned long paddr, unsigned long *page_sz)
> +{
> + struct page *page = damon_phys_get_page(PHYS_PFN(paddr));
> + 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;
Same here.
> +
> + 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;
> +}
> +
^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring
2020-08-18 7:24 [RFC v7 00/10] DAMON: Support Physical Memory Address Space Monitoring SeongJae Park
@ 2020-08-18 7:24 ` SeongJae Park
2020-08-20 0:26 ` Shakeel Butt
0 siblings, 1 reply; 7+ messages in thread
From: SeongJae Park @ 2020-08-18 7:24 UTC (permalink / raw)
To: akpm
Cc: SeongJae Park, Jonathan.Cameron, aarcange, acme,
alexander.shishkin, amit, benh, brendan.d.gregg, brendanhiggins,
cai, colin.king, corbet, david, dwmw, fan.du, foersleo, gthelen,
irogers, jolsa, kirill, mark.rutland, mgorman, minchan, mingo,
namhyung, peterz, rdunlap, riel, rientjes, rostedt, rppt, sblbir,
shakeelb, shuah, sj38.park, snu, vbabka, vdavydov.dev, yang.shi,
ying.huang, zgf574564920, linux-damon, linux-mm, linux-doc,
linux-kernel
From: SeongJae Park <sjpark@amazon.de>
This commit implements the four callbacks (->init_target_regions,
->update_target_regions, ->prepare_access_check, and ->check_accesses)
for the basic access monitoring of the physical memory address space.
By setting the callback pointers to point those, users can easily
monitor the accesses to the physical memory.
Internally, it uses the PTE Accessed bit, as similar to that of the
virtual memory support. Also, it supports only user memory pages, as
idle page tracking also does, for the same reason. If the monitoring
target physical memory address range contains non-user memory pages,
access check of the pages will do nothing but simply treat the pages as
not accessed.
Users who want to use other access check primitives and/or monitor the
non-user memory regions could implement and use their own callbacks.
Signed-off-by: SeongJae Park <sjpark@amazon.de>
---
include/linux/damon.h | 6 ++
mm/damon.c | 200 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 206 insertions(+)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 0e790af9c23a..306fa221ceae 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -242,6 +242,12 @@ unsigned int kdamond_check_vm_accesses(struct damon_ctx *ctx);
bool kdamond_vm_target_valid(struct damon_target *t);
void kdamond_vm_cleanup(struct damon_ctx *ctx);
+/* Reference callback implementations for physical memory */
+void kdamond_init_phys_regions(struct damon_ctx *ctx);
+void kdamond_update_phys_regions(struct damon_ctx *ctx);
+void kdamond_prepare_phys_access_checks(struct damon_ctx *ctx);
+unsigned int kdamond_check_phys_accesses(struct damon_ctx *ctx);
+
int damon_set_targets(struct damon_ctx *ctx,
unsigned long *ids, ssize_t nr_ids);
int damon_set_attrs(struct damon_ctx *ctx, unsigned long sample_int,
diff --git a/mm/damon.c b/mm/damon.c
index 9815d22fc4de..d0d55656553b 100644
--- a/mm/damon.c
+++ b/mm/damon.c
@@ -26,11 +26,14 @@
#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/kthread.h>
+#include <linux/memory_hotplug.h>
#include <linux/mm.h>
#include <linux/mmu_notifier.h>
#include <linux/module.h>
#include <linux/page_idle.h>
+#include <linux/pagemap.h>
#include <linux/random.h>
+#include <linux/rmap.h>
#include <linux/sched/mm.h>
#include <linux/sched/task.h>
#include <linux/slab.h>
@@ -547,6 +550,18 @@ void kdamond_init_vm_regions(struct damon_ctx *ctx)
}
}
+/*
+ * The initial regions construction function for the physical address space.
+ *
+ * This default version does nothing in actual. Users should set the initial
+ * regions by themselves before passing their damon_ctx to 'damon_start()', or
+ * implement their version of this and set '->init_target_regions' of their
+ * damon_ctx to point it.
+ */
+void kdamond_init_phys_regions(struct damon_ctx *ctx)
+{
+}
+
/*
* Functions for the dynamic monitoring target regions update
*/
@@ -630,6 +645,19 @@ void kdamond_update_vm_regions(struct damon_ctx *ctx)
}
}
+/*
+ * The dynamic monitoring target regions update function for the physical
+ * address space.
+ *
+ * This default version does nothing in actual. Users should update the
+ * regions in other callbacks such as '->aggregate_cb', or implement their
+ * version of this and set the '->init_target_regions' of their damon_ctx to
+ * point it.
+ */
+void kdamond_update_phys_regions(struct damon_ctx *ctx)
+{
+}
+
/*
* Functions for the access checking of the regions
*/
@@ -805,6 +833,178 @@ unsigned int kdamond_check_vm_accesses(struct damon_ctx *ctx)
return max_nr_accesses;
}
+/* access check functions for physical address based regions */
+
+/*
+ * Get a page by pfn if it is in the LRU list. Otherwise, returns NULL.
+ *
+ * The body of this function is stollen from the 'page_idle_get_page()'. We
+ * steal rather than reuse it because the code is quite simple .
+ */
+static struct page *damon_phys_get_page(unsigned long pfn)
+{
+ struct page *page = pfn_to_online_page(pfn);
+ pg_data_t *pgdat;
+
+ if (!page || !PageLRU(page) ||
+ !get_page_unless_zero(page))
+ return NULL;
+
+ pgdat = page_pgdat(page);
+ spin_lock_irq(&pgdat->lru_lock);
+ if (unlikely(!PageLRU(page))) {
+ put_page(page);
+ page = NULL;
+ }
+ spin_unlock_irq(&pgdat->lru_lock);
+ return page;
+}
+
+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));
+ 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;
+
+ 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 void damon_prepare_phys_access_check(struct damon_ctx *ctx,
+ struct damon_region *r)
+{
+ r->sampling_addr = damon_rand(r->ar.start, r->ar.end);
+
+ damon_phys_mkold(r->sampling_addr);
+}
+
+void kdamond_prepare_phys_access_checks(struct damon_ctx *ctx)
+{
+ struct damon_target *t;
+ struct damon_region *r;
+
+ damon_for_each_target(t, ctx) {
+ damon_for_each_region(r, t)
+ damon_prepare_phys_access_check(ctx, r);
+ }
+}
+
+struct damon_phys_access_chk_result {
+ unsigned long page_sz;
+ bool accessed;
+};
+
+static bool damon_page_accessed(struct page *page, struct vm_area_struct *vma,
+ unsigned long addr, void *arg)
+{
+ struct damon_phys_access_chk_result *result = arg;
+
+ result->accessed = damon_young(vma->vm_mm, addr, &result->page_sz);
+
+ /* If accessed, stop walking */
+ return !result->accessed;
+}
+
+static bool damon_phys_young(unsigned long paddr, unsigned long *page_sz)
+{
+ struct page *page = damon_phys_get_page(PHYS_PFN(paddr));
+ 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;
+
+ 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;
+}
+
+/*
+ * Check whether the region was accessed after the last preparation
+ *
+ * mm 'mm_struct' for the given virtual address space
+ * r the region of physical address space that needs to be checked
+ */
+static void damon_check_phys_access(struct damon_ctx *ctx,
+ struct damon_region *r)
+{
+ static unsigned long last_addr;
+ static unsigned long last_page_sz = PAGE_SIZE;
+ static bool last_accessed;
+
+ /* If the region is in the last checked page, reuse the result */
+ if (ALIGN_DOWN(last_addr, last_page_sz) ==
+ ALIGN_DOWN(r->sampling_addr, last_page_sz)) {
+ if (last_accessed)
+ r->nr_accesses++;
+ return;
+ }
+
+ last_accessed = damon_phys_young(r->sampling_addr, &last_page_sz);
+ if (last_accessed)
+ r->nr_accesses++;
+
+ last_addr = r->sampling_addr;
+}
+
+unsigned int kdamond_check_phys_accesses(struct damon_ctx *ctx)
+{
+ struct damon_target *t;
+ struct damon_region *r;
+ unsigned int max_nr_accesses = 0;
+
+ damon_for_each_target(t, ctx) {
+ damon_for_each_region(r, t) {
+ damon_check_phys_access(ctx, r);
+ max_nr_accesses = max(r->nr_accesses, max_nr_accesses);
+ }
+ }
+
+ return max_nr_accesses;
+}
/*
* Functions for the target validity check and cleanup
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-08-28 11:40 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CAJFQjcOCDFGq7pNgYWpKbrhBM5FQo76d274H8UU1ma_8TWjvuQ@mail.gmail.com>
2020-08-28 9:53 ` [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring SeongJae Park
2020-08-28 11:39 ` SeongJae Park
2020-08-18 7:24 [RFC v7 00/10] DAMON: Support Physical Memory Address Space Monitoring SeongJae Park
2020-08-18 7:24 ` [RFC v7 06/10] mm/damon: Implement callbacks for physical memory monitoring SeongJae Park
2020-08-20 0:26 ` Shakeel Butt
2020-08-20 7:16 ` SeongJae Park
2020-08-20 13:26 ` Shakeel Butt
2020-08-20 14:28 ` SeongJae Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®