[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1d9d8520-bb4d-4af3-a395-4a4e594e67e8@amd.com>
Date: Mon, 17 Mar 2025 12:19:07 +0530
From: Ravi Bangoria <ravi.bangoria@....com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: mingo@...nel.org, lucas.demarchi@...el.com, linux-kernel@...r.kernel.org,
acme@...nel.org, namhyung@...nel.org, mark.rutland@....com,
alexander.shishkin@...ux.intel.com, jolsa@...nel.org, irogers@...gle.com,
adrian.hunter@...el.com, kan.liang@...ux.intel.com,
Ravi Bangoria <ravi.bangoria@....com>
Subject: Re: [PATCH v3 3/7] perf: Simplify perf_event_free_task() wait
Hi Peter,
> @@ -1223,8 +1223,14 @@ static void put_ctx(struct perf_event_co
> if (refcount_dec_and_test(&ctx->refcount)) {
> if (ctx->parent_ctx)
> put_ctx(ctx->parent_ctx);
> - if (ctx->task && ctx->task != TASK_TOMBSTONE)
> - put_task_struct(ctx->task);
> + if (ctx->task) {
> + if (ctx->task == TASK_TOMBSTONE) {
> + smp_mb(); /* pairs with wait_var_event() */
> + wake_up_var(&ctx->refcount);
perf_event_free_task() waits on "ctx->refcount == 1". But moving
wake_up_var() under refcount_dec_and_test() will cause
perf_event_free_task() to wait indefinitely. Right? So, shouldn't
wake_up_var() be outside? something like:
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1281,15 +1281,14 @@ static void put_ctx(struct perf_event_context *ctx)
if (refcount_dec_and_test(&ctx->refcount)) {
if (ctx->parent_ctx)
put_ctx(ctx->parent_ctx);
- if (ctx->task) {
- if (ctx->task == TASK_TOMBSTONE) {
- smp_mb(); /* pairs with wait_var_event() */
- wake_up_var(&ctx->refcount);
- } else {
- put_task_struct(ctx->task);
- }
- }
+ if (ctx->task && ctx->task != TASK_TOMBSTONE)
+ put_task_struct(ctx->task);
call_rcu(&ctx->rcu_head, free_ctx);
+ } else {
+ if (ctx->task == TASK_TOMBSTONE) {
+ smp_mb(); /* pairs with wait_var_event() */
+ wake_up_var(&ctx->refcount);
+ }
}
}
Thanks,
Ravi
Powered by blists - more mailing lists