All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>,
	Corey Ashford <cjashfor@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: [PATCH 14/15] perf_counter: remove rq->lock usage
Date: Mon, 06 Apr 2009 11:45:12 +0200	[thread overview]
Message-ID: <20090406094518.531137582@chello.nl> (raw)
In-Reply-To: 20090406094458.977814421@chello.nl

[-- Attachment #1: per_counter-remove-rq_lock.patch --]
[-- Type: text/plain, Size: 5812 bytes --]

Now that all the task runtime clock users are gone, remove the ugly
rq->lock usage from perf counters, which solves the nasty deadlock
seen when a software task clock counter was read from an NMI overflow
context.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 include/linux/kernel_stat.h |    2 --
 kernel/perf_counter.c       |   42 ++++++++++++++++--------------------------
 kernel/sched.c              |   20 --------------------
 3 files changed, 16 insertions(+), 48 deletions(-)

Index: linux-2.6/include/linux/kernel_stat.h
===================================================================
--- linux-2.6.orig/include/linux/kernel_stat.h
+++ linux-2.6/include/linux/kernel_stat.h
@@ -85,8 +85,6 @@ static inline unsigned int kstat_irqs(un
 /*
  * Lock/unlock the current runqueue - to extract task statistics:
  */
-extern void curr_rq_lock_irq_save(unsigned long *flags);
-extern void curr_rq_unlock_irq_restore(unsigned long *flags);
 extern unsigned long long __task_delta_exec(struct task_struct *tsk, int update);
 extern unsigned long long task_delta_exec(struct task_struct *);
 
Index: linux-2.6/kernel/perf_counter.c
===================================================================
--- linux-2.6.orig/kernel/perf_counter.c
+++ linux-2.6/kernel/perf_counter.c
@@ -172,8 +172,7 @@ static void __perf_counter_remove_from_c
 	if (ctx->task && cpuctx->task_ctx != ctx)
 		return;
 
-	curr_rq_lock_irq_save(&flags);
-	spin_lock(&ctx->lock);
+	spin_lock_irqsave(&ctx->lock, flags);
 
 	counter_sched_out(counter, cpuctx, ctx);
 
@@ -198,8 +197,7 @@ static void __perf_counter_remove_from_c
 			    perf_max_counters - perf_reserved_percpu);
 	}
 
-	spin_unlock(&ctx->lock);
-	curr_rq_unlock_irq_restore(&flags);
+	spin_unlock_irqrestore(&ctx->lock, flags);
 }
 
 
@@ -319,8 +317,7 @@ static void __perf_counter_disable(void 
 	if (ctx->task && cpuctx->task_ctx != ctx)
 		return;
 
-	curr_rq_lock_irq_save(&flags);
-	spin_lock(&ctx->lock);
+	spin_lock_irqsave(&ctx->lock, flags);
 
 	/*
 	 * If the counter is on, turn it off.
@@ -336,8 +333,7 @@ static void __perf_counter_disable(void 
 		counter->state = PERF_COUNTER_STATE_OFF;
 	}
 
-	spin_unlock(&ctx->lock);
-	curr_rq_unlock_irq_restore(&flags);
+	spin_unlock_irqrestore(&ctx->lock, flags);
 }
 
 /*
@@ -515,8 +511,7 @@ static void __perf_install_in_context(vo
 	if (ctx->task && cpuctx->task_ctx != ctx)
 		return;
 
-	curr_rq_lock_irq_save(&flags);
-	spin_lock(&ctx->lock);
+	spin_lock_irqsave(&ctx->lock, flags);
 	update_context_time(ctx);
 
 	/*
@@ -565,8 +560,7 @@ static void __perf_install_in_context(vo
  unlock:
 	hw_perf_restore(perf_flags);
 
-	spin_unlock(&ctx->lock);
-	curr_rq_unlock_irq_restore(&flags);
+	spin_unlock_irqrestore(&ctx->lock, flags);
 }
 
 /*
@@ -641,8 +635,7 @@ static void __perf_counter_enable(void *
 	if (ctx->task && cpuctx->task_ctx != ctx)
 		return;
 
-	curr_rq_lock_irq_save(&flags);
-	spin_lock(&ctx->lock);
+	spin_lock_irqsave(&ctx->lock, flags);
 	update_context_time(ctx);
 
 	counter->prev_state = counter->state;
@@ -678,8 +671,7 @@ static void __perf_counter_enable(void *
 	}
 
  unlock:
-	spin_unlock(&ctx->lock);
-	curr_rq_unlock_irq_restore(&flags);
+	spin_unlock_irqrestore(&ctx->lock, flags);
 }
 
 /*
@@ -971,7 +963,7 @@ int perf_counter_task_disable(void)
 	if (likely(!ctx->nr_counters))
 		return 0;
 
-	curr_rq_lock_irq_save(&flags);
+	local_irq_save(flags);
 	cpu = smp_processor_id();
 
 	perf_counter_task_sched_out(curr, cpu);
@@ -992,9 +984,7 @@ int perf_counter_task_disable(void)
 
 	hw_perf_restore(perf_flags);
 
-	spin_unlock(&ctx->lock);
-
-	curr_rq_unlock_irq_restore(&flags);
+	spin_unlock_irqrestore(&ctx->lock, flags);
 
 	return 0;
 }
@@ -1011,7 +1001,7 @@ int perf_counter_task_enable(void)
 	if (likely(!ctx->nr_counters))
 		return 0;
 
-	curr_rq_lock_irq_save(&flags);
+	local_irq_save(flags);
 	cpu = smp_processor_id();
 
 	perf_counter_task_sched_out(curr, cpu);
@@ -1037,7 +1027,7 @@ int perf_counter_task_enable(void)
 
 	perf_counter_task_sched_in(curr, cpu);
 
-	curr_rq_unlock_irq_restore(&flags);
+	local_irq_restore(flags);
 
 	return 0;
 }
@@ -1095,12 +1085,12 @@ static void __read(void *info)
 	struct perf_counter_context *ctx = counter->ctx;
 	unsigned long flags;
 
-	curr_rq_lock_irq_save(&flags);
+	local_irq_save(flags);
 	if (ctx->is_active)
 		update_context_time(ctx);
 	counter->hw_ops->read(counter);
 	update_counter_times(counter);
-	curr_rq_unlock_irq_restore(&flags);
+	local_irq_restore(flags);
 }
 
 static u64 perf_counter_read(struct perf_counter *counter)
@@ -2890,7 +2880,7 @@ __perf_counter_exit_task(struct task_str
 		 * Be careful about zapping the list - IRQ/NMI context
 		 * could still be processing it:
 		 */
-		curr_rq_lock_irq_save(&flags);
+		local_irq_save(flags);
 		perf_flags = hw_perf_save_disable();
 
 		cpuctx = &__get_cpu_var(perf_cpu_context);
@@ -2903,7 +2893,7 @@ __perf_counter_exit_task(struct task_str
 		child_ctx->nr_counters--;
 
 		hw_perf_restore(perf_flags);
-		curr_rq_unlock_irq_restore(&flags);
+		local_irq_restore(flags);
 	}
 
 	parent_counter = child_counter->parent;
Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -1004,26 +1004,6 @@ static struct rq *task_rq_lock(struct ta
 	}
 }
 
