Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: SeongJae Park <sjpark@amazon.com>
Cc: <akpm@linux-foundation.org>, SeongJae Park <sjpark@amazon.de>,
	<aarcange@redhat.com>, <yang.shi@linux.alibaba.com>,
	<acme@kernel.org>, <alexander.shishkin@linux.intel.com>,
	<amit@kernel.org>, <brendan.d.gregg@gmail.com>,
	<brendanhiggins@google.com>, <cai@lca.pw>,
	<colin.king@canonical.com>, <corbet@lwn.net>, <dwmw@amazon.com>,
	<jolsa@redhat.com>, <kirill@shutemov.name>,
	<mark.rutland@arm.com>, <mgorman@suse.de>, <minchan@kernel.org>,
	<mingo@redhat.com>, <namhyung@kernel.org>, <peterz@infradead.org>,
	<rdunlap@infradead.org>, <rientjes@google.com>,
	<rostedt@goodmis.org>, <shuah@kernel.org>, <sj38.park@gmail.com>,
	<vbabka@suse.cz>, <vdavydov.dev@gmail.com>, <linux-mm@kvack.org>,
	<linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v6 07/14] mm/damon: Implement kernel space API
Date: Tue, 10 Mar 2020 09:01:52 +0000
Message-ID: <20200310090152.00002c6e@Huawei.com> (raw)
In-Reply-To: <20200224123047.32506-8-sjpark@amazon.com>

On Mon, 24 Feb 2020 13:30:40 +0100
SeongJae Park <sjpark@amazon.com> wrote:

> From: SeongJae Park <sjpark@amazon.de>
> 
> This commit implements the DAMON api for the kernel.  Other kernel code
> can use DAMON by calling damon_start() and damon_stop() with their own
> 'struct damon_ctx'.
> 
> Signed-off-by: SeongJae Park <sjpark@amazon.de>

Seems like it would have been easier to create the header as you went along
and avoid the need to have the bits here dropping static.

Or the moves for that matter.

Also, ideally have full kernel-doc for anything that forms part of an
interface that is intended for use by others.

Jonathan

