linux-kernel - Re: [PATCH v2] events/core: fix acoount failure for event's total_enable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241220153040.GP11133@noisy.programming.kicks-ass.net>
Date: Fri, 20 Dec 2024 16:30:40 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Yeoreum Yun <yeoreum.yun@....com>
Cc: mingo@...hat.com, 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
Subject: Re: [PATCH v2] events/core: fix acoount failure for event's
 total_enable_time

On Fri, Dec 20, 2024 at 03:26:42PM +0000, Yeoreum Yun wrote:

> > OK, how's this then?
> >
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 065f9188b44a..d12b402f9751 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -2422,6 +2422,7 @@ __perf_remove_from_context(struct perf_event *event,
> >  {
> >  	struct perf_event_pmu_context *pmu_ctx = event->pmu_ctx;
> >  	unsigned long flags = (unsigned long)info;
> > +	enum perf_event_state state = PERF_EVENT_STATE_OFF;
> >
> >  	ctx_time_update(cpuctx, ctx);
> >
> > @@ -2438,7 +2439,9 @@ __perf_remove_from_context(struct perf_event *event,
> >  		perf_child_detach(event);
> >  	list_del_event(event, ctx);
> >  	if (flags & DETACH_DEAD)
> > -		event->state = PERF_EVENT_STATE_DEAD;
> > +		state = PERF_EVENT_STATE_DEAD;
> > +
> > +	perf_event_set_state(event, state);
> >
> 
> It works. but what about this?
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 065f9188b44a..71ed8f847b04 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2432,6 +2432,7 @@ __perf_remove_from_context(struct perf_event *event,
>         if (flags & DETACH_DEAD)
>                 event->pending_disable = 1;
>         event_sched_out(event, ctx);
> +       perf_event_update_time(event);
>         if (flags & DETACH_GROUP)
>                 perf_group_detach(event);
>         if (flags & DETACH_CHILD)

Well, the rule is that timekeeping is tied to state change. And as you
can see __perf_remove_from_context() violates that.

Probably because someone (probably me) figured that the event would not
be observed again after detach or something like that.