[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250604151756.GD8020@e132581.arm.com>
Date: Wed, 4 Jun 2025 16:17:56 +0100
From: Leo Yan <leo.yan@....com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Yeoreum Yun <yeoreum.yun@....com>, mingo@...hat.com, mingo@...nel.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, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, David Wang <00107082@....com>
Subject: Re: [PATCH 1/1] perf/core: fix dangling cgroup pointer in cpuctx
On Wed, Jun 04, 2025 at 03:58:01PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 04, 2025 at 11:18:21AM +0100, Leo Yan wrote:
>
> > Totally agree. The comment would be very useful!
>
> I have the below. Let me go read your email in more than 2 seconds to
> see if I should amend things.
>
> I'm not sure doing the INACTIVE->INACTIVE cycle in the ASCII art is
> going to make it clearer, might leave that off. But yeah, possible.
>
> ---
>
> Subject: perf: Add comment to enum perf_event_state
> From: Peter Zijlstra <peterz@...radead.org>
> Date: Wed Jun 4 10:21:38 CEST 2025
>
> Better describe the event states.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
> include/linux/perf_event.h | 42 ++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 40 insertions(+), 2 deletions(-)
>
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -635,8 +635,46 @@ struct perf_addr_filter_range {
> unsigned long size;
> };
>
> -/**
> - * enum perf_event_state - the states of an event:
> +/*
> + * The normal states are:
> + *
> + * ACTIVE --.
> + * ^ |
> + * | |
> + * sched_{in,out}() |
> + * | |
> + * v |
> + * ,---> INACTIVE --+ <-.
> + * | | |
> + * | {dis,en}able()
> + * sched_in() | |
> + * | OFF <--' --+
> + * | |
> + * `---> ERROR ------'
> + *
> + * That is:
> + *
> + * sched_in: INACTIVE -> {ACTIVE,ERROR}
> + * sched_out: ACTIVE -> INACTIVE
> + * disable: {ACTIVE,INACTIVE} -> OFF
> + * enable: {OFF,ERROR} -> INACTIVE
> + *
> + * Where {OFF,ERROR} are disabled states.
> + *
> + * Then we have the {EXIT,REVOKED,DEAD} states which are various shades of
> + * defunct events:
> + *
> + * - EXIT means task that the even was assigned to died, but child events
> + * still live, and further children can still be created. But the event
> + * itself will never be active again. It can only transition to
> + * {REVOKED,DEAD};
> + *
> + * - REVOKED means the PMU the event was associated with is gone; all
> + * functionality is stopped but the event is still alive. Can only
> + * transition to DEAD;
> + *
> + * - DEAD event really is DYING tearing down state and freeing bits.
> + *
LGTM:
Reviewed-by: Leo Yan <leo.yan@....com>
> */
> enum perf_event_state {
> PERF_EVENT_STATE_DEAD = -5,
Powered by blists - more mailing lists