All of lore.kernel.org
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: SeongJae Park <sj@kernel.org>
Cc: "# 6 . 17 . x" <stable@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	damon@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: [RFC PATCH] mm/damon/stat: detect and use fresh enabled status
Date: Fri, 17 Apr 2026 18:48:09 -0700	[thread overview]
Message-ID: <20260418014809.6428-1-sj@kernel.org> (raw)
In-Reply-To: <20260416143857.76146-1-sj@kernel.org>

On Thu, 16 Apr 2026 07:38:55 -0700 SeongJae Park <sj@kernel.org> wrote:

> DAMON_STAT assumes the kdamond will keep running once damon_stat_start()
> succeeds, until it calls damon_stop() to stop it.  If the
> regions_score_histogram allocation in kdamond_fn() is tried after
> damon_stat_start() returns, however, and if the allocation fails, the
> kdamond can stop before DAMON_STAT calls damon_stop().  In this case,
> users will show the 'enabled' parameter value as 'true', while it is not
> working.  This could make users confused.
> 
> The user impact should be mild, though.  First of all, the issue may
> happen only quite rarely.  The allocation failure is arguably too small
> to fail (100 unsigned long objects) in common setups.  The time window
> for the race is also quite small.  Even if the race and the allocation
> failure happen, users could find the fact that the kdamond is stopped
> using 'ps' like commands.  By writing 'N' and 'Y' to the 'enabled'
> parameter sequentially, the user can also easily restart DAMON_STAT.
> 
> That said, the bug is a bug that needs to be fixed.  Instead of managing
> the complicated state in the variable, detect and use the real kdamond
> running status when the user reads the parameter, via the parameter read
> callback.  This will allow users to always read the correct 'enabled'
> value.
> 
> Note that the 'enabled' variable is no longer the argument for the
> 'enabled' parameter.  But it is still used for two use case.  For
> keeping the config/boot time user-set parameter value.  And for keeping
> the user request to compare against the current state, to see if the
> damon_start() or damon_stop() call are really needed.

Posted the next version of this patch as a part of another series [1], because
the patches of the series are fixing the similar type of bugs.

[1] https://lore.kernel.org/20260418014439.6353-1-sj@kernel.org


Thanks,
SJ

[...]

      parent reply	other threads:[~2026-04-18  1:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-16 14:38 [RFC PATCH] mm/damon/stat: detect and use fresh enabled status SeongJae Park
2026-04-16 18:51 ` sashiko-bot
2026-04-17  0:03   ` SeongJae Park
2026-04-18  1:48 ` SeongJae Park [this message]

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=20260418014809.6428-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=stable@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.