* [PATCH] mm/damon: Add option to monitor only writes
@ 2022-02-03 13:12 Pedro Demarchi Gomes
2022-02-03 13:39 ` SeongJae Park
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Pedro Demarchi Gomes @ 2022-02-03 13:12 UTC (permalink / raw)
Cc: Pedro Demarchi Gomes, SeongJae Park, Steven Rostedt, Ingo Molnar,
Andrew Morton, linux-mm, linux-kernel
When "writes" is written to /sys/kernel/debug/damon/counter_type damon will monitor only writes.
This patch also adds the actions mergeable and unmergeable to damos schemes. These actions are used by KSM as explained in [1].
[1] https://www.kernel.org/doc/html/latest/admin-guide/mm/ksm.html#controlling-ksm-with-madvise
Signed-off-by: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
---
include/linux/damon.h | 13 ++++++
include/trace/events/damon.h | 10 ++--
mm/damon/core.c | 91 +++++++++++++++++++++++++-----------
mm/damon/dbgfs.c | 72 +++++++++++++++++++++++++++-
mm/damon/prmtv-common.c | 88 ++++++++++++++++++++++++++++++++++
mm/damon/prmtv-common.h | 5 ++
mm/damon/vaddr.c | 80 ++++++++++++++++++++++++++++---
7 files changed, 318 insertions(+), 41 deletions(-)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index b4d4be3cc987..9efe89bbcec8 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -44,11 +44,13 @@ struct damon_region {
struct damon_addr_range ar;
unsigned long sampling_addr;
unsigned int nr_accesses;
+ unsigned int nr_writes;
struct list_head list;
unsigned int age;
/* private: Internal value for age calculation. */
unsigned int last_nr_accesses;
+ unsigned int last_nr_writes;
};
/**
@@ -88,6 +90,8 @@ enum damos_action {
DAMOS_PAGEOUT,
DAMOS_HUGEPAGE,
DAMOS_NOHUGEPAGE,
+ DAMOS_MERGEABLE,
+ DAMOS_UNMERGEABLE,
DAMOS_STAT, /* Do nothing but only record the stat */
};
@@ -185,6 +189,11 @@ struct damos_watermarks {
bool activated;
};
+enum damos_counter_type {
+ DAMOS_NUMBER_ACCESSES,
+ DAMOS_NUMBER_WRITES,
+};
+
/**
* struct damos - Represents a Data Access Monitoring-based Operation Scheme.
* @min_sz_region: Minimum size of target regions.
@@ -223,6 +232,9 @@ struct damos {
unsigned long max_sz_region;
unsigned int min_nr_accesses;
unsigned int max_nr_accesses;
+ unsigned int min_nr_writes;
+ unsigned int max_nr_writes;
+ enum damos_counter_type counter_type;
unsigned int min_age_region;
unsigned int max_age_region;
enum damos_action action;
@@ -386,6 +398,7 @@ struct damon_ctx {
struct damon_primitive primitive;
struct damon_callback callback;
+ enum damos_counter_type counter_type;
unsigned long min_nr_regions;
unsigned long max_nr_regions;
struct list_head adaptive_targets;
diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h
index 2f422f4f1fb9..45573f421707 100644
--- a/include/trace/events/damon.h
+++ b/include/trace/events/damon.h
@@ -11,17 +11,17 @@
TRACE_EVENT(damon_aggregated,
- TP_PROTO(struct damon_target *t, struct damon_region *r,
+ TP_PROTO(struct damon_ctx *c, struct damon_target *t, struct damon_region *r,
unsigned int nr_regions),
- TP_ARGS(t, r, nr_regions),
+ TP_ARGS(c, t, r, nr_regions),
TP_STRUCT__entry(
__field(unsigned long, target_id)
__field(unsigned int, nr_regions)
__field(unsigned long, start)
__field(unsigned long, end)
- __field(unsigned int, nr_accesses)
+ __field(unsigned int, nr)
),
TP_fast_assign(
@@ -29,12 +29,12 @@ TRACE_EVENT(damon_aggregated,
__entry->nr_regions = nr_regions;
__entry->start = r->ar.start;
__entry->end = r->ar.end;
- __entry->nr_accesses = r->nr_accesses;
+ __entry->nr = c->counter_type == DAMOS_NUMBER_WRITES ? r->nr_writes : r->nr_accesses;
),
TP_printk("target_id=%lu nr_regions=%u %lu-%lu: %u",
__entry->target_id, __entry->nr_regions,
- __entry->start, __entry->end, __entry->nr_accesses)
+ __entry->start, __entry->end, __entry->nr)
);
#endif /* _TRACE_DAMON_H */
diff --git a/mm/damon/core.c b/mm/damon/core.c
index e92497895202..e827231366db 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -45,10 +45,12 @@ struct damon_region *damon_new_region(unsigned long start, unsigned long end)
region->ar.start = start;
region->ar.end = end;
region->nr_accesses = 0;
+ region->nr_writes = 0;
INIT_LIST_HEAD(®ion->list);
region->age = 0;
region->last_nr_accesses = 0;
+ region->last_nr_writes = 0;
return region;
}
@@ -89,7 +91,7 @@ void damon_destroy_region(struct damon_region *r, struct damon_target *t)
struct damos *damon_new_scheme(
unsigned long min_sz_region, unsigned long max_sz_region,
- unsigned int min_nr_accesses, unsigned int max_nr_accesses,
+ unsigned int min_nr, unsigned int max_nr,
unsigned int min_age_region, unsigned int max_age_region,
enum damos_action action, struct damos_quota *quota,
struct damos_watermarks *wmarks)
@@ -101,8 +103,10 @@ struct damos *damon_new_scheme(
return NULL;
scheme->min_sz_region = min_sz_region;
scheme->max_sz_region = max_sz_region;
- scheme->min_nr_accesses = min_nr_accesses;
- scheme->max_nr_accesses = max_nr_accesses;
+ scheme->max_nr_writes = max_nr;
+ scheme->min_nr_writes = min_nr;
+ scheme->min_nr_accesses = min_nr;
+ scheme->max_nr_accesses = max_nr;
scheme->min_age_region = min_age_region;
scheme->max_age_region = max_age_region;
scheme->action = action;
@@ -221,6 +225,7 @@ struct damon_ctx *damon_new_ctx(void)
ctx->sample_interval = 5 * 1000;
ctx->aggr_interval = 100 * 1000;
ctx->primitive_update_interval = 60 * 1000 * 1000;
+ ctx->counter_type = DAMOS_NUMBER_ACCESSES;
ktime_get_coarse_ts64(&ctx->last_aggregation);
ctx->last_primitive_update = ctx->last_aggregation;
@@ -535,9 +540,11 @@ static void kdamond_reset_aggregated(struct damon_ctx *c)
struct damon_region *r;
damon_for_each_region(r, t) {
- trace_damon_aggregated(t, r, damon_nr_regions(t));
+ trace_damon_aggregated(c, t, r, damon_nr_regions(t));
r->last_nr_accesses = r->nr_accesses;
r->nr_accesses = 0;
+ r->last_nr_writes = r->nr_writes;
+ r->nr_writes = 0;
}
}
}
@@ -546,21 +553,27 @@ static void damon_split_region_at(struct damon_ctx *ctx,
struct damon_target *t, struct damon_region *r,
unsigned long sz_r);
-static bool __damos_valid_target(struct damon_region *r, struct damos *s)
+static bool __damos_valid_target(struct damon_ctx *ctx, struct damon_region *r, struct damos *s)
{
unsigned long sz;
-
sz = r->ar.end - r->ar.start;
- return s->min_sz_region <= sz && sz <= s->max_sz_region &&
- s->min_nr_accesses <= r->nr_accesses &&
- r->nr_accesses <= s->max_nr_accesses &&
- s->min_age_region <= r->age && r->age <= s->max_age_region;
+
+ if (ctx->counter_type == DAMOS_NUMBER_WRITES)
+ return s->min_sz_region <= sz && sz <= s->max_sz_region &&
+ s->min_nr_writes <= r->nr_writes &&
+ r->nr_writes <= s->max_nr_writes &&
+ s->min_age_region <= r->age && r->age <= s->max_age_region;
+ else
+ return s->min_sz_region <= sz && sz <= s->max_sz_region &&
+ s->min_nr_accesses <= r->nr_accesses &&
+ r->nr_accesses <= s->max_nr_accesses &&
+ s->min_age_region <= r->age && r->age <= s->max_age_region;
}
static bool damos_valid_target(struct damon_ctx *c, struct damon_target *t,
struct damon_region *r, struct damos *s)
{
- bool ret = __damos_valid_target(r, s);
+ bool ret = __damos_valid_target(c, r, s);
if (!ret || !s->quota.esz || !c->primitive.get_scheme_score)
return ret;
@@ -707,7 +720,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
memset(quota->histogram, 0, sizeof(quota->histogram));
damon_for_each_target(t, c) {
damon_for_each_region(r, t) {
- if (!__damos_valid_target(r, s))
+ if (!__damos_valid_target(c, r, s))
continue;
score = c->primitive.get_scheme_score(
c, t, r, s);
@@ -738,13 +751,18 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
/*
* Merge two adjacent regions into one region
*/
-static void damon_merge_two_regions(struct damon_target *t,
+static void damon_merge_two_regions(struct damon_ctx *c, struct damon_target *t,
struct damon_region *l, struct damon_region *r)
{
unsigned long sz_l = sz_damon_region(l), sz_r = sz_damon_region(r);
- l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
+ if (c->counter_type == DAMOS_NUMBER_WRITES)
+ l->nr_writes = (l->nr_writes * sz_l + r->nr_writes * sz_r) /
(sz_l + sz_r);
+ else
+ l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
+ (sz_l + sz_r);
+
l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r);
l->ar.end = r->ar.end;
damon_destroy_region(r, t);
@@ -759,23 +777,39 @@ static void damon_merge_two_regions(struct damon_target *t,
* thres '->nr_accesses' diff threshold for the merge
* sz_limit size upper limit of each region
*/
-static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
+static void damon_merge_regions_of(struct damon_ctx *c, struct damon_target *t, unsigned int thres,
unsigned long sz_limit)
{
struct damon_region *r, *prev = NULL, *next;
- damon_for_each_region_safe(r, next, t) {
- if (diff_of(r->nr_accesses, r->last_nr_accesses) > thres)
- r->age = 0;
- else
- r->age++;
-
- if (prev && prev->ar.end == r->ar.start &&
- diff_of(prev->nr_accesses, r->nr_accesses) <= thres &&
- sz_damon_region(prev) + sz_damon_region(r) <= sz_limit)
- damon_merge_two_regions(t, prev, r);
- else
- prev = r;
+ if (c->counter_type == DAMOS_NUMBER_WRITES) {
+ damon_for_each_region_safe(r, next, t) {
+ if (diff_of(r->nr_writes, r->last_nr_writes) > thres)
+ r->age = 0;
+ else
+ r->age++;
+
+ if (prev && prev->ar.end == r->ar.start &&
+ diff_of(prev->nr_writes, r->nr_writes) <= thres &&
+ sz_damon_region(prev) + sz_damon_region(r) <= sz_limit)
+ damon_merge_two_regions(c, t, prev, r);
+ else
+ prev = r;
+ }
+ } else {
+ damon_for_each_region_safe(r, next, t) {
+ if (diff_of(r->nr_accesses, r->last_nr_accesses) > thres)
+ r->age = 0;
+ else
+ r->age++;
+
+ if (prev && prev->ar.end == r->ar.start &&
+ diff_of(prev->nr_accesses, r->nr_accesses) <= thres &&
+ sz_damon_region(prev) + sz_damon_region(r) <= sz_limit)
+ damon_merge_two_regions(c, t, prev, r);
+ else
+ prev = r;
+ }
}
}
@@ -796,7 +830,7 @@ static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
struct damon_target *t;
damon_for_each_target(t, c)
- damon_merge_regions_of(t, threshold, sz_limit);
+ damon_merge_regions_of(c, t, threshold, sz_limit);
}
/*
@@ -819,6 +853,7 @@ static void damon_split_region_at(struct damon_ctx *ctx,
new->age = r->age;
new->last_nr_accesses = r->last_nr_accesses;
+ new->last_nr_writes = r->last_nr_writes;
damon_insert_region(new, r, damon_next_region(r), t);
}
diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
index ad65436756af..6cf2501148f5 100644
--- a/mm/damon/dbgfs.c
+++ b/mm/damon/dbgfs.c
@@ -166,6 +166,8 @@ static bool damos_action_valid(int action)
case DAMOS_PAGEOUT:
case DAMOS_HUGEPAGE:
case DAMOS_NOHUGEPAGE:
+ case DAMOS_MERGEABLE:
+ case DAMOS_UNMERGEABLE:
case DAMOS_STAT:
return true;
default:
@@ -477,6 +479,66 @@ static ssize_t dbgfs_init_regions_read(struct file *file, char __user *buf,
return len;
}
+static ssize_t dbgfs_counter_type_write(struct file *file,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ struct damon_ctx *ctx = file->private_data;
+ char *kbuf;
+ ssize_t ret;
+
+ mutex_lock(&ctx->kdamond_lock);
+ if (ctx->kdamond) {
+ mutex_unlock(&ctx->kdamond_lock);
+ ret = -EBUSY;
+ goto out;
+ }
+ ret = count;
+ kbuf = user_input_str(buf, count, ppos);
+ if (IS_ERR(kbuf))
+ return PTR_ERR(kbuf);
+
+ if (!strncmp(kbuf, "writes\n", count))
+ ctx->counter_type = DAMOS_NUMBER_WRITES;
+ else
+ ctx->counter_type = DAMOS_NUMBER_ACCESSES;
+
+ mutex_unlock(&ctx->kdamond_lock);
+out:
+ kfree(kbuf);
+ return ret;
+}
+
+static ssize_t dbgfs_counter_type_read(struct file *file, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct damon_ctx *ctx = file->private_data;
+ char *kbuf;
+ ssize_t len;
+
+ kbuf = kmalloc(count, GFP_KERNEL | __GFP_NOWARN);
+ if (!kbuf)
+ return -ENOMEM;
+
+ mutex_lock(&ctx->kdamond_lock);
+ if (ctx->kdamond) {
+ mutex_unlock(&ctx->kdamond_lock);
+ len = -EBUSY;
+ goto out;
+ }
+
+ if (ctx->counter_type == DAMOS_NUMBER_WRITES)
+ len = scnprintf(kbuf, count, "writes");
+ else
+ len = scnprintf(kbuf, count, "accesses");
+ mutex_unlock(&ctx->kdamond_lock);
+ len = simple_read_from_buffer(buf, count, ppos, kbuf, len);
+
+out:
+ kfree(kbuf);
+ return len;
+}
+
+
static int add_init_region(struct damon_ctx *c,
unsigned long target_id, struct damon_addr_range *ar)
{
@@ -636,12 +698,18 @@ static const struct file_operations kdamond_pid_fops = {
.read = dbgfs_kdamond_pid_read,
};
+static const struct file_operations counter_type_fops = {
+ .open = damon_dbgfs_open,
+ .read = dbgfs_counter_type_read,
+ .write = dbgfs_counter_type_write,
+};
+
static void dbgfs_fill_ctx_dir(struct dentry *dir, struct damon_ctx *ctx)
{
const char * const file_names[] = {"attrs", "schemes", "target_ids",
- "init_regions", "kdamond_pid"};
+ "init_regions", "kdamond_pid", "counter_type"};
const struct file_operations *fops[] = {&attrs_fops, &schemes_fops,
- &target_ids_fops, &init_regions_fops, &kdamond_pid_fops};
+ &target_ids_fops, &init_regions_fops, &kdamond_pid_fops, &counter_type_fops};
int i;
for (i = 0; i < ARRAY_SIZE(file_names); i++)
diff --git a/mm/damon/prmtv-common.c b/mm/damon/prmtv-common.c
index 92a04f5831d6..09ba2b5b895e 100644
--- a/mm/damon/prmtv-common.c
+++ b/mm/damon/prmtv-common.c
@@ -9,6 +9,8 @@
#include <linux/page_idle.h>
#include <linux/pagemap.h>
#include <linux/rmap.h>
+#include <linux/swap.h>
+#include <linux/swapops.h>
#include "prmtv-common.h"
@@ -58,6 +60,92 @@ void damon_ptep_mkold(pte_t *pte, struct mm_struct *mm, unsigned long addr)
put_page(page);
}
+static inline bool pte_is_pinned(struct vm_area_struct *vma, unsigned long addr, pte_t pte)
+{
+ struct page *page;
+
+ if (!pte_write(pte))
+ return false;
+ if (!is_cow_mapping(vma->vm_flags))
+ return false;
+ if (likely(!test_bit(MMF_HAS_PINNED, &vma->vm_mm->flags)))
+ return false;
+ page = vm_normal_page(vma, addr, pte);
+ if (!page)
+ return false;
+ return page_maybe_dma_pinned(page);
+}
+
+static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma,
+ unsigned long addr, pmd_t *pmdp)
+{
+ pmd_t old, pmd = *pmdp;
+
+ if (pmd_present(pmd)) {
+ /* See comment in change_huge_pmd() */
+ old = pmdp_invalidate(vma, addr, pmdp);
+ if (pmd_dirty(old))
+ pmd = pmd_mkdirty(pmd);
+ if (pmd_young(old))
+ pmd = pmd_mkyoung(pmd);
+
+ pmd = pmd_wrprotect(pmd);
+ pmd = pmd_clear_soft_dirty(pmd);
+
+ set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
+ } else if (is_migration_entry(pmd_to_swp_entry(pmd))) {
+ pmd = pmd_swp_clear_soft_dirty(pmd);
+ set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
+ }
+}
+
+static inline void clear_soft_dirty(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *pte)
+{
+ /*
+ * The soft-dirty tracker uses #PF-s to catch writes
+ * to pages, so write-protect the pte as well. See the
+ * Documentation/admin-guide/mm/soft-dirty.rst for full description
+ * of how soft-dirty works.
+ */
+ pte_t ptent = *pte;
+
+ if (pte_present(ptent)) {
+ pte_t old_pte;
+
+ if (pte_is_pinned(vma, addr, ptent))
+ return;
+ old_pte = ptep_modify_prot_start(vma, addr, pte);
+ ptent = pte_wrprotect(old_pte);
+ ptent = pte_clear_soft_dirty(ptent);
+ ptep_modify_prot_commit(vma, addr, pte, old_pte, ptent);
+ } else if (is_swap_pte(ptent)) {
+ ptent = pte_swp_clear_soft_dirty(ptent);
+ set_pte_at(vma->vm_mm, addr, pte, ptent);
+ }
+}
+
+void damon_pmd_clean_soft_dirty(struct vm_area_struct *vma, unsigned long addr,
+ pmd_t *pmd)
+{
+ vma->vm_flags &= ~VM_SOFTDIRTY;
+ vma_set_page_prot(vma);
+
+ if (pmd_soft_dirty(*pmd))
+ clear_soft_dirty_pmd(vma, addr, pmd);
+
+}
+
+void damon_ptep_clean_soft_dirty(struct vm_area_struct *vma, unsigned long addr,
+ pte_t *pte)
+{
+ vma->vm_flags &= ~VM_SOFTDIRTY;
+ vma_set_page_prot(vma);
+
+ if (pte_soft_dirty(*pte))
+ clear_soft_dirty(vma, addr, pte);
+}
+
void damon_pmdp_mkold(pmd_t *pmd, struct mm_struct *mm, unsigned long addr)
{
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
diff --git a/mm/damon/prmtv-common.h b/mm/damon/prmtv-common.h
index 61f27037603e..03847d149f0e 100644
--- a/mm/damon/prmtv-common.h
+++ b/mm/damon/prmtv-common.h
@@ -13,6 +13,11 @@
struct page *damon_get_page(unsigned long pfn);
+void damon_ptep_clean_soft_dirty(struct vm_area_struct *vma, unsigned long addr,
+ pte_t *pte);
+void damon_pmd_clean_soft_dirty(struct vm_area_struct *vma, unsigned long addr,
+ pmd_t *pmd);
+
void damon_ptep_mkold(pte_t *pte, struct mm_struct *mm, unsigned long addr);
void damon_pmdp_mkold(pmd_t *pmd, struct mm_struct *mm, unsigned long addr);
diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index 20a9a9d69eb1..a7d9c9563d1d 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -368,6 +368,44 @@ void damon_va_update(struct damon_ctx *ctx)
}
}
+static int damon_clean_soft_dirty_pmd_entry(pmd_t *pmd, unsigned long addr,
+ unsigned long next, struct mm_walk *walk)
+{
+ pte_t *pte;
+ spinlock_t *ptl;
+
+ if (pmd_huge(*pmd)) {
+ ptl = pmd_lock(walk->mm, pmd);
+ if (pmd_huge(*pmd)) {
+ damon_pmd_clean_soft_dirty(walk->vma, addr, pmd);
+ spin_unlock(ptl);
+ return 0;
+ }
+ spin_unlock(ptl);
+ }
+
+ if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
+ return 0;
+ pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl);
+ if (!pte_present(*pte))
+ goto out;
+ damon_ptep_clean_soft_dirty(walk->vma, addr, pte);
+out:
+ pte_unmap_unlock(pte, ptl);
+ return 0;
+}
+
+static const struct mm_walk_ops damon_clean_soft_dirty_ops = {
+ .pmd_entry = damon_clean_soft_dirty_pmd_entry,
+};
+
+static void damon_va_clean_soft_dirty(struct mm_struct *mm, unsigned long addr)
+{
+ mmap_read_lock(mm);
+ walk_page_range(mm, addr, addr + 1, &damon_clean_soft_dirty_ops, NULL);
+ mmap_read_unlock(mm);
+}
+
static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr,
unsigned long next, struct mm_walk *walk)
{
@@ -415,7 +453,10 @@ static void damon_va_prepare_access_check(struct damon_ctx *ctx,
{
r->sampling_addr = damon_rand(r->ar.start, r->ar.end);
- damon_va_mkold(mm, r->sampling_addr);
+ if (ctx->counter_type == DAMOS_NUMBER_WRITES)
+ damon_va_clean_soft_dirty(mm, r->sampling_addr);
+ else
+ damon_va_mkold(mm, r->sampling_addr);
}
void damon_va_prepare_access_checks(struct damon_ctx *ctx)
@@ -437,6 +478,7 @@ void damon_va_prepare_access_checks(struct damon_ctx *ctx)
struct damon_young_walk_private {
unsigned long *page_sz;
bool young;
+ bool dirty;
};
static int damon_young_pmd_entry(pmd_t *pmd, unsigned long addr,
@@ -463,6 +505,10 @@ static int damon_young_pmd_entry(pmd_t *pmd, unsigned long addr,
*priv->page_sz = ((1UL) << HPAGE_PMD_SHIFT);
priv->young = true;
}
+ if (pmd_soft_dirty(*pmd)) {
+ *priv->page_sz = ((1UL) << HPAGE_PMD_SHIFT);
+ priv->dirty = true;
+ }
put_page(page);
huge_out:
spin_unlock(ptl);
@@ -485,6 +531,10 @@ static int damon_young_pmd_entry(pmd_t *pmd, unsigned long addr,
*priv->page_sz = PAGE_SIZE;
priv->young = true;
}
+ if (pte_soft_dirty(*pte)) {
+ *priv->page_sz = PAGE_SIZE;
+ priv->dirty = true;
+ }
put_page(page);
out:
pte_unmap_unlock(pte, ptl);
@@ -496,16 +546,19 @@ static const struct mm_walk_ops damon_young_ops = {
};
static bool damon_va_young(struct mm_struct *mm, unsigned long addr,
- unsigned long *page_sz)
+ unsigned long *page_sz, bool *dirty)
{
struct damon_young_walk_private arg = {
.page_sz = page_sz,
.young = false,
+ .dirty = false,
};
mmap_read_lock(mm);
walk_page_range(mm, addr, addr + 1, &damon_young_ops, &arg);
mmap_read_unlock(mm);
+
+ *dirty = arg.dirty;
return arg.young;
}
@@ -522,18 +575,23 @@ static void damon_va_check_access(struct damon_ctx *ctx,
static unsigned long last_addr;
static unsigned long last_page_sz = PAGE_SIZE;
static bool last_accessed;
+ static bool last_written;
/* If the region is in the last checked page, reuse the result */
if (mm == last_mm && (ALIGN_DOWN(last_addr, last_page_sz) ==
ALIGN_DOWN(r->sampling_addr, last_page_sz))) {
if (last_accessed)
r->nr_accesses++;
+ if (last_written)
+ r->nr_writes++;
return;
}
- last_accessed = damon_va_young(mm, r->sampling_addr, &last_page_sz);
+ last_accessed = damon_va_young(mm, r->sampling_addr, &last_page_sz, &last_written);
if (last_accessed)
r->nr_accesses++;
+ if (last_written)
+ r->nr_writes++;
last_mm = mm;
last_addr = r->sampling_addr;
@@ -544,7 +602,7 @@ unsigned int damon_va_check_accesses(struct damon_ctx *ctx)
struct damon_target *t;
struct mm_struct *mm;
struct damon_region *r;
- unsigned int max_nr_accesses = 0;
+ unsigned int max_nr = 0;
damon_for_each_target(t, ctx) {
mm = damon_get_mm(t);
@@ -552,12 +610,15 @@ unsigned int damon_va_check_accesses(struct damon_ctx *ctx)
continue;
damon_for_each_region(r, t) {
damon_va_check_access(ctx, mm, r);
- max_nr_accesses = max(r->nr_accesses, max_nr_accesses);
+ if (ctx->counter_type == DAMOS_NUMBER_WRITES)
+ max_nr = max(r->nr_writes, max_nr);
+ else
+ max_nr = max(r->nr_accesses, max_nr);
}
mmput(mm);
}
- return max_nr_accesses;
+ return max_nr;
}
/*
@@ -597,6 +658,7 @@ static int damos_madvise(struct damon_target *target, struct damon_region *r,
ret = do_madvise(mm, PAGE_ALIGN(r->ar.start),
PAGE_ALIGN(r->ar.end - r->ar.start), behavior);
+
mmput(mm);
out:
return ret;
@@ -624,6 +686,12 @@ int damon_va_apply_scheme(struct damon_ctx *ctx, struct damon_target *t,
case DAMOS_NOHUGEPAGE:
madv_action = MADV_NOHUGEPAGE;
break;
+ case DAMOS_MERGEABLE:
+ madv_action = MADV_MERGEABLE;
+ break;
+ case DAMOS_UNMERGEABLE:
+ madv_action = MADV_UNMERGEABLE;
+ break;
case DAMOS_STAT:
return 0;
default:
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/damon: Add option to monitor only writes
2022-02-03 13:12 [PATCH] mm/damon: Add option to monitor only writes Pedro Demarchi Gomes
@ 2022-02-03 13:39 ` SeongJae Park
2022-02-04 15:11 ` Pedro Gomes
2022-02-04 1:29 ` kernel test robot
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: SeongJae Park @ 2022-02-03 13:39 UTC (permalink / raw)
To: Pedro Demarchi Gomes
Cc: SeongJae Park, Steven Rostedt, Ingo Molnar, Andrew Morton,
linux-mm, linux-kernel
Hi Pedro,
Thank you for this patch!
On Thu, 3 Feb 2022 10:12:36 -0300 Pedro Demarchi Gomes <pedrodemargomes@gmail.com> wrote:
> When "writes" is written to /sys/kernel/debug/damon/counter_type damon will
> monitor only writes.
I think this would be better to be implemented as another monitoring primitive
based on the virtual address space monitoring primitives, e.g., vaddr-writes?
Then the implementation would be simpler and changes to other files will be
minimized. For the user space interface, we could use a prefix to target_ids
debugfs file. For example,
# echo "vaddr-writes $(pidof workload)" > $debugfs/damon/target_ids
> This patch also adds the actions mergeable and unmergeable to damos schemes.
> These actions are used by KSM as explained in [1].
Please do that in a separate patch, and also update the document
(Documentation/admin-guide/mm/damon/usage.rst). And, what's the expected usage
of the action and benefits?
Thanks,
SJ
>
> [1] https://www.kernel.org/doc/html/latest/admin-guide/mm/ksm.html#controlling-ksm-with-madvise
>
> Signed-off-by: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
> ---
> include/linux/damon.h | 13 ++++++
> include/trace/events/damon.h | 10 ++--
> mm/damon/core.c | 91 +++++++++++++++++++++++++-----------
> mm/damon/dbgfs.c | 72 +++++++++++++++++++++++++++-
> mm/damon/prmtv-common.c | 88 ++++++++++++++++++++++++++++++++++
> mm/damon/prmtv-common.h | 5 ++
> mm/damon/vaddr.c | 80 ++++++++++++++++++++++++++++---
> 7 files changed, 318 insertions(+), 41 deletions(-)
>
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index b4d4be3cc987..9efe89bbcec8 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -44,11 +44,13 @@ struct damon_region {
> struct damon_addr_range ar;
> unsigned long sampling_addr;
> unsigned int nr_accesses;
> + unsigned int nr_writes;
> struct list_head list;
>
> unsigned int age;
> /* private: Internal value for age calculation. */
> unsigned int last_nr_accesses;
> + unsigned int last_nr_writes;
> };
>
> /**
> @@ -88,6 +90,8 @@ enum damos_action {
> DAMOS_PAGEOUT,
> DAMOS_HUGEPAGE,
> DAMOS_NOHUGEPAGE,
> + DAMOS_MERGEABLE,
> + DAMOS_UNMERGEABLE,
> DAMOS_STAT, /* Do nothing but only record the stat */
> };
>
> @@ -185,6 +189,11 @@ struct damos_watermarks {
> bool activated;
> };
>
> +enum damos_counter_type {
> + DAMOS_NUMBER_ACCESSES,
> + DAMOS_NUMBER_WRITES,
> +};
> +
> /**
> * struct damos - Represents a Data Access Monitoring-based Operation Scheme.
> * @min_sz_region: Minimum size of target regions.
> @@ -223,6 +232,9 @@ struct damos {
> unsigned long max_sz_region;
> unsigned int min_nr_accesses;
> unsigned int max_nr_accesses;
> + unsigned int min_nr_writes;
> + unsigned int max_nr_writes;
> + enum damos_counter_type counter_type;
> unsigned int min_age_region;
> unsigned int max_age_region;
> enum damos_action action;
> @@ -386,6 +398,7 @@ struct damon_ctx {
> struct damon_primitive primitive;
> struct damon_callback callback;
>
> + enum damos_counter_type counter_type;
> unsigned long min_nr_regions;
> unsigned long max_nr_regions;
> struct list_head adaptive_targets;
> diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h
> index 2f422f4f1fb9..45573f421707 100644
> --- a/include/trace/events/damon.h
> +++ b/include/trace/events/damon.h
> @@ -11,17 +11,17 @@
>
> TRACE_EVENT(damon_aggregated,
>
> - TP_PROTO(struct damon_target *t, struct damon_region *r,
> + TP_PROTO(struct damon_ctx *c, struct damon_target *t, struct damon_region *r,
> unsigned int nr_regions),
>
> - TP_ARGS(t, r, nr_regions),
> + TP_ARGS(c, t, r, nr_regions),
>
> TP_STRUCT__entry(
> __field(unsigned long, target_id)
> __field(unsigned int, nr_regions)
> __field(unsigned long, start)
> __field(unsigned long, end)
> - __field(unsigned int, nr_accesses)
> + __field(unsigned int, nr)
> ),
>
> TP_fast_assign(
> @@ -29,12 +29,12 @@ TRACE_EVENT(damon_aggregated,
> __entry->nr_regions = nr_regions;
> __entry->start = r->ar.start;
> __entry->end = r->ar.end;
> - __entry->nr_accesses = r->nr_accesses;
> + __entry->nr = c->counter_type == DAMOS_NUMBER_WRITES ? r->nr_writes : r->nr_accesses;
> ),
>
> TP_printk("target_id=%lu nr_regions=%u %lu-%lu: %u",
> __entry->target_id, __entry->nr_regions,
> - __entry->start, __entry->end, __entry->nr_accesses)
> + __entry->start, __entry->end, __entry->nr)
> );
>
> #endif /* _TRACE_DAMON_H */
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index e92497895202..e827231366db 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -45,10 +45,12 @@ struct damon_region *damon_new_region(unsigned long start, unsigned long end)
> region->ar.start = start;
> region->ar.end = end;
> region->nr_accesses = 0;
> + region->nr_writes = 0;
> INIT_LIST_HEAD(®ion->list);
>
> region->age = 0;
> region->last_nr_accesses = 0;
> + region->last_nr_writes = 0;
>
> return region;
> }
> @@ -89,7 +91,7 @@ void damon_destroy_region(struct damon_region *r, struct damon_target *t)
>
> struct damos *damon_new_scheme(
> unsigned long min_sz_region, unsigned long max_sz_region,
> - unsigned int min_nr_accesses, unsigned int max_nr_accesses,
> + unsigned int min_nr, unsigned int max_nr,
> unsigned int min_age_region, unsigned int max_age_region,
> enum damos_action action, struct damos_quota *quota,
> struct damos_watermarks *wmarks)
> @@ -101,8 +103,10 @@ struct damos *damon_new_scheme(
> return NULL;
> scheme->min_sz_region = min_sz_region;
> scheme->max_sz_region = max_sz_region;
> - scheme->min_nr_accesses = min_nr_accesses;
> - scheme->max_nr_accesses = max_nr_accesses;
> + scheme->max_nr_writes = max_nr;
> + scheme->min_nr_writes = min_nr;
> + scheme->min_nr_accesses = min_nr;
> + scheme->max_nr_accesses = max_nr;
> scheme->min_age_region = min_age_region;
> scheme->max_age_region = max_age_region;
> scheme->action = action;
> @@ -221,6 +225,7 @@ struct damon_ctx *damon_new_ctx(void)
> ctx->sample_interval = 5 * 1000;
> ctx->aggr_interval = 100 * 1000;
> ctx->primitive_update_interval = 60 * 1000 * 1000;
> + ctx->counter_type = DAMOS_NUMBER_ACCESSES;
>
> ktime_get_coarse_ts64(&ctx->last_aggregation);
> ctx->last_primitive_update = ctx->last_aggregation;
> @@ -535,9 +540,11 @@ static void kdamond_reset_aggregated(struct damon_ctx *c)
> struct damon_region *r;
>
> damon_for_each_region(r, t) {
> - trace_damon_aggregated(t, r, damon_nr_regions(t));
> + trace_damon_aggregated(c, t, r, damon_nr_regions(t));
> r->last_nr_accesses = r->nr_accesses;
> r->nr_accesses = 0;
> + r->last_nr_writes = r->nr_writes;
> + r->nr_writes = 0;
> }
> }
> }
> @@ -546,21 +553,27 @@ static void damon_split_region_at(struct damon_ctx *ctx,
> struct damon_target *t, struct damon_region *r,
> unsigned long sz_r);
>
> -static bool __damos_valid_target(struct damon_region *r, struct damos *s)
> +static bool __damos_valid_target(struct damon_ctx *ctx, struct damon_region *r, struct damos *s)
> {
> unsigned long sz;
> -
> sz = r->ar.end - r->ar.start;
> - return s->min_sz_region <= sz && sz <= s->max_sz_region &&
> - s->min_nr_accesses <= r->nr_accesses &&
> - r->nr_accesses <= s->max_nr_accesses &&
> - s->min_age_region <= r->age && r->age <= s->max_age_region;
> +
> + if (ctx->counter_type == DAMOS_NUMBER_WRITES)
> + return s->min_sz_region <= sz && sz <= s->max_sz_region &&
> + s->min_nr_writes <= r->nr_writes &&
> + r->nr_writes <= s->max_nr_writes &&
> + s->min_age_region <= r->age && r->age <= s->max_age_region;
> + else
> + return s->min_sz_region <= sz && sz <= s->max_sz_region &&
> + s->min_nr_accesses <= r->nr_accesses &&
> + r->nr_accesses <= s->max_nr_accesses &&
> + s->min_age_region <= r->age && r->age <= s->max_age_region;
> }
>
> static bool damos_valid_target(struct damon_ctx *c, struct damon_target *t,
> struct damon_region *r, struct damos *s)
> {
> - bool ret = __damos_valid_target(r, s);
> + bool ret = __damos_valid_target(c, r, s);
>
> if (!ret || !s->quota.esz || !c->primitive.get_scheme_score)
> return ret;
> @@ -707,7 +720,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
> memset(quota->histogram, 0, sizeof(quota->histogram));
> damon_for_each_target(t, c) {
> damon_for_each_region(r, t) {
> - if (!__damos_valid_target(r, s))
> + if (!__damos_valid_target(c, r, s))
> continue;
> score = c->primitive.get_scheme_score(
> c, t, r, s);
> @@ -738,13 +751,18 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
> /*
> * Merge two adjacent regions into one region
> */
> -static void damon_merge_two_regions(struct damon_target *t,
> +static void damon_merge_two_regions(struct damon_ctx *c, struct damon_target *t,
> struct damon_region *l, struct damon_region *r)
> {
> unsigned long sz_l = sz_damon_region(l), sz_r = sz_damon_region(r);
>
> - l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
> + if (c->counter_type == DAMOS_NUMBER_WRITES)
> + l->nr_writes = (l->nr_writes * sz_l + r->nr_writes * sz_r) /
> (sz_l + sz_r);
> + else
> + l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
> + (sz_l + sz_r);
> +
> l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r);
> l->ar.end = r->ar.end;
> damon_destroy_region(r, t);
> @@ -759,23 +777,39 @@ static void damon_merge_two_regions(struct damon_target *t,
> * thres '->nr_accesses' diff threshold for the merge
> * sz_limit size upper limit of each region
> */
> -static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> +static void damon_merge_regions_of(struct damon_ctx *c, struct damon_target *t, unsigned int thres,
> unsigned long sz_limit)
> {
> struct damon_region *r, *prev = NULL, *next;
>
> - damon_for_each_region_safe(r, next, t) {
> - if (diff_of(r->nr_accesses, r->last_nr_accesses) > thres)
> - r->age = 0;
> - else
> - r->age++;
> -
> - if (prev && prev->ar.end == r->ar.start &&
> - diff_of(prev->nr_accesses, r->nr_accesses) <= thres &&
> - sz_damon_region(prev) + sz_damon_region(r) <= sz_limit)
> - damon_merge_two_regions(t, prev, r);
> - else
> - prev = r;
> + if (c->counter_type == DAMOS_NUMBER_WRITES) {
> + damon_for_each_region_safe(r, next, t) {
> + if (diff_of(r->nr_writes, r->last_nr_writes) > thres)
> + r->age = 0;
> + else
> + r->age++;
> +
> + if (prev && prev->ar.end == r->ar.start &&
> + diff_of(prev->nr_writes, r->nr_writes) <= thres &&
> + sz_damon_region(prev) + sz_damon_region(r) <= sz_limit)
> + damon_merge_two_regions(c, t, prev, r);
> + else
> + prev = r;
> + }
> + } else {
> + damon_for_each_region_safe(r, next, t) {
> + if (diff_of(r->nr_accesses, r->last_nr_accesses) > thres)
> + r->age = 0;
> + else
> + r->age++;
> +
> + if (prev && prev->ar.end == r->ar.start &&
> + diff_of(prev->nr_accesses, r->nr_accesses) <= thres &&
> + sz_damon_region(prev) + sz_damon_region(r) <= sz_limit)
> + damon_merge_two_regions(c, t, prev, r);
> + else
> + prev = r;
> + }
> }
> }
>
> @@ -796,7 +830,7 @@ static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
> struct damon_target *t;
>
> damon_for_each_target(t, c)
> - damon_merge_regions_of(t, threshold, sz_limit);
> + damon_merge_regions_of(c, t, threshold, sz_limit);
> }
>
> /*
> @@ -819,6 +853,7 @@ static void damon_split_region_at(struct damon_ctx *ctx,
>
> new->age = r->age;
> new->last_nr_accesses = r->last_nr_accesses;
> + new->last_nr_writes = r->last_nr_writes;
>
> damon_insert_region(new, r, damon_next_region(r), t);
> }
> diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
> index ad65436756af..6cf2501148f5 100644
> --- a/mm/damon/dbgfs.c
> +++ b/mm/damon/dbgfs.c
> @@ -166,6 +166,8 @@ static bool damos_action_valid(int action)
> case DAMOS_PAGEOUT:
> case DAMOS_HUGEPAGE:
> case DAMOS_NOHUGEPAGE:
> + case DAMOS_MERGEABLE:
> + case DAMOS_UNMERGEABLE:
> case DAMOS_STAT:
> return true;
> default:
> @@ -477,6 +479,66 @@ static ssize_t dbgfs_init_regions_read(struct file *file, char __user *buf,
> return len;
> }
>
> +static ssize_t dbgfs_counter_type_write(struct file *file,
> + const char __user *buf, size_t count, loff_t *ppos)
> +{
> + struct damon_ctx *ctx = file->private_data;
> + char *kbuf;
> + ssize_t ret;
> +
> + mutex_lock(&ctx->kdamond_lock);
> + if (ctx->kdamond) {
> + mutex_unlock(&ctx->kdamond_lock);
> + ret = -EBUSY;
> + goto out;
> + }
> + ret = count;
> + kbuf = user_input_str(buf, count, ppos);
> + if (IS_ERR(kbuf))
> + return PTR_ERR(kbuf);
> +
> + if (!strncmp(kbuf, "writes\n", count))
> + ctx->counter_type = DAMOS_NUMBER_WRITES;
> + else
> + ctx->counter_type = DAMOS_NUMBER_ACCESSES;
> +
> + mutex_unlock(&ctx->kdamond_lock);
> +out:
> + kfree(kbuf);
> + return ret;
> +}
> +
> +static ssize_t dbgfs_counter_type_read(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct damon_ctx *ctx = file->private_data;
> + char *kbuf;
> + ssize_t len;
> +
> + kbuf = kmalloc(count, GFP_KERNEL | __GFP_NOWARN);
> + if (!kbuf)
> + return -ENOMEM;
> +
> + mutex_lock(&ctx->kdamond_lock);
> + if (ctx->kdamond) {
> + mutex_unlock(&ctx->kdamond_lock);
> + len = -EBUSY;
> + goto out;
> + }
> +
> + if (ctx->counter_type == DAMOS_NUMBER_WRITES)
> + len = scnprintf(kbuf, count, "writes");
> + else
> + len = scnprintf(kbuf, count, "accesses");
> + mutex_unlock(&ctx->kdamond_lock);
> + len = simple_read_from_buffer(buf, count, ppos, kbuf, len);
> +
> +out:
> + kfree(kbuf);
> + return len;
> +}
> +
> +
> static int add_init_region(struct damon_ctx *c,
> unsigned long target_id, struct damon_addr_range *ar)
> {
> @@ -636,12 +698,18 @@ static const struct file_operations kdamond_pid_fops = {
> .read = dbgfs_kdamond_pid_read,
> };
>
> +static const struct file_operations counter_type_fops = {
> + .open = damon_dbgfs_open,
> + .read = dbgfs_counter_type_read,
> + .write = dbgfs_counter_type_write,
> +};
> +
> static void dbgfs_fill_ctx_dir(struct dentry *dir, struct damon_ctx *ctx)
> {
> const char * const file_names[] = {"attrs", "schemes", "target_ids",
> - "init_regions", "kdamond_pid"};
> + "init_regions", "kdamond_pid", "counter_type"};
> const struct file_operations *fops[] = {&attrs_fops, &schemes_fops,
> - &target_ids_fops, &init_regions_fops, &kdamond_pid_fops};
> + &target_ids_fops, &init_regions_fops, &kdamond_pid_fops, &counter_type_fops};
> int i;
>
> for (i = 0; i < ARRAY_SIZE(file_names); i++)
> diff --git a/mm/damon/prmtv-common.c b/mm/damon/prmtv-common.c
> index 92a04f5831d6..09ba2b5b895e 100644
> --- a/mm/damon/prmtv-common.c
> +++ b/mm/damon/prmtv-common.c
> @@ -9,6 +9,8 @@
> #include <linux/page_idle.h>
> #include <linux/pagemap.h>
> #include <linux/rmap.h>
> +#include <linux/swap.h>
> +#include <linux/swapops.h>
>
> #include "prmtv-common.h"
>
> @@ -58,6 +60,92 @@ void damon_ptep_mkold(pte_t *pte, struct mm_struct *mm, unsigned long addr)
> put_page(page);
> }
>
> +static inline bool pte_is_pinned(struct vm_area_struct *vma, unsigned long addr, pte_t pte)
> +{
> + struct page *page;
> +
> + if (!pte_write(pte))
> + return false;
> + if (!is_cow_mapping(vma->vm_flags))
> + return false;
> + if (likely(!test_bit(MMF_HAS_PINNED, &vma->vm_mm->flags)))
> + return false;
> + page = vm_normal_page(vma, addr, pte);
> + if (!page)
> + return false;
> + return page_maybe_dma_pinned(page);
> +}
> +
> +static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma,
> + unsigned long addr, pmd_t *pmdp)
> +{
> + pmd_t old, pmd = *pmdp;
> +
> + if (pmd_present(pmd)) {
> + /* See comment in change_huge_pmd() */
> + old = pmdp_invalidate(vma, addr, pmdp);
> + if (pmd_dirty(old))
> + pmd = pmd_mkdirty(pmd);
> + if (pmd_young(old))
> + pmd = pmd_mkyoung(pmd);
> +
> + pmd = pmd_wrprotect(pmd);
> + pmd = pmd_clear_soft_dirty(pmd);
> +
> + set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
> + } else if (is_migration_entry(pmd_to_swp_entry(pmd))) {
> + pmd = pmd_swp_clear_soft_dirty(pmd);
> + set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
> + }
> +}
> +
> +static inline void clear_soft_dirty(struct vm_area_struct *vma,
> + unsigned long addr, pte_t *pte)
> +{
> + /*
> + * The soft-dirty tracker uses #PF-s to catch writes
> + * to pages, so write-protect the pte as well. See the
> + * Documentation/admin-guide/mm/soft-dirty.rst for full description
> + * of how soft-dirty works.
> + */
> + pte_t ptent = *pte;
> +
> + if (pte_present(ptent)) {
> + pte_t old_pte;
> +
> + if (pte_is_pinned(vma, addr, ptent))
> + return;
> + old_pte = ptep_modify_prot_start(vma, addr, pte);
> + ptent = pte_wrprotect(old_pte);
> + ptent = pte_clear_soft_dirty(ptent);
> + ptep_modify_prot_commit(vma, addr, pte, old_pte, ptent);
> + } else if (is_swap_pte(ptent)) {
> + ptent = pte_swp_clear_soft_dirty(ptent);
> + set_pte_at(vma->vm_mm, addr, pte, ptent);
> + }
> +}
> +
> +void damon_pmd_clean_soft_dirty(struct vm_area_struct *vma, unsigned long addr,
> + pmd_t *pmd)
> +{
> + vma->vm_flags &= ~VM_SOFTDIRTY;
> + vma_set_page_prot(vma);
> +
> + if (pmd_soft_dirty(*pmd))
> + clear_soft_dirty_pmd(vma, addr, pmd);
> +
> +}
> +
> +void damon_ptep_clean_soft_dirty(struct vm_area_struct *vma, unsigned long addr,
> + pte_t *pte)
> +{
> + vma->vm_flags &= ~VM_SOFTDIRTY;
> + vma_set_page_prot(vma);
> +
> + if (pte_soft_dirty(*pte))
> + clear_soft_dirty(vma, addr, pte);
> +}
> +
> void damon_pmdp_mkold(pmd_t *pmd, struct mm_struct *mm, unsigned long addr)
> {
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> diff --git a/mm/damon/prmtv-common.h b/mm/damon/prmtv-common.h
> index 61f27037603e..03847d149f0e 100644
> --- a/mm/damon/prmtv-common.h
> +++ b/mm/damon/prmtv-common.h
> @@ -13,6 +13,11 @@
>
> struct page *damon_get_page(unsigned long pfn);
>
> +void damon_ptep_clean_soft_dirty(struct vm_area_struct *vma, unsigned long addr,
> + pte_t *pte);
> +void damon_pmd_clean_soft_dirty(struct vm_area_struct *vma, unsigned long addr,
> + pmd_t *pmd);
> +
> void damon_ptep_mkold(pte_t *pte, struct mm_struct *mm, unsigned long addr);
> void damon_pmdp_mkold(pmd_t *pmd, struct mm_struct *mm, unsigned long addr);
>
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 20a9a9d69eb1..a7d9c9563d1d 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -368,6 +368,44 @@ void damon_va_update(struct damon_ctx *ctx)
> }
> }
>
> +static int damon_clean_soft_dirty_pmd_entry(pmd_t *pmd, unsigned long addr,
> + unsigned long next, struct mm_walk *walk)
> +{
> + pte_t *pte;
> + spinlock_t *ptl;
> +
> + if (pmd_huge(*pmd)) {
> + ptl = pmd_lock(walk->mm, pmd);
> + if (pmd_huge(*pmd)) {
> + damon_pmd_clean_soft_dirty(walk->vma, addr, pmd);
> + spin_unlock(ptl);
> + return 0;
> + }
> + spin_unlock(ptl);
> + }
> +
> + if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
> + return 0;
> + pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl);
> + if (!pte_present(*pte))
> + goto out;
> + damon_ptep_clean_soft_dirty(walk->vma, addr, pte);
> +out:
> + pte_unmap_unlock(pte, ptl);
> + return 0;
> +}
> +
> +static const struct mm_walk_ops damon_clean_soft_dirty_ops = {
> + .pmd_entry = damon_clean_soft_dirty_pmd_entry,
> +};
> +
> +static void damon_va_clean_soft_dirty(struct mm_struct *mm, unsigned long addr)
> +{
> + mmap_read_lock(mm);
> + walk_page_range(mm, addr, addr + 1, &damon_clean_soft_dirty_ops, NULL);
> + mmap_read_unlock(mm);
> +}
> +
> static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> @@ -415,7 +453,10 @@ static void damon_va_prepare_access_check(struct damon_ctx *ctx,
> {
> r->sampling_addr = damon_rand(r->ar.start, r->ar.end);
>
> - damon_va_mkold(mm, r->sampling_addr);
> + if (ctx->counter_type == DAMOS_NUMBER_WRITES)
> + damon_va_clean_soft_dirty(mm, r->sampling_addr);
> + else
> + damon_va_mkold(mm, r->sampling_addr);
> }
>
> void damon_va_prepare_access_checks(struct damon_ctx *ctx)
> @@ -437,6 +478,7 @@ void damon_va_prepare_access_checks(struct damon_ctx *ctx)
> struct damon_young_walk_private {
> unsigned long *page_sz;
> bool young;
> + bool dirty;
> };
>
> static int damon_young_pmd_entry(pmd_t *pmd, unsigned long addr,
> @@ -463,6 +505,10 @@ static int damon_young_pmd_entry(pmd_t *pmd, unsigned long addr,
> *priv->page_sz = ((1UL) << HPAGE_PMD_SHIFT);
> priv->young = true;
> }
> + if (pmd_soft_dirty(*pmd)) {
> + *priv->page_sz = ((1UL) << HPAGE_PMD_SHIFT);
> + priv->dirty = true;
> + }
> put_page(page);
> huge_out:
> spin_unlock(ptl);
> @@ -485,6 +531,10 @@ static int damon_young_pmd_entry(pmd_t *pmd, unsigned long addr,
> *priv->page_sz = PAGE_SIZE;
> priv->young = true;
> }
> + if (pte_soft_dirty(*pte)) {
> + *priv->page_sz = PAGE_SIZE;
> + priv->dirty = true;
> + }
> put_page(page);
> out:
> pte_unmap_unlock(pte, ptl);
> @@ -496,16 +546,19 @@ static const struct mm_walk_ops damon_young_ops = {
> };
>
> static bool damon_va_young(struct mm_struct *mm, unsigned long addr,
> - unsigned long *page_sz)
> + unsigned long *page_sz, bool *dirty)
> {
> struct damon_young_walk_private arg = {
> .page_sz = page_sz,
> .young = false,
> + .dirty = false,
> };
>
> mmap_read_lock(mm);
> walk_page_range(mm, addr, addr + 1, &damon_young_ops, &arg);
> mmap_read_unlock(mm);
> +
> + *dirty = arg.dirty;
> return arg.young;
> }
>
> @@ -522,18 +575,23 @@ static void damon_va_check_access(struct damon_ctx *ctx,
> static unsigned long last_addr;
> static unsigned long last_page_sz = PAGE_SIZE;
> static bool last_accessed;
> + static bool last_written;
>
> /* If the region is in the last checked page, reuse the result */
> if (mm == last_mm && (ALIGN_DOWN(last_addr, last_page_sz) ==
> ALIGN_DOWN(r->sampling_addr, last_page_sz))) {
> if (last_accessed)
> r->nr_accesses++;
> + if (last_written)
> + r->nr_writes++;
> return;
> }
>
> - last_accessed = damon_va_young(mm, r->sampling_addr, &last_page_sz);
> + last_accessed = damon_va_young(mm, r->sampling_addr, &last_page_sz, &last_written);
> if (last_accessed)
> r->nr_accesses++;
> + if (last_written)
> + r->nr_writes++;
>
> last_mm = mm;
> last_addr = r->sampling_addr;
> @@ -544,7 +602,7 @@ unsigned int damon_va_check_accesses(struct damon_ctx *ctx)
> struct damon_target *t;
> struct mm_struct *mm;
> struct damon_region *r;
> - unsigned int max_nr_accesses = 0;
> + unsigned int max_nr = 0;
>
> damon_for_each_target(t, ctx) {
> mm = damon_get_mm(t);
> @@ -552,12 +610,15 @@ unsigned int damon_va_check_accesses(struct damon_ctx *ctx)
> continue;
> damon_for_each_region(r, t) {
> damon_va_check_access(ctx, mm, r);
> - max_nr_accesses = max(r->nr_accesses, max_nr_accesses);
> + if (ctx->counter_type == DAMOS_NUMBER_WRITES)
> + max_nr = max(r->nr_writes, max_nr);
> + else
> + max_nr = max(r->nr_accesses, max_nr);
> }
> mmput(mm);
> }
>
> - return max_nr_accesses;
> + return max_nr;
> }
>
> /*
> @@ -597,6 +658,7 @@ static int damos_madvise(struct damon_target *target, struct damon_region *r,
>
> ret = do_madvise(mm, PAGE_ALIGN(r->ar.start),
> PAGE_ALIGN(r->ar.end - r->ar.start), behavior);
> +
> mmput(mm);
> out:
> return ret;
> @@ -624,6 +686,12 @@ int damon_va_apply_scheme(struct damon_ctx *ctx, struct damon_target *t,
> case DAMOS_NOHUGEPAGE:
> madv_action = MADV_NOHUGEPAGE;
> break;
> + case DAMOS_MERGEABLE:
> + madv_action = MADV_MERGEABLE;
> + break;
> + case DAMOS_UNMERGEABLE:
> + madv_action = MADV_UNMERGEABLE;
> + break;
> case DAMOS_STAT:
> return 0;
> default:
> --
> 2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/damon: Add option to monitor only writes
2022-02-03 13:12 [PATCH] mm/damon: Add option to monitor only writes Pedro Demarchi Gomes
2022-02-03 13:39 ` SeongJae Park
@ 2022-02-04 1:29 ` kernel test robot
2022-02-04 1:29 ` kernel test robot
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2022-02-04 1:29 UTC (permalink / raw)
To: Pedro Demarchi Gomes
Cc: kbuild-all, Pedro Demarchi Gomes, SeongJae Park, Steven Rostedt,
Ingo Molnar, Andrew Morton, Linux Memory Management List,
linux-kernel
Hi Pedro,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on hnaz-mm/master]
url: https://github.com/0day-ci/linux/commits/Pedro-Demarchi-Gomes/mm-damon-Add-option-to-monitor-only-writes/20220203-211407
base: https://github.com/hnaz/linux-mm master
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220204/202202040109.d4u6Jwvm-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/0958bb7e2514b62f174b8e5ccfdcff07ce2a2b6b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Pedro-Demarchi-Gomes/mm-damon-Add-option-to-monitor-only-writes/20220203-211407
git checkout 0958bb7e2514b62f174b8e5ccfdcff07ce2a2b6b
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash mm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from mm/damon/core.c:1123:
mm/damon/core-test.h: In function 'damon_test_merge_two':
mm/damon/core-test.h:154:33: error: passing argument 1 of 'damon_merge_two_regions' from incompatible pointer type [-Werror=incompatible-pointer-types]
154 | damon_merge_two_regions(t, r, r2);
| ^
| |
| struct damon_target *
mm/damon/core.c:761:55: note: expected 'struct damon_ctx *' but argument is of type 'struct damon_target *'
761 | static void damon_merge_two_regions(struct damon_ctx *c, struct damon_target *t,
| ~~~~~~~~~~~~~~~~~~^
In file included from mm/damon/core.c:1123:
mm/damon/core-test.h:154:36: error: passing argument 2 of 'damon_merge_two_regions' from incompatible pointer type [-Werror=incompatible-pointer-types]
154 | damon_merge_two_regions(t, r, r2);
| ^
| |
| struct damon_region *
mm/damon/core.c:761:79: note: expected 'struct damon_target *' but argument is of type 'struct damon_region *'
761 | static void damon_merge_two_regions(struct damon_ctx *c, struct damon_target *t,
| ~~~~~~~~~~~~~~~~~~~~~^
In file included from mm/damon/core.c:1123:
mm/damon/core-test.h:154:9: error: too few arguments to function 'damon_merge_two_regions'
154 | damon_merge_two_regions(t, r, r2);
| ^~~~~~~~~~~~~~~~~~~~~~~
mm/damon/core.c:761:13: note: declared here
761 | static void damon_merge_two_regions(struct damon_ctx *c, struct damon_target *t,
| ^~~~~~~~~~~~~~~~~~~~~~~
In file included from mm/damon/core.c:1123:
mm/damon/core-test.h: In function 'damon_test_merge_regions_of':
mm/damon/core-test.h:201:32: error: passing argument 1 of 'damon_merge_regions_of' from incompatible pointer type [-Werror=incompatible-pointer-types]
201 | damon_merge_regions_of(t, 9, 9999);
| ^
| |
| struct damon_target *
mm/damon/core.c:787:54: note: expected 'struct damon_ctx *' but argument is of type 'struct damon_target *'
787 | static void damon_merge_regions_of(struct damon_ctx *c, struct damon_target *t, unsigned int thres,
| ~~~~~~~~~~~~~~~~~~^
In file included from mm/damon/core.c:1123:
>> mm/damon/core-test.h:201:35: warning: passing argument 2 of 'damon_merge_regions_of' makes pointer from integer without a cast [-Wint-conversion]
201 | damon_merge_regions_of(t, 9, 9999);
| ^
| |
| int
mm/damon/core.c:787:78: note: expected 'struct damon_target *' but argument is of type 'int'
787 | static void damon_merge_regions_of(struct damon_ctx *c, struct damon_target *t, unsigned int thres,
| ~~~~~~~~~~~~~~~~~~~~~^
In file included from mm/damon/core.c:1123:
mm/damon/core-test.h:201:9: error: too few arguments to function 'damon_merge_regions_of'
201 | damon_merge_regions_of(t, 9, 9999);
| ^~~~~~~~~~~~~~~~~~~~~~
mm/damon/core.c:787:13: note: declared here
787 | static void damon_merge_regions_of(struct damon_ctx *c, struct damon_target *t, unsigned int thres,
| ^~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/trace_events.h:21,
from include/trace/define_trace.h:102,
from include/trace/events/damon.h:43,
from mm/damon/core.c:19:
At top level:
arch/arc/include/asm/perf_event.h:126:27: warning: 'arc_pmu_cache_map' defined but not used [-Wunused-const-variable=]
126 | static const unsigned int arc_pmu_cache_map[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
| ^~~~~~~~~~~~~~~~~
arch/arc/include/asm/perf_event.h:91:27: warning: 'arc_pmu_ev_hw_map' defined but not used [-Wunused-const-variable=]
91 | static const char * const arc_pmu_ev_hw_map[] = {
| ^~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/damon_merge_regions_of +201 mm/damon/core-test.h
17ccae8bb5c9289 SeongJae Park 2021-09-07 181
17ccae8bb5c9289 SeongJae Park 2021-09-07 182 static void damon_test_merge_regions_of(struct kunit *test)
17ccae8bb5c9289 SeongJae Park 2021-09-07 183 {
17ccae8bb5c9289 SeongJae Park 2021-09-07 184 struct damon_target *t;
17ccae8bb5c9289 SeongJae Park 2021-09-07 185 struct damon_region *r;
17ccae8bb5c9289 SeongJae Park 2021-09-07 186 unsigned long sa[] = {0, 100, 114, 122, 130, 156, 170, 184};
17ccae8bb5c9289 SeongJae Park 2021-09-07 187 unsigned long ea[] = {100, 112, 122, 130, 156, 170, 184, 230};
17ccae8bb5c9289 SeongJae Park 2021-09-07 188 unsigned int nrs[] = {0, 0, 10, 10, 20, 30, 1, 2};
17ccae8bb5c9289 SeongJae Park 2021-09-07 189
17ccae8bb5c9289 SeongJae Park 2021-09-07 190 unsigned long saddrs[] = {0, 114, 130, 156, 170};
17ccae8bb5c9289 SeongJae Park 2021-09-07 191 unsigned long eaddrs[] = {112, 130, 156, 170, 230};
17ccae8bb5c9289 SeongJae Park 2021-09-07 192 int i;
17ccae8bb5c9289 SeongJae Park 2021-09-07 193
17ccae8bb5c9289 SeongJae Park 2021-09-07 194 t = damon_new_target(42);
17ccae8bb5c9289 SeongJae Park 2021-09-07 195 for (i = 0; i < ARRAY_SIZE(sa); i++) {
17ccae8bb5c9289 SeongJae Park 2021-09-07 196 r = damon_new_region(sa[i], ea[i]);
17ccae8bb5c9289 SeongJae Park 2021-09-07 197 r->nr_accesses = nrs[i];
17ccae8bb5c9289 SeongJae Park 2021-09-07 198 damon_add_region(r, t);
17ccae8bb5c9289 SeongJae Park 2021-09-07 199 }
17ccae8bb5c9289 SeongJae Park 2021-09-07 200
17ccae8bb5c9289 SeongJae Park 2021-09-07 @201 damon_merge_regions_of(t, 9, 9999);
17ccae8bb5c9289 SeongJae Park 2021-09-07 202 /* 0-112, 114-130, 130-156, 156-170 */
17ccae8bb5c9289 SeongJae Park 2021-09-07 203 KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
17ccae8bb5c9289 SeongJae Park 2021-09-07 204 for (i = 0; i < 5; i++) {
17ccae8bb5c9289 SeongJae Park 2021-09-07 205 r = __nth_region_of(t, i);
17ccae8bb5c9289 SeongJae Park 2021-09-07 206 KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
17ccae8bb5c9289 SeongJae Park 2021-09-07 207 KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
17ccae8bb5c9289 SeongJae Park 2021-09-07 208 }
17ccae8bb5c9289 SeongJae Park 2021-09-07 209 damon_free_target(t);
17ccae8bb5c9289 SeongJae Park 2021-09-07 210 }
17ccae8bb5c9289 SeongJae Park 2021-09-07 211
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/damon: Add option to monitor only writes
2022-02-03 13:12 [PATCH] mm/damon: Add option to monitor only writes Pedro Demarchi Gomes
2022-02-03 13:39 ` SeongJae Park
2022-02-04 1:29 ` kernel test robot
@ 2022-02-04 1:29 ` kernel test robot
2022-02-04 1:30 ` kernel test robot
2022-02-07 10:32 ` David Hildenbrand
4 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2022-02-04 1:29 UTC (permalink / raw)
To: Pedro Demarchi Gomes
Cc: llvm, kbuild-all, Pedro Demarchi Gomes, SeongJae Park,
Steven Rostedt, Ingo Molnar, Andrew Morton,
Linux Memory Management List, linux-kernel
Hi Pedro,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on hnaz-mm/master]
url: https://github.com/0day-ci/linux/commits/Pedro-Demarchi-Gomes/mm-damon-Add-option-to-monitor-only-writes/20220203-211407
base: https://github.com/hnaz/linux-mm master
config: hexagon-randconfig-r041-20220130 (https://download.01.org/0day-ci/archive/20220204/202202040414.4fF2j3mp-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a73e4ce6a59b01f0e37037761c1e6889d539d233)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/0958bb7e2514b62f174b8e5ccfdcff07ce2a2b6b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Pedro-Demarchi-Gomes/mm-damon-Add-option-to-monitor-only-writes/20220203-211407
git checkout 0958bb7e2514b62f174b8e5ccfdcff07ce2a2b6b
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> mm/damon/prmtv-common.c:87:7: error: implicit declaration of function 'pmd_dirty' [-Werror,-Wimplicit-function-declaration]
if (pmd_dirty(old))
^
mm/damon/prmtv-common.c:87:7: note: did you mean 'pte_dirty'?
arch/hexagon/include/asm/pgtable.h:301:19: note: 'pte_dirty' declared here
static inline int pte_dirty(pte_t pte)
^
>> mm/damon/prmtv-common.c:88:10: error: implicit declaration of function 'pmd_mkdirty' [-Werror,-Wimplicit-function-declaration]
pmd = pmd_mkdirty(pmd);
^
>> mm/damon/prmtv-common.c:88:8: error: assigning to 'pmd_t' from incompatible type 'int'
pmd = pmd_mkdirty(pmd);
^ ~~~~~~~~~~~~~~~~
>> mm/damon/prmtv-common.c:89:7: error: implicit declaration of function 'pmd_young' [-Werror,-Wimplicit-function-declaration]
if (pmd_young(old))
^
mm/damon/prmtv-common.c:89:7: note: did you mean 'pte_young'?
arch/hexagon/include/asm/pgtable.h:295:19: note: 'pte_young' declared here
static inline int pte_young(pte_t pte)
^
>> mm/damon/prmtv-common.c:90:10: error: implicit declaration of function 'pmd_mkyoung' [-Werror,-Wimplicit-function-declaration]
pmd = pmd_mkyoung(pmd);
^
mm/damon/prmtv-common.c:90:8: error: assigning to 'pmd_t' from incompatible type 'int'
pmd = pmd_mkyoung(pmd);
^ ~~~~~~~~~~~~~~~~
>> mm/damon/prmtv-common.c:92:9: error: implicit declaration of function 'pmd_wrprotect' [-Werror,-Wimplicit-function-declaration]
pmd = pmd_wrprotect(pmd);
^
mm/damon/prmtv-common.c:92:9: note: did you mean 'pte_wrprotect'?
arch/hexagon/include/asm/pgtable.h:315:21: note: 'pte_wrprotect' declared here
static inline pte_t pte_wrprotect(pte_t pte)
^
mm/damon/prmtv-common.c:92:7: error: assigning to 'pmd_t' from incompatible type 'int'
pmd = pmd_wrprotect(pmd);
^ ~~~~~~~~~~~~~~~~~~
>> mm/damon/prmtv-common.c:95:3: error: implicit declaration of function 'set_pmd_at' [-Werror,-Wimplicit-function-declaration]
set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
^
mm/damon/prmtv-common.c:98:3: error: implicit declaration of function 'set_pmd_at' [-Werror,-Wimplicit-function-declaration]
set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
^
10 errors generated.
vim +/pmd_dirty +87 mm/damon/prmtv-common.c
78
79 static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma,
80 unsigned long addr, pmd_t *pmdp)
81 {
82 pmd_t old, pmd = *pmdp;
83
84 if (pmd_present(pmd)) {
85 /* See comment in change_huge_pmd() */
86 old = pmdp_invalidate(vma, addr, pmdp);
> 87 if (pmd_dirty(old))
> 88 pmd = pmd_mkdirty(pmd);
> 89 if (pmd_young(old))
> 90 pmd = pmd_mkyoung(pmd);
91
> 92 pmd = pmd_wrprotect(pmd);
93 pmd = pmd_clear_soft_dirty(pmd);
94
> 95 set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
96 } else if (is_migration_entry(pmd_to_swp_entry(pmd))) {
97 pmd = pmd_swp_clear_soft_dirty(pmd);
98 set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
99 }
100 }
101
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/damon: Add option to monitor only writes
2022-02-03 13:12 [PATCH] mm/damon: Add option to monitor only writes Pedro Demarchi Gomes
` (2 preceding siblings ...)
2022-02-04 1:29 ` kernel test robot
@ 2022-02-04 1:30 ` kernel test robot
2022-02-07 10:32 ` David Hildenbrand
4 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2022-02-04 1:30 UTC (permalink / raw)
To: Pedro Demarchi Gomes
Cc: kbuild-all, Pedro Demarchi Gomes, SeongJae Park, Steven Rostedt,
Ingo Molnar, Andrew Morton, Linux Memory Management List,
linux-kernel
Hi Pedro,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on hnaz-mm/master]
url: https://github.com/0day-ci/linux/commits/Pedro-Demarchi-Gomes/mm-damon-Add-option-to-monitor-only-writes/20220203-211407
base: https://github.com/hnaz/linux-mm master
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220204/202202040626.isVzU9Xl-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/0958bb7e2514b62f174b8e5ccfdcff07ce2a2b6b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Pedro-Demarchi-Gomes/mm-damon-Add-option-to-monitor-only-writes/20220203-211407
git checkout 0958bb7e2514b62f174b8e5ccfdcff07ce2a2b6b
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from mm/damon/core.c:1123:
mm/damon/core-test.h: In function 'damon_test_merge_two':
>> mm/damon/core-test.h:154:33: error: passing argument 1 of 'damon_merge_two_regions' from incompatible pointer type [-Werror=incompatible-pointer-types]
154 | damon_merge_two_regions(t, r, r2);
| ^
| |
| struct damon_target *
mm/damon/core.c:761:55: note: expected 'struct damon_ctx *' but argument is of type 'struct damon_target *'
761 | static void damon_merge_two_regions(struct damon_ctx *c, struct damon_target *t,
| ~~~~~~~~~~~~~~~~~~^
In file included from mm/damon/core.c:1123:
mm/damon/core-test.h:154:36: error: passing argument 2 of 'damon_merge_two_regions' from incompatible pointer type [-Werror=incompatible-pointer-types]
154 | damon_merge_two_regions(t, r, r2);
| ^
| |
| struct damon_region *
mm/damon/core.c:761:79: note: expected 'struct damon_target *' but argument is of type 'struct damon_region *'
761 | static void damon_merge_two_regions(struct damon_ctx *c, struct damon_target *t,
| ~~~~~~~~~~~~~~~~~~~~~^
In file included from mm/damon/core.c:1123:
>> mm/damon/core-test.h:154:9: error: too few arguments to function 'damon_merge_two_regions'
154 | damon_merge_two_regions(t, r, r2);
| ^~~~~~~~~~~~~~~~~~~~~~~
mm/damon/core.c:761:13: note: declared here
761 | static void damon_merge_two_regions(struct damon_ctx *c, struct damon_target *t,
| ^~~~~~~~~~~~~~~~~~~~~~~
In file included from mm/damon/core.c:1123:
mm/damon/core-test.h: In function 'damon_test_merge_regions_of':
>> mm/damon/core-test.h:201:32: error: passing argument 1 of 'damon_merge_regions_of' from incompatible pointer type [-Werror=incompatible-pointer-types]
201 | damon_merge_regions_of(t, 9, 9999);
| ^
| |
| struct damon_target *
mm/damon/core.c:787:54: note: expected 'struct damon_ctx *' but argument is of type 'struct damon_target *'
787 | static void damon_merge_regions_of(struct damon_ctx *c, struct damon_target *t, unsigned int thres,
| ~~~~~~~~~~~~~~~~~~^
In file included from mm/damon/core.c:1123:
mm/damon/core-test.h:201:35: warning: passing argument 2 of 'damon_merge_regions_of' makes pointer from integer without a cast [-Wint-conversion]
201 | damon_merge_regions_of(t, 9, 9999);
| ^
| |
| int
mm/damon/core.c:787:78: note: expected 'struct damon_target *' but argument is of type 'int'
787 | static void damon_merge_regions_of(struct damon_ctx *c, struct damon_target *t, unsigned int thres,
| ~~~~~~~~~~~~~~~~~~~~~^
In file included from mm/damon/core.c:1123:
>> mm/damon/core-test.h:201:9: error: too few arguments to function 'damon_merge_regions_of'
201 | damon_merge_regions_of(t, 9, 9999);
| ^~~~~~~~~~~~~~~~~~~~~~
mm/damon/core.c:787:13: note: declared here
787 | static void damon_merge_regions_of(struct damon_ctx *c, struct damon_target *t, unsigned int thres,
| ^~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/perf_event.h:25,
from include/linux/trace_events.h:10,
from include/trace/trace_events.h:21,
from include/trace/define_trace.h:102,
from include/trace/events/damon.h:43,
from mm/damon/core.c:19:
At top level:
arch/arc/include/asm/perf_event.h:126:27: warning: 'arc_pmu_cache_map' defined but not used [-Wunused-const-variable=]
126 | static const unsigned int arc_pmu_cache_map[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
| ^~~~~~~~~~~~~~~~~
arch/arc/include/asm/perf_event.h:91:27: warning: 'arc_pmu_ev_hw_map' defined but not used [-Wunused-const-variable=]
91 | static const char * const arc_pmu_ev_hw_map[] = {
| ^~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/damon_merge_two_regions +154 mm/damon/core-test.h
17ccae8bb5c928 SeongJae Park 2021-09-07 139
17ccae8bb5c928 SeongJae Park 2021-09-07 140 static void damon_test_merge_two(struct kunit *test)
17ccae8bb5c928 SeongJae Park 2021-09-07 141 {
17ccae8bb5c928 SeongJae Park 2021-09-07 142 struct damon_target *t;
17ccae8bb5c928 SeongJae Park 2021-09-07 143 struct damon_region *r, *r2, *r3;
17ccae8bb5c928 SeongJae Park 2021-09-07 144 int i;
17ccae8bb5c928 SeongJae Park 2021-09-07 145
17ccae8bb5c928 SeongJae Park 2021-09-07 146 t = damon_new_target(42);
17ccae8bb5c928 SeongJae Park 2021-09-07 147 r = damon_new_region(0, 100);
17ccae8bb5c928 SeongJae Park 2021-09-07 148 r->nr_accesses = 10;
17ccae8bb5c928 SeongJae Park 2021-09-07 149 damon_add_region(r, t);
17ccae8bb5c928 SeongJae Park 2021-09-07 150 r2 = damon_new_region(100, 300);
17ccae8bb5c928 SeongJae Park 2021-09-07 151 r2->nr_accesses = 20;
17ccae8bb5c928 SeongJae Park 2021-09-07 152 damon_add_region(r2, t);
17ccae8bb5c928 SeongJae Park 2021-09-07 153
17ccae8bb5c928 SeongJae Park 2021-09-07 @154 damon_merge_two_regions(t, r, r2);
17ccae8bb5c928 SeongJae Park 2021-09-07 155 KUNIT_EXPECT_EQ(test, r->ar.start, 0ul);
17ccae8bb5c928 SeongJae Park 2021-09-07 156 KUNIT_EXPECT_EQ(test, r->ar.end, 300ul);
17ccae8bb5c928 SeongJae Park 2021-09-07 157 KUNIT_EXPECT_EQ(test, r->nr_accesses, 16u);
17ccae8bb5c928 SeongJae Park 2021-09-07 158
17ccae8bb5c928 SeongJae Park 2021-09-07 159 i = 0;
17ccae8bb5c928 SeongJae Park 2021-09-07 160 damon_for_each_region(r3, t) {
17ccae8bb5c928 SeongJae Park 2021-09-07 161 KUNIT_EXPECT_PTR_EQ(test, r, r3);
17ccae8bb5c928 SeongJae Park 2021-09-07 162 i++;
17ccae8bb5c928 SeongJae Park 2021-09-07 163 }
17ccae8bb5c928 SeongJae Park 2021-09-07 164 KUNIT_EXPECT_EQ(test, i, 1);
17ccae8bb5c928 SeongJae Park 2021-09-07 165
17ccae8bb5c928 SeongJae Park 2021-09-07 166 damon_free_target(t);
17ccae8bb5c928 SeongJae Park 2021-09-07 167 }
17ccae8bb5c928 SeongJae Park 2021-09-07 168
17ccae8bb5c928 SeongJae Park 2021-09-07 169 static struct damon_region *__nth_region_of(struct damon_target *t, int idx)
17ccae8bb5c928 SeongJae Park 2021-09-07 170 {
17ccae8bb5c928 SeongJae Park 2021-09-07 171 struct damon_region *r;
17ccae8bb5c928 SeongJae Park 2021-09-07 172 unsigned int i = 0;
17ccae8bb5c928 SeongJae Park 2021-09-07 173
17ccae8bb5c928 SeongJae Park 2021-09-07 174 damon_for_each_region(r, t) {
17ccae8bb5c928 SeongJae Park 2021-09-07 175 if (i++ == idx)
17ccae8bb5c928 SeongJae Park 2021-09-07 176 return r;
17ccae8bb5c928 SeongJae Park 2021-09-07 177 }
17ccae8bb5c928 SeongJae Park 2021-09-07 178
17ccae8bb5c928 SeongJae Park 2021-09-07 179 return NULL;
17ccae8bb5c928 SeongJae Park 2021-09-07 180 }
17ccae8bb5c928 SeongJae Park 2021-09-07 181
17ccae8bb5c928 SeongJae Park 2021-09-07 182 static void damon_test_merge_regions_of(struct kunit *test)
17ccae8bb5c928 SeongJae Park 2021-09-07 183 {
17ccae8bb5c928 SeongJae Park 2021-09-07 184 struct damon_target *t;
17ccae8bb5c928 SeongJae Park 2021-09-07 185 struct damon_region *r;
17ccae8bb5c928 SeongJae Park 2021-09-07 186 unsigned long sa[] = {0, 100, 114, 122, 130, 156, 170, 184};
17ccae8bb5c928 SeongJae Park 2021-09-07 187 unsigned long ea[] = {100, 112, 122, 130, 156, 170, 184, 230};
17ccae8bb5c928 SeongJae Park 2021-09-07 188 unsigned int nrs[] = {0, 0, 10, 10, 20, 30, 1, 2};
17ccae8bb5c928 SeongJae Park 2021-09-07 189
17ccae8bb5c928 SeongJae Park 2021-09-07 190 unsigned long saddrs[] = {0, 114, 130, 156, 170};
17ccae8bb5c928 SeongJae Park 2021-09-07 191 unsigned long eaddrs[] = {112, 130, 156, 170, 230};
17ccae8bb5c928 SeongJae Park 2021-09-07 192 int i;
17ccae8bb5c928 SeongJae Park 2021-09-07 193
17ccae8bb5c928 SeongJae Park 2021-09-07 194 t = damon_new_target(42);
17ccae8bb5c928 SeongJae Park 2021-09-07 195 for (i = 0; i < ARRAY_SIZE(sa); i++) {
17ccae8bb5c928 SeongJae Park 2021-09-07 196 r = damon_new_region(sa[i], ea[i]);
17ccae8bb5c928 SeongJae Park 2021-09-07 197 r->nr_accesses = nrs[i];
17ccae8bb5c928 SeongJae Park 2021-09-07 198 damon_add_region(r, t);
17ccae8bb5c928 SeongJae Park 2021-09-07 199 }
17ccae8bb5c928 SeongJae Park 2021-09-07 200
17ccae8bb5c928 SeongJae Park 2021-09-07 @201 damon_merge_regions_of(t, 9, 9999);
17ccae8bb5c928 SeongJae Park 2021-09-07 202 /* 0-112, 114-130, 130-156, 156-170 */
17ccae8bb5c928 SeongJae Park 2021-09-07 203 KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 5u);
17ccae8bb5c928 SeongJae Park 2021-09-07 204 for (i = 0; i < 5; i++) {
17ccae8bb5c928 SeongJae Park 2021-09-07 205 r = __nth_region_of(t, i);
17ccae8bb5c928 SeongJae Park 2021-09-07 206 KUNIT_EXPECT_EQ(test, r->ar.start, saddrs[i]);
17ccae8bb5c928 SeongJae Park 2021-09-07 207 KUNIT_EXPECT_EQ(test, r->ar.end, eaddrs[i]);
17ccae8bb5c928 SeongJae Park 2021-09-07 208 }
17ccae8bb5c928 SeongJae Park 2021-09-07 209 damon_free_target(t);
17ccae8bb5c928 SeongJae Park 2021-09-07 210 }
17ccae8bb5c928 SeongJae Park 2021-09-07 211
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/damon: Add option to monitor only writes
2022-02-03 13:39 ` SeongJae Park
@ 2022-02-04 15:11 ` Pedro Gomes
2022-02-07 8:16 ` SeongJae Park
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Gomes @ 2022-02-04 15:11 UTC (permalink / raw)
To: SeongJae Park
Cc: Steven Rostedt, Ingo Molnar, Andrew Morton, linux-mm, linux-kernel
On Thu, Feb 3, 2022 at 10:39 AM SeongJae Park <sj@kernel.org> wrote:
> I think this would be better to be implemented as another monitoring primitive
> based on the virtual address space monitoring primitives, e.g., vaddr-writes?
> Then the implementation would be simpler and changes to other files will be
> minimized. For the user space interface, we could use a prefix to target_ids
> debugfs file. For example,
>
> # echo "vaddr-writes $(pidof workload)" > $debugfs/damon/target_ids
I will do that.
> > This patch also adds the actions mergeable and unmergeable to damos schemes.
> > These actions are used by KSM as explained in [1].
>
> Please do that in a separate patch, and also update the document
> (Documentation/admin-guide/mm/damon/usage.rst). And, what's the expected usage
> of the action and benefits?
The idea is to use this action to all areas that are not written too frequently,
this way KSM can save more memory without too much overhead.
But I have to test it better and collect some data to see if it really
makes sense,
perhaps it is better to leave this patch for later.
I would like to know your opinion on this, do you think it makes sense?
--
Atenciosamente,
Pedro Demarchi Gomes.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/damon: Add option to monitor only writes
2022-02-04 15:11 ` Pedro Gomes
@ 2022-02-07 8:16 ` SeongJae Park
0 siblings, 0 replies; 11+ messages in thread
From: SeongJae Park @ 2022-02-07 8:16 UTC (permalink / raw)
To: Pedro Gomes
Cc: SeongJae Park, Steven Rostedt, Ingo Molnar, Andrew Morton,
linux-mm, linux-kernel
On Fri, 4 Feb 2022 12:11:44 -0300 Pedro Gomes <pedrodemargomes@gmail.com> wrote:
> On Thu, Feb 3, 2022 at 10:39 AM SeongJae Park <sj@kernel.org> wrote:
>
> > I think this would be better to be implemented as another monitoring primitive
> > based on the virtual address space monitoring primitives, e.g., vaddr-writes?
> > Then the implementation would be simpler and changes to other files will be
> > minimized. For the user space interface, we could use a prefix to target_ids
> > debugfs file. For example,
> >
> > # echo "vaddr-writes $(pidof workload)" > $debugfs/damon/target_ids
>
> I will do that.
Thanks!
>
> > > This patch also adds the actions mergeable and unmergeable to damos schemes.
> > > These actions are used by KSM as explained in [1].
> >
> > Please do that in a separate patch, and also update the document
> > (Documentation/admin-guide/mm/damon/usage.rst). And, what's the expected usage
> > of the action and benefits?
>
> The idea is to use this action to all areas that are not written too frequently,
> this way KSM can save more memory without too much overhead.
> But I have to test it better and collect some data to see if it really
> makes sense,
> perhaps it is better to leave this patch for later.
> I would like to know your opinion on this, do you think it makes sense?
Yes, that idea makes sense to me :)
Thanks,
SJ
>
>
> --
> Atenciosamente,
> Pedro Demarchi Gomes.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/damon: Add option to monitor only writes
2022-02-03 13:12 [PATCH] mm/damon: Add option to monitor only writes Pedro Demarchi Gomes
` (3 preceding siblings ...)
2022-02-04 1:30 ` kernel test robot
@ 2022-02-07 10:32 ` David Hildenbrand
2022-02-08 2:05 ` Pedro Gomes
4 siblings, 1 reply; 11+ messages in thread
From: David Hildenbrand @ 2022-02-07 10:32 UTC (permalink / raw)
To: Pedro Demarchi Gomes
Cc: SeongJae Park, Steven Rostedt, Ingo Molnar, Andrew Morton,
linux-mm, linux-kernel, John Hubbard
On 03.02.22 14:12, Pedro Demarchi Gomes wrote:
> When "writes" is written to /sys/kernel/debug/damon/counter_type damon will monitor only writes.
> This patch also adds the actions mergeable and unmergeable to damos schemes. These actions are used by KSM as explained in [1].
[...]
>
> +static inline bool pte_is_pinned(struct vm_area_struct *vma, unsigned long addr, pte_t pte)
> +{
> + struct page *page;
> +
> + if (!pte_write(pte))
> + return false;
> + if (!is_cow_mapping(vma->vm_flags))
> + return false;
> + if (likely(!test_bit(MMF_HAS_PINNED, &vma->vm_mm->flags)))
> + return false;
> + page = vm_normal_page(vma, addr, pte);
> + if (!page)
> + return false;
> + return page_maybe_dma_pinned(page);
> +}
> +
> +static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma,
> + unsigned long addr, pmd_t *pmdp)
> +{
> + pmd_t old, pmd = *pmdp;
> +
> + if (pmd_present(pmd)) {
> + /* See comment in change_huge_pmd() */
> + old = pmdp_invalidate(vma, addr, pmdp);
> + if (pmd_dirty(old))
> + pmd = pmd_mkdirty(pmd);
> + if (pmd_young(old))
> + pmd = pmd_mkyoung(pmd);
> +
> + pmd = pmd_wrprotect(pmd);
> + pmd = pmd_clear_soft_dirty(pmd);
> +
> + set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
> + } else if (is_migration_entry(pmd_to_swp_entry(pmd))) {
> + pmd = pmd_swp_clear_soft_dirty(pmd);
> + set_pmd_at(vma->vm_mm, addr, pmdp, pmd);
> + }
> +}
> +
> +static inline void clear_soft_dirty(struct vm_area_struct *vma,
> + unsigned long addr, pte_t *pte)
> +{
> + /*
> + * The soft-dirty tracker uses #PF-s to catch writes
> + * to pages, so write-protect the pte as well. See the
> + * Documentation/admin-guide/mm/soft-dirty.rst for full description
> + * of how soft-dirty works.
> + */
> + pte_t ptent = *pte;
> +
> + if (pte_present(ptent)) {
> + pte_t old_pte;
> +
> + if (pte_is_pinned(vma, addr, ptent))
> + return;
> + old_pte = ptep_modify_prot_start(vma, addr, pte);
> + ptent = pte_wrprotect(old_pte);
> + ptent = pte_clear_soft_dirty(ptent);
> + ptep_modify_prot_commit(vma, addr, pte, old_pte, ptent);
> + } else if (is_swap_pte(ptent)) {
> + ptent = pte_swp_clear_soft_dirty(ptent);
> + set_pte_at(vma->vm_mm, addr, pte, ptent);
> + }
> +}
Just like clearrefs, this can race against GUP-fast to detect pinned
pages. And just like clearrefs, we're not handling PMDs properly. And
just like anything that write-protects random anon pages right now, this
does not consider O_DIRECT as is.
Fortunately, there are not too many users of clearreefs/softdirty
tracking out there (my search a while ago returned no open source
users). My assumption is that your feature might see more widespread use.
Adding more random write protection until we fixed the COW issues [1]
really makes my stomach hurt on a Monday morning.
Please, let's defer any more features that rely on write-protecting
random anon pages until we have ways in place to not corrupt random user
space.
That is:
1) Teaching the COW logic to not copy pages that are pinned -- I'm
working on that.
2) Converting O_DIRECT to use FOLL_PIN instead of FOLL_GET. John is
working on that.
So I'm not against this change. I'm against this change at this point in
time.
[1]
https://lore.kernel.org/all/3ae33b08-d9ef-f846-56fb-645e3b9b4c66@redhat.com/
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/damon: Add option to monitor only writes
2022-02-07 10:32 ` David Hildenbrand
@ 2022-02-08 2:05 ` Pedro Gomes
2022-02-08 9:01 ` David Hildenbrand
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Gomes @ 2022-02-08 2:05 UTC (permalink / raw)
To: David Hildenbrand
Cc: SeongJae Park, Steven Rostedt, Ingo Molnar, Andrew Morton,
linux-mm, linux-kernel, John Hubbard
On Mon, Feb 7, 2022 at 7:32 AM David Hildenbrand <david@redhat.com> wrote:
>
> Just like clearrefs, this can race against GUP-fast to detect pinned
> pages. And just like clearrefs, we're not handling PMDs properly. And
> just like anything that write-protects random anon pages right now, this
> does not consider O_DIRECT as is.
>
> Fortunately, there are not too many users of clearreefs/softdirty
> tracking out there (my search a while ago returned no open source
> users). My assumption is that your feature might see more widespread use.
>
> Adding more random write protection until we fixed the COW issues [1]
> really makes my stomach hurt on a Monday morning.
I was not aware of these issues.
> Please, let's defer any more features that rely on write-protecting
> random anon pages until we have ways in place to not corrupt random user
> space.
>
> That is:
> 1) Teaching the COW logic to not copy pages that are pinned -- I'm
> working on that.
> 2) Converting O_DIRECT to use FOLL_PIN instead of FOLL_GET. John is
> working on that.
>
> So I'm not against this change. I'm against this change at this point in
> time.
I agree. I will wait until the COW problems are solved to send this patch.
--
Atenciosamente,
Pedro Demarchi Gomes.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/damon: Add option to monitor only writes
2022-02-08 2:05 ` Pedro Gomes
@ 2022-02-08 9:01 ` David Hildenbrand
2022-02-08 13:18 ` Pedro Gomes
0 siblings, 1 reply; 11+ messages in thread
From: David Hildenbrand @ 2022-02-08 9:01 UTC (permalink / raw)
To: Pedro Gomes
Cc: SeongJae Park, Steven Rostedt, Ingo Molnar, Andrew Morton,
linux-mm, linux-kernel, John Hubbard
On 08.02.22 03:05, Pedro Gomes wrote:
> On Mon, Feb 7, 2022 at 7:32 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> Just like clearrefs, this can race against GUP-fast to detect pinned
>> pages. And just like clearrefs, we're not handling PMDs properly. And
>> just like anything that write-protects random anon pages right now, this
>> does not consider O_DIRECT as is.
>>
>> Fortunately, there are not too many users of clearreefs/softdirty
>> tracking out there (my search a while ago returned no open source
>> users). My assumption is that your feature might see more widespread use.
>>
>> Adding more random write protection until we fixed the COW issues [1]
>> really makes my stomach hurt on a Monday morning.
>
> I was not aware of these issues.
>
>> Please, let's defer any more features that rely on write-protecting
>> random anon pages until we have ways in place to not corrupt random user
>> space.
>>
>> That is:
>> 1) Teaching the COW logic to not copy pages that are pinned -- I'm
>> working on that.
>> 2) Converting O_DIRECT to use FOLL_PIN instead of FOLL_GET. John is
>> working on that.
>>
>> So I'm not against this change. I'm against this change at this point in
>> time.
>
> I agree. I will wait until the COW problems are solved to send this patch.
>
>
I'll put you on CC once I have something ready to at least handle pinned
pages (FOLL_PIN) as expected, to not get them accidentally COWed.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mm/damon: Add option to monitor only writes
2022-02-08 9:01 ` David Hildenbrand
@ 2022-02-08 13:18 ` Pedro Gomes
0 siblings, 0 replies; 11+ messages in thread
From: Pedro Gomes @ 2022-02-08 13:18 UTC (permalink / raw)
To: David Hildenbrand
Cc: SeongJae Park, Steven Rostedt, Ingo Molnar, Andrew Morton,
linux-mm, linux-kernel, John Hubbard
On Tue, Feb 8, 2022 at 6:01 AM David Hildenbrand <david@redhat.com> wrote:
>
> I'll put you on CC once I have something ready to at least handle pinned
> pages (FOLL_PIN) as expected, to not get them accidentally COWed.
>
OK, thanks!
--
Atenciosamente,
Pedro Demarchi Gomes.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-02-08 13:25 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-03 13:12 [PATCH] mm/damon: Add option to monitor only writes Pedro Demarchi Gomes
2022-02-03 13:39 ` SeongJae Park
2022-02-04 15:11 ` Pedro Gomes
2022-02-07 8:16 ` SeongJae Park
2022-02-04 1:29 ` kernel test robot
2022-02-04 1:29 ` kernel test robot
2022-02-04 1:30 ` kernel test robot
2022-02-07 10:32 ` David Hildenbrand
2022-02-08 2:05 ` Pedro Gomes
2022-02-08 9:01 ` David Hildenbrand
2022-02-08 13:18 ` Pedro Gomes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).