lkml.org 
[lkml]   [2021]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [RFC PATCH] mm/damon: remove damon_lock
Date
Thank you for this patch, Alex!

On Wed, 10 Nov 2021 19:47:21 +0800 alexs@kernel.org wrote:

> From: Alex Shi <alexs@kernel.org>
>
> Variable nr_running_ctxs guards by damon_lock, but a lock for a int
> variable seems a bit heavy, a atomic_t is enough.

The lock is not only for protecting nr_running_ctxs, but also for avoiding
different users concurrently executing damon_start(), because that could allow
the users interfering others.

>
> Signed-off-by: Alex Shi <alexs@kernel.org>
> Cc: SeongJae Park <sj@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> ---
> include/linux/damon.h | 1 -
> mm/damon/core.c | 31 +++++--------------------------
> mm/damon/dbgfs.c | 8 +++++---
> 3 files changed, 10 insertions(+), 30 deletions(-)
>
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index b4d4be3cc987..e5dcc6336ef2 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -453,7 +453,6 @@ int damon_set_attrs(struct damon_ctx *ctx, unsigned long sample_int,
> unsigned long min_nr_reg, unsigned long max_nr_reg);
> int damon_set_schemes(struct damon_ctx *ctx,
> struct damos **schemes, ssize_t nr_schemes);
> -int damon_nr_running_ctxs(void);
>
> int damon_start(struct damon_ctx **ctxs, int nr_ctxs);
> int damon_stop(struct damon_ctx **ctxs, int nr_ctxs);
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index c381b3c525d0..e821e36d5c10 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[...]
> @@ -437,19 +422,15 @@ int damon_start(struct damon_ctx **ctxs, int nr_ctxs)
> int i;
> int err = 0;
>
> - mutex_lock(&damon_lock);
> - if (nr_running_ctxs) {
> - mutex_unlock(&damon_lock);
> + if (atomic_read(&nr_running_ctxs))
> return -EBUSY;
> - }
>
> for (i = 0; i < nr_ctxs; i++) {
> err = __damon_start(ctxs[i]);
> if (err)
> break;
> - nr_running_ctxs++;
> + atomic_inc(&nr_running_ctxs);
> }
> - mutex_unlock(&damon_lock);
>
> return err;
> }

This would let multiple concurrent threads seeing nr_running_ctxs of zero and
therefore proceed together.


Thanks,
SJ

\
 
 \ /
  Last update: 2021-11-10 13:41    [W:0.598 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site