Re: [PATCH] mm/damon/core: avoid use of half-online-committed context

[SeongJae Park <sj@kernel.org>]

On Fri, 20 Mar 2026 19:16:27 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Thu, 19 Mar 2026 19:48:49 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Thu, 19 Mar 2026 07:52:17 -0700 SeongJae Park <sj@kernel.org> wrote:
> > 
> > > One major usage of damon_call() is online DAMON parameters update.  It
> > > is done by calling damon_commit_ctx() inside the damon_call() callback
> > > function.  damon_commit_ctx() can fail for two reasons: 1) invalid
> > > parameters and 2) internal memory allocation failures.  In case of
> > > failures, the damon_ctx that attempted to be updated (commit
> > > destination) can be partially updated (or, corrupted from a
> > > perspective), and therefore shouldn't be used anymore.  The function
> > > only ensures the damon_ctx object can safely deallocated using
> > > damon_destroy_ctx().
> > > 
> > > The API callers are, however, calling damon_commit_ctx() only after
> > > asserting the parameters are valid, to avoid damon_commit_ctx() fails
> > > due to invalid input parameters.  But it can still theoretically fail if
> > > the internal memory allocation fails.  In the case, DAMON may run with
> > > the partially updated damon_ctx.  This can result in unexpected
> > > behaviors including even NULL pointer dereference in case of
> > > damos_commit_dests() failure [1].  Such allocation failure is arguably
> > > too small to fail, so the real world impact would be rare.  But, given
> > > the bad consequence, this needs to be fixed.
> > > 
> > > Avoid such partially-committed (maybe-corrupted) damon_ctx use by saving
> > > the damon_commit_ctx() failure on the damon_ctx object.  For this,
> > > introduce damon_ctx->maybe_corrupted field.  damon_commit_ctx() sets it
> > > when it is failed.  kdamond_call() checks if the field is set after each
> > > damon_call_control->fn() is executed.  If it is set, ignore remaining
> > > callback requests and return.  All kdamond_call() callers including
> > > kdamond_fn() also check the maybe_corrupted field right after
> > > kdamond_call() invocations.  If the field is set, break the
> > > kdamond_fn() main loop so that DAMON sill doesn't use the context that
> > > might be corrupted.
> > 
> > I guess you saw the AI review?
> > 	https://sashiko.dev/#/patchset/20260319145218.86197-1-sj%40kernel.org
> 
> By the way, I am also doing monitoring of sashiko.dev for all DAMON patches.
> It will be much easier once sashiko.dev's email feature is ready, since I
> already onboarded DAMON for that.
> 
> Meanwhile, the monitoring using web browser is somewhat tedious for me, so I
> just implemented an hkml feature, namely
> 'hkml patch sashiko_dev --thread_status'.  It receives a message id of a mail,
> and prints the review status/result of all patches of the thread.
> 
> E.g.,
> 
>     $ hkml patch sashiko_dev --thread_status 20260319-memory-failure-mf-delayed-fix-rfc-v2-v2-0-92c596402a7a@google.com
>     - [PATCH RFC v2 1/7] mm: memory_failure: Clarify the MF_DELAYED definition
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 2/7] mm: memory_failure: Allow truncate_error_folio to return MF_DELAYED
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 3/7] mm: shmem: Update shmem handler to the MF_DELAYED definition
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 4/7] mm: memory_failure: Generalize extra_pins handling to all MF_DELAYED cases
>       - Pending (None)
>     - [PATCH RFC v2 4/7] mm: memory_failure: Generalize extra_pins handling to all MF_DELAYED cases
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 5/7] mm: selftests: Add shmem memory failure test
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 6/7] KVM: selftests: Add memory failure tests in guest_memfd_test
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 7/7] KVM: selftests: Test guest_memfd behavior with respect to stage 2 page tables
>       - Reviewed (Review completed successfully.)
> 
> I'm planning to implement another feature for formatting and sending the review
> result and inline comments as emails, probably this weekend.

I wanted to add the link to the commit that implementing the feature, but
forgot that, sorry.  Here it is:
https://github.com/sjp38/hackermail/commit/cf1b4e167067e5684823137c5296dfb268364175

FWIW, iiuc, 'b4 review' should also provide similar or better feature.


Thanks,
SJ

[...]