Messages in this thread | | | From | Alexander Shishkin <> | Subject | Re: [PATCH 2/7] perf: Generalize task_function_call()ers | Date | Mon, 11 Jan 2016 12:44:48 +0200 |
| |
Peter Zijlstra <peterz@infradead.org> writes:
> On Thu, Dec 17, 2015 at 04:07:32PM +0100, Peter Zijlstra wrote: >> On Thu, Dec 17, 2015 at 04:25:14PM +0200, Alexander Shishkin wrote: >> >> > That aside, why I brought it up in the first place is because the two >> > functions are asymmetric: one is called with irqs disabled and the >> > other -- with ctx::lock held (and not because I'm into bikeshedding or >> > anything like that). Looking at the pair of them sets off my "that's not >> > right" trigger and sends me to the event_function_call() >> > implementation. So in that sense, prepending an extra underscore kind of >> > made sense. Maybe __perf_remove_from_context_{on,off}()? >> >> You are quite right, and I think I've found more problems because of >> this. Let me prod at this some more. > > So this then... > > This fixes, I think, 3 separate bugs: > > - remove_from_context used to clear ->is_active, this is against the > update rules from ctx_sched_in() which set ->is_active even though > there might be !nr_events > > - install_in_context did bad things to cpuctx->task_ctx; it would not > validate that ctx->task == current and could do random things because > of that. > > - cpuctx->task_ctx tracking was iffy > > It also unifies a lot of the weird and fragile code we had around all > those IPI calls and adds a bunch of assertions. > > It seems to survive a little pounding with 'normal' workloads. > > Please have an extra careful look..
I notice that you dropped this from your queue, do you have any plans to proceed with this, or should I pick it up?
Regards, -- Alex
| |