Messages in this thread | | | Date | Thu, 5 Jun 2014 16:11:50 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 5/9] perf/x86: implement cross-HT corruption bug workaround |
| |
On Thu, Jun 05, 2014 at 05:01:25PM +0300, Maria Dimakopoulou wrote: > On Thu, Jun 5, 2014 at 4:48 PM, Peter Zijlstra <peterz@infradead.org> wrote: > > On Wed, Jun 04, 2014 at 11:34:14PM +0200, Stephane Eranian wrote: > > > >> +static struct event_constraint * > >> +intel_get_excl_constraints(struct cpu_hw_events *cpuc, struct perf_event *event, > >> + struct event_constraint *c) > >> +{ > > > >> + if (!(c->flags & PERF_X86_EVENT_DYNAMIC)) { > >> + > >> + /* > >> + * in case we fail, we assume no counter > >> + * is supported to be on the safe side > >> + */ > >> + cx = kmalloc(sizeof(*cx), GFP_KERNEL); > >> + if (!cx) > >> + return &emptyconstraint; > >> + > > > > Ok, so forgive me if I'm wrong, but the way we get here is through: > > > > x86_schedule_event() > > ->start_scheduling() > > spin_lock() > > ->get_event_constraints() > > intel_get_excl_constraints() > > kmalloc(.gfp=GFP_KERNEL) > > > > How can that ever work?
> Are you saying it is illegal to call kmalloc() from > this context?
Nobody will come and arrest you for it, so no. Broken though. GFP_KERNEL will attempt to sleep to wait for reclaim, and you're holding a spinlock.
> kmalloc is needed because we need to allocate > a new constraint struct since the static constraint > cannot be modified. > > Worst case we can statically allocate a second > constraint struct in the event struct.
Nah, since you will need at most one constraint per counter, you could preallocate num_counter constraints for each cpu. [unhandled content-type:application/pgp-signature] | |