mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Stephane Eranian <eranian@google.com>
Cc: linux-kernel@vger.kernel.org, mingo@elte.hu, ak@linux.intel.com,
	acme@redhat.com, jolsa@redhat.com, zheng.z.yan@intel.com,
	bp@alien8.de, maria.n.dimakopoulou@gmail.com
Subject: Re: [PATCH v5 3/4] perf,x86: add Intel RAPL PMU support
Date: Wed, 6 Nov 2013 11:42:43 +0100	[thread overview]
Message-ID: <20131106104243.GF10651@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <1383670886-30827-4-git-send-email-eranian@google.com>

On Tue, Nov 05, 2013 at 06:01:25PM +0100, Stephane Eranian wrote:
> +static int rapl_cpu_dying(int cpu)
> +{
> +	struct rapl_pmu *pmu = per_cpu(rapl_pmu, cpu);
> +	struct perf_event *event, *tmp;
> +
> +	if (!pmu)
> +		return 0;
> +
> +	spin_lock(&rapl_hotplug_lock);
> +
> +	/*
> +	 * stop all syswide RAPL events on that  CPU
> +	 * as a consequence also stops the hrtimer
> +	 */
> +	list_for_each_entry_safe(event, tmp, &pmu->active_list, active_entry) {
> +		rapl_pmu_event_stop(event, PERF_EF_UPDATE);
> +	}
> +
> +	per_cpu(rapl_pmu, cpu) = NULL;
> +
> +	if (atomic_dec_and_test(&pmu->refcnt))
> +		kfree(pmu);
> +
> +	spin_unlock(&rapl_hotplug_lock);
> +	return 0;
> +}

Could you do an add-on patch similar to the below -- no need to respin
the entire series once again for this.

---
commit 22cc4ccf63e10e361531bf61e6e6c96c53a2f665
Author: Yan, Zheng <zheng.z.yan@intel.com>
Date:   Tue Apr 16 19:51:05 2013 +0800

    perf/x86: Avoid kfree() in CPU_{STARTING,DYING}
    
    On -rt kfree() can schedule, but CPU_{STARTING,DYING} should be
    atomic. So use a list to defer kfree until CPU_{ONLINE,DEAD}.
    
    Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
    Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
    Cc: peterz@infradead.org
    Cc: eranian@google.com
    Cc: ak@linux.intel.com
    Link: http://lkml.kernel.org/r/1366113067-3262-2-git-send-email-zheng.z.yan@intel.com
    Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index 75da9e18b128..50d4a1c58106 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -2622,6 +2622,21 @@ static void __init uncore_pci_exit(void)
 	}
 }
 
+/* CPU hot plug/unplug are serialized by cpu_add_remove_lock mutex */
+static LIST_HEAD(boxes_to_free);
+
+static void __cpuinit uncore_kfree_boxes(void)
+{
+	struct intel_uncore_box *box;
+
+	while (!list_empty(&boxes_to_free)) {
+		box = list_entry(boxes_to_free.next,
+				 struct intel_uncore_box, list);
+		list_del(&box->list);
+		kfree(box);
+	}
+}
+
 static void __cpuinit uncore_cpu_dying(int cpu)
 {
 	struct intel_uncore_type *type;
@@ -2636,7 +2651,7 @@ static void __cpuinit uncore_cpu_dying(int cpu)
 			box = *per_cpu_ptr(pmu->box, cpu);
 			*per_cpu_ptr(pmu->box, cpu) = NULL;
 			if (box && atomic_dec_and_test(&box->refcnt))
-				kfree(box);
+				list_add(&box->list, &boxes_to_free);
 		}
 	}
 }
@@ -2666,8 +2681,11 @@ static int __cpuinit uncore_cpu_starting(int cpu)
 				if (exist && exist->phys_id == phys_id) {
 					atomic_inc(&exist->refcnt);
 					*per_cpu_ptr(pmu->box, cpu) = exist;
-					kfree(box);
-					box = NULL;
+					if (box) {
+						list_add(&box->list,
+							 &boxes_to_free);
+						box = NULL;
+					}
 					break;
 				}
 			}
@@ -2806,6 +2824,10 @@ static int
 	case CPU_DYING:
 		uncore_cpu_dying(cpu);
 		break;
+	case CPU_ONLINE:
+	case CPU_DEAD:
+		uncore_kfree_boxes();
+		break;
 	default:
 		break;
 	}

  reply	other threads:[~2013-11-06 10:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-05 17:01 [PATCH v5 0/4] perf/x86: " Stephane Eranian
2013-11-05 17:01 ` [PATCH v5 1/4] perf: add active_entry list head to struct perf_event Stephane Eranian
2013-11-05 17:01 ` [PATCH v5 2/4] perf stat: add event unit and scale support Stephane Eranian
2013-11-05 17:01 ` [PATCH v5 3/4] perf,x86: add Intel RAPL PMU support Stephane Eranian
2013-11-06 10:42   ` Peter Zijlstra [this message]
2013-11-06 10:50   ` Peter Zijlstra
2013-11-06 10:52   ` Peter Zijlstra
2013-11-07 17:24     ` Stephane Eranian
2013-11-05 17:01 ` [PATCH v5 4/4] perf,x86: add RAPL hrtimer support Stephane Eranian

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=20131106104243.GF10651@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maria.n.dimakopoulou@gmail.com \
    --cc=mingo@elte.hu \
    --cc=zheng.z.yan@intel.com \
    /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

all inboxes | Powered by JetHome®