[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220120174630.26102-1-sj@kernel.org>
Date: Thu, 20 Jan 2022 17:46:30 +0000
From: SeongJae Park <sj@...nel.org>
To: Xin Hao <xhao@...ux.alibaba.com>
Cc: sj@...nel.org, akpm@...ux-foundation.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH V1 2/2] mm/damon/dbgfs: Modify Damon dbfs interface dependency in Kconfig
Hi Xin,
I guess this should be at least v3, as you posted v1[1] and v2[2] before.
Could you please also summarize what changes are made from v2?
On Fri, 21 Jan 2022 00:04:28 +0800 Xin Hao <xhao@...ux.alibaba.com> wrote:
> If you want to support "DAMON_DBGFS" in config file, it only depends on
> any one of "DAMON_VADDR" and "DAMON_PADDR", and sometimes we just want to
> use damon virtual address function, but it is unreasonable to include
> "DAMON_PADDR" in config file which cause the damon/paddr.c be compiled.
My comment[3] to the v2 was,
So, unless you make me believe the benefit is big enough and all possible
corner cases are well handled and sufficiently tested, I'm sorry but I would
not be convinced with this change.
I don't find answers to the requests here. Could you please add those if you
have?
[1] https://lore.kernel.org/linux-mm/fbf27c39e8f23d12d5474b9b659d3d45f8dd38ca.1637429074.git.xhao@lin
ux.alibaba.com/
[2] https://lore.kernel.org/linux-mm/b6bd0063-efb2-62ce-53ee-0193f63262d8@linux.alibaba.com/
[3] https://lore.kernel.org/linux-mm/20211123103855.12592-1-sj@kernel.org/
Thanks,
SJ
>
> So there just do little change to fix it.
>
> Signed-off-by: Xin Hao <xhao@...ux.alibaba.com>
> ---
> include/linux/damon.h | 16 ++++++++++++++++
> mm/damon/Kconfig | 2 +-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 794b5f453662..c83e4588aa45 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -501,12 +501,28 @@ int damon_stop(struct damon_ctx **ctxs, int nr_ctxs);
> #ifdef CONFIG_DAMON_VADDR
> bool damon_va_target_valid(void *t);
> void damon_va_set_primitives(struct damon_ctx *ctx);
> +#else
> +static inline void damon_va_set_primitives(struct damon_ctx *ctx) { }
> +static inline bool damon_va_target_valid(void *t)
> +{
> + return false;
> +}
> #endif /* CONFIG_DAMON_VADDR */
>
> #ifdef CONFIG_DAMON_PADDR
> bool damon_pa_check(char *buf, size_t count);
> bool damon_pa_target_valid(void *t);
> void damon_pa_set_primitives(struct damon_ctx *ctx);
> +#else
> +static inline bool damon_pa_check(char *buf, size_t count)
> +{
> + return false;
> +}
> +static inline void damon_pa_set_primitives(struct damon_ctx *ctx) { }
> +static inline bool damon_pa_target_valid(void *t)
> +{
> + return false;
> +}
> #endif /* CONFIG_DAMON_PADDR */
>
> #endif /* _DAMON_H */
> diff --git a/mm/damon/Kconfig b/mm/damon/Kconfig
> index 5bcf05851ad0..a8952af3c8bf 100644
> --- a/mm/damon/Kconfig
> +++ b/mm/damon/Kconfig
> @@ -54,7 +54,7 @@ config DAMON_VADDR_KUNIT_TEST
>
> config DAMON_DBGFS
> bool "DAMON debugfs interface"
> - depends on DAMON_VADDR && DAMON_PADDR && DEBUG_FS
> + depends on (DAMON_VADDR || DAMON_PADDR) && DEBUG_FS
> help
> This builds the debugfs interface for DAMON. The user space admins
> can use the interface for arbitrary data access monitoring.
> --
> 2.27.0
>
Powered by blists - more mailing lists