Hi Hailong, On Thu, 21 Apr 2022 09:06:41 +0800 Hailong Tu <tuhailong@xxxxxxxxx> wrote: > The timer stays active even if the reclaim mechanism is never enabled. I'd like to make it clear that this change is for DAMON_RECLAIM by making the first part of the subject 'mm/damon/reclaim:', and adjusting above sentence. > It is unnecessary overhead can be completely avoided by using module_param_call() for enabled flag. Let's wrap the line at 75 columns (https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format). > > Signed-off-by: Hailong Tu <tuhailong@xxxxxxxxx> > --- > mm/damon/reclaim.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c > index e34c4d0c4d93..46505c501cd6 100644 > --- a/mm/damon/reclaim.c > +++ b/mm/damon/reclaim.c > @@ -28,7 +28,6 @@ > * this. > */ > static bool enabled __read_mostly; > -module_param(enabled, bool, 0600); > > /* > * Time threshold for cold memory regions identification in microseconds. > @@ -358,11 +357,27 @@ static void damon_reclaim_timer_fn(struct work_struct *work) > enabled = last_enabled; > } > > - schedule_delayed_work(&damon_reclaim_timer, > + if (enabled) > + schedule_delayed_work(&damon_reclaim_timer, > msecs_to_jiffies(ENABLE_CHECK_INTERVAL_MS)); > } > static DECLARE_DELAYED_WORK(damon_reclaim_timer, damon_reclaim_timer_fn); > > +static int enabled_store(const char *val, > + const struct kernel_param *kp) > +{ > + int rc = param_set_bool(val, kp); > + > + if (rc < 0) > + return rc; > + > + if (enabled) > + schedule_delayed_work(&damon_reclaim_timer, 0); > + > + return 0; > +} > +module_param_call(enabled, enabled_store, param_get_bool, &enabled, 0600); module_param_call() is commented as obsolete. Could we use module_param_cb() instead as suggested by the comment? > + > static int damon_reclaim_after_aggregation(struct damon_ctx *c) > { > struct damos *s; > -- > 2.25.1 > Thanks, SJ