-void curr_rq_lock_irq_save(unsigned long *flags)
-	__acquires(rq->lock)
-{
-	struct rq *rq;
-
-	local_irq_save(*flags);
-	rq = cpu_rq(smp_processor_id());
-	spin_lock(&rq->lock);
-}
-
-void curr_rq_unlock_irq_restore(unsigned long *flags)
-	__releases(rq->lock)
-{
-	struct rq *rq;
-
-	rq = cpu_rq(smp_processor_id());
-	spin_unlock(&rq->lock);
-	local_irq_restore(*flags);
-}
-
 void task_rq_unlock_wait(struct task_struct *p)
 {
 	struct rq *rq = task_rq(p);

-- 


  parent reply	other threads:[~2009-04-06  9:49 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-06  9:44 [PATCH 00/15] various perf counter bits Peter Zijlstra
2009-04-06  9:44 ` [PATCH 01/15] perf_counter: update mmap() counter read, take 2 Peter Zijlstra
2009-04-07  9:06   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 02/15] perf_counter: add more context information Peter Zijlstra
2009-04-07  9:07   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 03/15] perf_counter: SIGIO support Peter Zijlstra
2009-04-07  9:07   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 04/15] perf_counter: generalize pending infrastructure Peter Zijlstra
2009-04-07  9:07   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 05/15] perf_counter: x86: self-IPI for pending work Peter Zijlstra
2009-04-07  9:07   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 06/15] perf_counter: theres more to overflow than writing events Peter Zijlstra
2009-04-07  9:07   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 07/15] perf_counter: fix the mlock accounting Peter Zijlstra
2009-04-07  9:08   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 08/15] perf_counter: PERF_RECORD_TIME Peter Zijlstra
2009-04-07  9:08   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 09/15] perf_counter: counter overflow limit Peter Zijlstra
2009-04-07  9:08   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 10/15] perf_counter: comment the perf_event_type stuff Peter Zijlstra
2009-04-07  9:08   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 11/15] perf_counter: change event defenition Peter Zijlstra
2009-04-07  9:08   ` [tip:perfcounters/core] perf_counter: change event definition Peter Zijlstra
2009-04-06  9:45 ` [PATCH 12/15] perf_counter: rework context time Peter Zijlstra
2009-04-07  9:09   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-06  9:45 ` [PATCH 13/15] perf_counter: rework the task clock software counter Peter Zijlstra
2009-04-07  9:09   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-07  9:36   ` [tip:perfcounters/core] x86, perfcounters: add atomic64_xchg() Ingo Molnar
2009-04-07 11:19     ` Paul Mackerras
2009-04-07 13:28       ` Ingo Molnar
2009-04-07 10:06   ` Ingo Molnar
2009-04-06  9:45 ` Peter Zijlstra [this message]
2009-04-07  9:09   ` [tip:perfcounters/core] perf_counter: remove rq->lock usage Peter Zijlstra
2009-04-06  9:45 ` [PATCH 15/15] perf_counter: minimize context time updates Peter Zijlstra
2009-04-07  9:09   ` [tip:perfcounters/core] " Peter Zijlstra
2009-04-07  9:21 ` [PATCH 00/15] various perf counter bits Ingo Molnar
2009-04-07  9:21   ` Ingo Molnar
2009-04-07  9:33   ` Ingo Molnar
2009-04-07  9:23 ` 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=20090406094518.531137582@chello.nl \
    --to=a.p.zijlstra@chello.nl \
    --cc=cjashfor@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.