linux-kernel - Re: [PATCH 2/7] perf: Generalize task_function

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] [day] [month] [year] [list]

Message-ID: <877fjg9yzj.fsf@ashishki-desk.ger.corp.intel.com>
Date:	Mon, 11 Jan 2016 12:44:48 +0200
From:	Alexander Shishkin <alexander.shishkin@...ux.intel.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
	vince@...ter.net, eranian@...gle.com, johannes@...solutions.net,
	Arnaldo Carvalho de Melo <acme@...radead.org>
Subject: Re: [PATCH 2/7] perf: Generalize task_function_call()ers

Peter Zijlstra <peterz@...radead.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