mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
To: Peter Zijlstra <peterz@infradead.org>,
	mingo@kernel.org, eranian@google.com
Cc: linux-kernel@vger.kernel.org, vince@deater.net,
	dvyukov@google.com, andi@firstfloor.org, jolsa@redhat.com,
	peterz@infradead.org
Subject: Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users
Date: Wed, 13 Jan 2016 17:00:50 +0200	[thread overview]
Message-ID: <87r3hl8qxp.fsf@ashishki-desk.ger.corp.intel.com> (raw)
In-Reply-To: <20160111163229.411314288@infradead.org>

I think I caught one, below.

Peter Zijlstra <peterz@infradead.org> writes:

> +static int event_function(void *info)
> +{
> +	struct event_function_struct *efs = info;
> +	struct perf_event *event = efs->event;
> +	struct perf_event_context *ctx = event->ctx;
> +	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
> +	struct perf_event_context *task_ctx = cpuctx->task_ctx;
> +
> +	WARN_ON_ONCE(!irqs_disabled());
> +
> +	/*
> +	 * Since we do the IPI call without holding ctx->lock things can have
> +	 * changed, double check we hit the task we set out to hit.
> +	 *
> +	 * If ctx->task == current, we know things must remain valid because
> +	 * we have IRQs disabled so we cannot schedule.
> +	 */
> +	if (ctx->task) {
> +		if (ctx->task != current)
> +			return -EAGAIN;
> +
> +		WARN_ON_ONCE(task_ctx != ctx);

Looks like between dropping ctx::lock in event_function_call() and here,
cpuctx::task_ctx may still become NULL.

> +	} else {
> +		WARN_ON_ONCE(&cpuctx->ctx != ctx);
> +	}
> +
> +	perf_ctx_lock(cpuctx, task_ctx);
> +	/*
> +	 * Now that we hold locks, double check state. Paranoia pays.
> +	 */
> +	if (task_ctx) {
> +		WARN_ON_ONCE(task_ctx->task != current);
> +		/*
> +		 * We only use event_function_call() on established contexts,
> +		 * and event_function() is only ever called when active (or
> +		 * rather, we'll have bailed in task_function_call() or the
> +		 * above ctx->task != current test), therefore we must have
> +		 * ctx->is_active here.
> +		 */
> +		WARN_ON_ONCE(!ctx->is_active);
> +		/*
> +		 * And since we have ctx->is_active, cpuctx->task_ctx must
> +		 * match.
> +		 */
> +		WARN_ON_ONCE(cpuctx->task_ctx != task_ctx);
> +	}
> +	efs->func(event, cpuctx, ctx, efs->data);

In which case we probably don't want to call the callback.

Not sure if this is what Dmitry ran into, his logs contain warnings from
this function, but hard to tell exactly which ones.

Regards,
--
Alex

  parent reply	other threads:[~2016-01-13 15:00 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-11 16:24 [RFC][PATCH 00/12] various perf fixes Peter Zijlstra
2016-01-11 16:24 ` [RFC][PATCH 01/12] perf: Add lockdep assertions Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 02/12] perf: Fix cgroup event scheduling Peter Zijlstra
2016-01-11 19:43   ` Stephane Eranian
2016-01-11 22:03     ` Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 03/12] perf: Fix cgroup scheduling in enable_on_exec Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 04/12] perf: Remove stale comment Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 05/12] perf: Fix enable_on_exec event scheduling Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 06/12] perf: Use task_ctx_sched_out() Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 07/12] perf: Simplify/fix perf_event_enable() event scheduling Peter Zijlstra
2016-03-08 10:04   ` James Morse
2016-03-08 10:26     ` Peter Zijlstra
2016-03-08 10:42       ` James Morse
2016-03-08 11:29         ` Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 08/12] perf: Optimize perf_sched_events usage Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 09/12] perf: Make ctx->is_active and cpuctx->task_ctx consistent Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 10/12] perf: Fix task context scheduling Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 11/12] perf: Specialize perf_event_exit_task() Peter Zijlstra
2016-01-11 16:25 ` [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users Peter Zijlstra
2016-01-13 10:50   ` Peter Zijlstra
2016-01-13 13:46     ` Alexander Shishkin
2016-01-13 17:30       ` Peter Zijlstra
2016-01-13 15:00   ` Alexander Shishkin [this message]
2016-01-13 15:38     ` Alexander Shishkin
2016-01-13 18:10     ` Peter Zijlstra
2016-01-13 20:44       ` Peter Zijlstra
2016-01-14 10:44     ` Peter Zijlstra
2016-01-14 16:30       ` Peter Zijlstra
2016-02-17 22:38   ` Sasha Levin
2016-02-18  7:46     ` Peter Zijlstra
2016-02-18 12:37       ` Peter Zijlstra
2016-01-11 18:44 ` [RFC][PATCH 00/12] various perf fixes Dmitry Vyukov
2016-01-11 19:56   ` Andi Kleen
2016-01-11 22:00   ` Peter Zijlstra
2016-01-12  9:59     ` Ingo Molnar
2016-01-12 10:11 ` Ingo Molnar
2016-01-12 10:57   ` Dmitry Vyukov
2016-01-12 11:00     ` Dmitry Vyukov
2016-01-12 11:01       ` Dmitry Vyukov
2016-01-12 11:26         ` Dmitry Vyukov
2016-01-12 11:35           ` Dmitry Vyukov
2016-01-12 12:01           ` Peter Zijlstra
2016-01-13 15:18           ` Alexander Shishkin
2016-01-13 15:22             ` Dmitry Vyukov
2016-01-13 15:35               ` Alexander Shishkin
2016-01-14  9:35           ` Peter Zijlstra
2016-01-14 10:05             ` Dmitry Vyukov
2016-02-15 15:04               ` Dmitry Vyukov
2016-02-15 15:38                 ` Peter Zijlstra
2016-02-15 15:47                   ` Dmitry Vyukov
2016-02-15 16:01                   ` Vince Weaver
2016-02-15 16:17               ` Peter Zijlstra
2016-02-15 16:29                 ` Dmitry Vyukov
2016-02-15 16:29               ` Peter Zijlstra
2016-02-15 16:35                 ` Dmitry Vyukov
2016-02-15 16:38                   ` Dmitry Vyukov
2016-02-15 16:52                     ` Peter Zijlstra
2016-02-15 17:04                       ` Dmitry Vyukov
2016-02-15 17:07                         ` Peter Zijlstra
2016-02-15 17:45                           ` Dmitry Vyukov
2016-02-15 18:01                             ` Peter Zijlstra
2016-02-15 18:33                               ` Dmitry Vyukov
2016-02-15 16:41                   ` Peter Zijlstra
2016-02-15 16:54                     ` Dmitry Vyukov
2016-02-15 16:59                       ` Peter Zijlstra
2016-01-12 13:13   ` Peter Zijlstra
2016-01-12 13:31     ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r3hl8qxp.fsf@ashishki-desk.ger.corp.intel.com \
    --to=alexander.shishkin@linux.intel.com \
    --cc=andi@firstfloor.org \
    --cc=dvyukov@google.com \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=vince@deater.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome