tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

mm/damon/core: fix damon_call() vs kdamond_fn() exit race

Patch series "mm/damon/core: fix damon_call()/damos_walk() vs kdmond exit
race".

damon_call() and damos_walk() can leak memory and/or deadlock when they
race with kdamond terminations. Fix those.


This patch (of 2);

When kdamond_fn() main loop is finished, the function cancels all
remaining damon_call() requests and unset the damon_ctx->kdamond so that
API callers and API functions themselves can know the context is
terminated. damon_call() adds the caller's request to the queue first.
After that, it shows if the kdamond of the damon_ctx is still running
(damon_ctx->kdamond is set). Only if the kdamond is running, damon_call()
starts waiting for the kdamond's handling of the newly added request.

The damon_call() requests registration and damon_ctx->kdamond unset are
protected by different mutexes, though. Hence, damon_call() could race
with damon_ctx->kdamond unset, and result in deadlocks.

For example, let's suppose kdamond successfully finished the damon_call()
requests cancelling. Right after that, damon_call() is called for the
context. It registers the new request, and shows the context is still
running, because damon_ctx->kdamond unset is not yet done. Hence the
damon_call() caller starts waiting for the handling of the request.
However, the kdamond is already on the termination steps, so it never
handles the new request. As a result, the damon_call() caller threads
infinitely waits.

Fix this by introducing another damon_ctx field, namely
call_controls_obsolete. It is protected by the
damon_ctx->call_controls_lock, which protects damon_call() requests
registration. Initialize (unset) it in kdamond_fn() before letting
damon_start() returns and set it just before the cancelling of remaining
damon_call() requests is executed. damon_call() reads the obsolete field
under the lock and avoids adding a new request.

After this change, only requests that are guaranteed to be handled or
cancelled are registered. Hence the after-registration DAMON context
termination check is no longer needed. Remove it together.

Note that the deadlock will not happen when damon_call() is called for
repeat mode request. In tis case, damon_call() returns instead of waiting
for the handling when the request registration succeeds and it shows the
kdamond is running. However, if the request also has dealloc_on_cancel,
the request memory would be leaked.

The issue is found by sashiko [1].

Link: https://lore.kernel.org/20260327233319.3528-1-sj@kernel.org
Link: https://lore.kernel.org/20260327233319.3528-2-sj@kernel.org
Link: https://lore.kernel.org/20260325141956.87144-1-sj@kernel.org [1]
Fixes: 42b7491af14c ("mm/damon/core: introduce damon_call()")
Signed-off-by: SeongJae Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> # 6.14.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

authored by

SeongJae Park and committed by
Andrew Morton 55da8166 ef3c0f6c

+15 -31
+1
include/linux/damon.h
··· 818 818 819 819 /* lists of &struct damon_call_control */ 820 820 struct list_head call_controls; 821 + bool call_controls_obsolete; 821 822 struct mutex call_controls_lock; 822 823 823 824 struct damos_walk_control *walk_control;
+14 -31
mm/damon/core.c
··· 1573 1573 return pid; 1574 1574 } 1575 1575 1576 - /* 1577 - * damon_call_handle_inactive_ctx() - handle DAMON call request that added to 1578 - * an inactive context. 1579 - * @ctx: The inactive DAMON context. 1580 - * @control: Control variable of the call request. 1581 - * 1582 - * This function is called in a case that @control is added to @ctx but @ctx is 1583 - * not running (inactive). See if @ctx handled @control or not, and cleanup 1584 - * @control if it was not handled. 1585 - * 1586 - * Returns 0 if @control was handled by @ctx, negative error code otherwise. 1587 - */ 1588 - static int damon_call_handle_inactive_ctx( 1589 - struct damon_ctx *ctx, struct damon_call_control *control) 1590 - { 1591 - struct damon_call_control *c; 1592 - 1593 - mutex_lock(&ctx->call_controls_lock); 1594 - list_for_each_entry(c, &ctx->call_controls, list) { 1595 - if (c == control) { 1596 - list_del(&control->list); 1597 - mutex_unlock(&ctx->call_controls_lock); 1598 - return -EINVAL; 1599 - } 1600 - } 1601 - mutex_unlock(&ctx->call_controls_lock); 1602 - return 0; 1603 - } 1604 - 1605 1576 /** 1606 1577 * damon_call() - Invoke a given function on DAMON worker thread (kdamond). 1607 1578 * @ctx: DAMON context to call the function for. ··· 1590 1619 * synchronization. The return value of the function will be saved in 1591 1620 * &damon_call_control->return_code. 1592 1621 * 1622 + * Note that this function should be called only after damon_start() with the 1623 + * @ctx has succeeded. Otherwise, this function could fall into an indefinite 1624 + * wait. 1625 + * 1593 1626 * Return: 0 on success, negative error code otherwise. 1594 1627 */ 1595 1628 int damon_call(struct damon_ctx *ctx, struct damon_call_control *control) ··· 1604 1629 INIT_LIST_HEAD(&control->list); 1605 1630 1606 1631 mutex_lock(&ctx->call_controls_lock); 1632 + if (ctx->call_controls_obsolete) { 1633 + mutex_unlock(&ctx->call_controls_lock); 1634 + return -ECANCELED; 1635 + } 1607 1636 list_add_tail(&control->list, &ctx->call_controls); 1608 1637 mutex_unlock(&ctx->call_controls_lock); 1609 - if (!damon_is_running(ctx)) 1610 - return damon_call_handle_inactive_ctx(ctx, control); 1611 1638 if (control->repeat) 1612 1639 return 0; 1613 1640 wait_for_completion(&control->completion); ··· 2929 2952 2930 2953 pr_debug("kdamond (%d) starts\n", current->pid); 2931 2954 2955 + mutex_lock(&ctx->call_controls_lock); 2956 + ctx->call_controls_obsolete = false; 2957 + mutex_unlock(&ctx->call_controls_lock); 2932 2958 complete(&ctx->kdamond_started); 2933 2959 kdamond_init_ctx(ctx); 2934 2960 ··· 3042 3062 damon_destroy_targets(ctx); 3043 3063 3044 3064 kfree(ctx->regions_score_histogram); 3065 + mutex_lock(&ctx->call_controls_lock); 3066 + ctx->call_controls_obsolete = true; 3067 + mutex_unlock(&ctx->call_controls_lock); 3045 3068 kdamond_call(ctx, true); 3046 3069 damos_walk_cancel(ctx); 3047 3070