> ---
>  include/linux/damon.h | 71 +++++++++++++++++++++++++++++++++++++++++++
>  mm/damon.c            | 71 +++++++++----------------------------------
>  2 files changed, 85 insertions(+), 57 deletions(-)
>  create mode 100644 include/linux/damon.h
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> new file mode 100644
> index 000000000000..78785cb88d42
> --- /dev/null
> +++ b/include/linux/damon.h
> @@ -0,0 +1,71 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * DAMON api
> + *
> + * Copyright 2019 Amazon.com, Inc. or its affiliates.  All rights reserved.
> + *
> + * Author: SeongJae Park <sjpark@amazon.de>
> + */
> +
> +#ifndef _DAMON_H_
> +#define _DAMON_H_
> +
> +#include <linux/random.h>
> +#include <linux/spinlock_types.h>
> +#include <linux/time64.h>
> +#include <linux/types.h>
> +
> +/* Represents a monitoring target region on the virtual address space */
> +struct damon_region {
> +	unsigned long vm_start;
> +	unsigned long vm_end;
> +	unsigned long sampling_addr;
> +	unsigned int nr_accesses;
> +	struct list_head list;
> +};
> +
> +/* Represents a monitoring target task */
> +struct damon_task {
> +	unsigned long pid;
> +	struct list_head regions_list;
> +	struct list_head list;
> +};
> +
> +struct damon_ctx {
> +	unsigned long sample_interval;
> +	unsigned long aggr_interval;
> +	unsigned long regions_update_interval;
> +	unsigned long min_nr_regions;
> +	unsigned long max_nr_regions;
> +
> +	struct timespec64 last_aggregation;
> +	struct timespec64 last_regions_update;
> +
> +	unsigned char *rbuf;
> +	unsigned int rbuf_len;
> +	unsigned int rbuf_offset;
> +	char *rfile_path;
> +
> +	struct task_struct *kdamond;
> +	bool kdamond_stop;
> +	spinlock_t kdamond_lock;
> +
> +	struct rnd_state rndseed;
> +
> +	struct list_head tasks_list;	/* 'damon_task' objects */
> +
> +	/* callbacks */
> +	void (*sample_cb)(struct damon_ctx *context);
> +	void (*aggregate_cb)(struct damon_ctx *context);
> +};
> +
> +int damon_set_pids(struct damon_ctx *ctx,
> +			unsigned long *pids, ssize_t nr_pids);
> +int damon_set_recording(struct damon_ctx *ctx,
> +			unsigned int rbuf_len, char *rfile_path);
> +int damon_set_attrs(struct damon_ctx *ctx, unsigned long s, unsigned long a,
> +			unsigned long r, unsigned long min, unsigned long max);
> +int damon_start(struct damon_ctx *ctx);
> +int damon_stop(struct damon_ctx *ctx);
> +
> +#endif
> diff --git a/mm/damon.c b/mm/damon.c
> index a7edb2dfa700..b3e9b9da5720 100644
> --- a/mm/damon.c
> +++ b/mm/damon.c
> @@ -9,6 +9,7 @@
>  
>  #define pr_fmt(fmt) "damon: " fmt
>  
> +#include <linux/damon.h>
>  #include <linux/delay.h>
>  #include <linux/kthread.h>
>  #include <linux/mm.h>
> @@ -40,60 +41,6 @@
>  #define damon_for_each_task_safe(ctx, t, next) \
>  	list_for_each_entry_safe(t, next, &(ctx)->tasks_list, list)
>  
> -/* Represents a monitoring target region on the virtual address space */
> -struct damon_region {
> -	unsigned long vm_start;
> -	unsigned long vm_end;
> -	unsigned long sampling_addr;
> -	unsigned int nr_accesses;
> -	struct list_head list;
> -};
> -
> -/* Represents a monitoring target task */
> -struct damon_task {
> -	unsigned long pid;
> -	struct list_head regions_list;
> -	struct list_head list;
> -};
> -
> -/*
> - * For each 'sample_interval', DAMON checks whether each region is accessed or
> - * not.  It aggregates and keeps the access information (number of accesses to
> - * each region) for each 'aggr_interval' time.  And for each
> - * 'regions_update_interval', damon checks whether the memory mapping of the
> - * target tasks has changed (e.g., by mmap() calls from the applications) and
> - * applies the changes.
> - *
> - * All time intervals are in micro-seconds.
> - */
> -struct damon_ctx {
> -	unsigned long sample_interval;
> -	unsigned long aggr_interval;
> -	unsigned long regions_update_interval;
> -	unsigned long min_nr_regions;
> -	unsigned long max_nr_regions;
> -
> -	struct timespec64 last_aggregation;
> -	struct timespec64 last_regions_update;
> -
> -	unsigned char *rbuf;
> -	unsigned int rbuf_len;
> -	unsigned int rbuf_offset;
> -	char *rfile_path;
> -
> -	struct task_struct *kdamond;
> -	bool kdamond_stop;
> -	spinlock_t kdamond_lock;
> -
> -	struct rnd_state rndseed;
> -
> -	struct list_head tasks_list;	/* 'damon_task' objects */
> -
> -	/* callbacks */
> -	void (*sample_cb)(struct damon_ctx *context);
> -	void (*aggregate_cb)(struct damon_ctx *context);
> -};
> -
>  #define MAX_RFILE_PATH_LEN	256
>  
>  /* Get a random number in [l, r) */
> @@ -961,10 +908,20 @@ static int damon_turn_kdamond(struct damon_ctx *ctx, bool on)
>  	return 0;
>  }
>  
> +int damon_start(struct damon_ctx *ctx)
> +{
> +	return damon_turn_kdamond(ctx, true);
> +}
> +
> +int damon_stop(struct damon_ctx *ctx)
> +{
> +	return damon_turn_kdamond(ctx, false);
> +}
> +
>  /*
>   * This function should not be called while the kdamond is running.
>   */
> -static int damon_set_pids(struct damon_ctx *ctx,
> +int damon_set_pids(struct damon_ctx *ctx,
>  			unsigned long *pids, ssize_t nr_pids)
>  {
>  	ssize_t i;
> @@ -998,7 +955,7 @@ static int damon_set_pids(struct damon_ctx *ctx,
>   *
>   * Returns 0 on success, negative error code otherwise.
>   */
> -static int damon_set_recording(struct damon_ctx *ctx,
> +int damon_set_recording(struct damon_ctx *ctx,
>  				unsigned int rbuf_len, char *rfile_path)
>  {
>  	size_t rfile_path_len;
> @@ -1046,7 +1003,7 @@ static int damon_set_recording(struct damon_ctx *ctx,
>   *
>   * Returns 0 on success, negative error code otherwise.
>   */
> -static int damon_set_attrs(struct damon_ctx *ctx, unsigned long sample_int,
> +int damon_set_attrs(struct damon_ctx *ctx, unsigned long sample_int,
>  		unsigned long aggr_int, unsigned long regions_update_int,
>  		unsigned long min_nr_reg, unsigned long max_nr_reg)
>  {




  reply index

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-24 12:30 [PATCH v6 00/14] Introduce Data Access MONitor (DAMON) SeongJae Park
2020-02-24 12:30 ` [PATCH v6 01/14] mm: " SeongJae Park
2020-03-10  8:54   ` Jonathan Cameron
2020-03-10 11:50     ` SeongJae Park
2020-02-24 12:30 ` [PATCH v6 02/14] mm/damon: Implement region based sampling SeongJae Park
2020-03-10  8:57   ` Jonathan Cameron
2020-03-10 11:52     ` SeongJae Park
2020-03-10 15:55       ` Jonathan Cameron
2020-03-10 16:22         ` SeongJae Park
2020-03-10 17:39           ` Jonathan Cameron
2020-03-12  9:20             ` SeongJae Park
2020-03-13 17:29   ` Jonathan Cameron
2020-03-13 20:16     ` SeongJae Park
2020-03-17 11:32       ` SeongJae Park
2020-02-24 12:30 ` [PATCH v6 03/14] mm/damon: Adaptively adjust regions SeongJae Park
2020-03-10  8:57   ` Jonathan Cameron
2020-03-10 11:53     ` SeongJae Park
2020-02-24 12:30 ` [PATCH v6 04/14] mm/damon: Apply dynamic memory mapping changes SeongJae Park
2020-03-10  9:00   ` Jonathan Cameron
2020-03-10 11:53     ` SeongJae Park
2020-02-24 12:30 ` [PATCH v6 05/14] mm/damon: Implement callbacks SeongJae Park
2020-03-10  9:01   ` Jonathan Cameron
2020-03-10 11:55     ` SeongJae Park
2020-02-24 12:30 ` [PATCH v6 06/14] mm/damon: Implement access pattern recording SeongJae Park
2020-03-10  9:01   ` Jonathan Cameron
2020-03-10 11:55     ` SeongJae Park
2020-02-24 12:30 ` [PATCH v6 07/14] mm/damon: Implement kernel space API SeongJae Park
2020-03-10  9:01   ` Jonathan Cameron [this message]
2020-03-10 11:56     ` SeongJae Park
2020-02-24 12:30 ` [PATCH v6 08/14] mm/damon: Add debugfs interface SeongJae Park
2020-03-10  9:02   ` Jonathan Cameron
2020-03-10 11:56     ` SeongJae Park
2020-02-24 12:30 ` [PATCH v6 09/14] mm/damon: Add a tracepoint for result writing SeongJae Park
2020-03-10  9:03   ` Jonathan Cameron
2020-03-10 11:57     ` SeongJae Park
2020-02-24 12:30 ` [PATCH v6 10/14] tools: Add a minimal user-space tool for DAMON SeongJae Park
2020-02-24 12:30 ` [PATCH v6 11/14] Documentation/admin-guide/mm: Add a document " SeongJae Park
2020-03-10  9:03   ` Jonathan Cameron
2020-03-10 11:57     ` SeongJae Park
2020-02-24 12:30 ` [PATCH v6 12/14] mm/damon: Add kunit tests SeongJae Park
2020-02-24 12:30 ` [PATCH v6 13/14] mm/damon: Add user selftests SeongJae Park
2020-02-24 12:30 ` [PATCH v6 14/14] MAINTAINERS: Update for DAMON SeongJae Park
2020-03-02 11:35 ` [PATCH v6 00/14] Introduce Data Access MONitor (DAMON) SeongJae Park
2020-03-09 10:23   ` SeongJae Park
2020-03-10 17:21 ` Shakeel Butt
2020-03-12 10:07   ` SeongJae Park
2020-03-12 10:43     ` SeongJae Park
2020-03-18 19:52       ` Shakeel Butt
2020-03-19  9:03         ` SeongJae Park
2020-03-23 17:29           ` Shakeel Butt
2020-03-24  8:34             ` SeongJae Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200310090152.00002c6e@Huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=aarcange@redhat.com \
    --cc=acme@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=amit@kernel.org \
    --cc=brendan.d.gregg@gmail.com \
    --cc=brendanhiggins@google.com \
    --cc=cai@lca.pw \
    --cc=colin.king@canonical.com \
    --cc=corbet@lwn.net \
    --cc=dwmw@amazon.com \
    --cc=jolsa@redhat.com \
    --cc=kirill@shutemov.name \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mark.rutland@arm.com \
    --cc=mgorman@suse.de \
    --cc=minchan@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=rientjes@google.com \
    --cc=rostedt@goodmis.org \
    --cc=shuah@kernel.org \
    --cc=sj38.park@gmail.com \
    --cc=sjpark@amazon.com \
    --cc=sjpark@amazon.de \
    --cc=vbabka@suse.cz \
    --cc=vdavydov.dev@gmail.com \
    --cc=yang.shi@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git