Messages in this thread | | | Date | Thu, 21 May 2015 14:56:15 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 01/10] perf,x86: Fix event/group validation |
| |
On Thu, May 21, 2015 at 05:35:02AM -0700, Stephane Eranian wrote: > Peter, > > On Thu, May 21, 2015 at 4:17 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > Commit 43b4578071c0 ("perf/x86: Reduce stack usage of > > x86_schedule_events()") violated the rule that 'fake' scheduling; as > > used for event/group validation; should not change the event state. > > > > This went mostly un-noticed because repeated calls of > > x86_pmu::get_event_constraints() would give the same result. And > > x86_pmu::put_event_constraints() would mostly not do anything. > > > > Things could still go wrong esp. for shared_regs, because > > cpuc->is_fake can return a different constraint and then the > > put_event_constraint will not match up. > > > I don't follow this here. What do you mean by 'match up'?
Ah, I wrote that Changelog for a prior patch; which by writing the changelog I found faulty.
But I then forgot to update the Changelog.
I was under the impression put_event_constraints() would actually take the constraint as an argument, and with the below example, it would not do put on the same it would get.
> > Commit e979121b1b15 ("perf/x86/intel: Implement cross-HT corruption > > bug workaround") made the situation much worse by actually setting the > > event->hw.constraint value to NULL, so when validation and actual > > scheduling interact we get NULL ptr derefs. > > > > But x86_schedule_events() does reset the hw.constraint for each invocation: > > c = x86_pmu.get_event_constraints(cpuc, i, cpuc->event_list[i]); > hwc->constraint = c;
Yes, so if you have:
validate_group()
hwc->constraint = c;
<context switch>
c = hwc->constraint;
The second c might not be the first.
| |