Re: [PATCH 1/1] perf: Add CPU hotplug support for events
On Fri, Feb 16, 2018 at 05:48:21PM -0800, Raghavendra Rao Ananta wrote: > I am sure we can fix it, but apart > from the "why we are doing hotplug?" question, was was there specifically > any issue with our patch? Yes, the extra list is crazy. We don't keep events in extra lists when a task isn't currently running either. A CPU being offline shouldn't be (much) different from that. Thinking more, we should also get rid of that HOTPLUG offset thing. > > > > Also, you _still_ don't explain why you care about dead CPUs. > > > > > I wanted to understand, if we no longer care about hotplugging of CPUs, then > why do we still have exported symbols such as cpu_up() and cpu_down()? Not sure why we have them, legacy probably. The rcu/lock torture module is the only legitimate user of them. But if having those exports gives people the impression its a sane thing to do hotplug from modules, we should just take it out. > Moreover, we also have the hotplug interface exposed to users-space as well > (through sysfs). As long as these interfaces exist, there's always a > potential chance of bringing the CPU up/down. Can you please clear this > thing up for me? Hotplug is an absolute utter slow path. We do our absolute best to put the entire burden on the hotplug path such that we don't perturb normal things. Its primary existence is for physical CPU hotplug, not resource management. Although there seems to be a misguided 'I have this hammer, everthing is a nail' thing going on. I suppose these 'once' things like changing the topology of the machine -- eg. 'unplug' all but one of the SMT threads, are OK as well. And RAS things that take a CPU down when there's 'trouble' is also fine. But anything that does hotplug semi regularly is batshit insane. One of the first things hotplug does is synchronize_rcu(), that can take a _long_ time.
Re: [PATCH 1/1] perf: Add CPU hotplug support for events
On Fri, Feb 16, 2018 at 05:48:21PM -0800, Raghavendra Rao Ananta wrote: > I am sure we can fix it, but apart > from the "why we are doing hotplug?" question, was was there specifically > any issue with our patch? Yes, the extra list is crazy. We don't keep events in extra lists when a task isn't currently running either. A CPU being offline shouldn't be (much) different from that. Thinking more, we should also get rid of that HOTPLUG offset thing. > > > > Also, you _still_ don't explain why you care about dead CPUs. > > > > > I wanted to understand, if we no longer care about hotplugging of CPUs, then > why do we still have exported symbols such as cpu_up() and cpu_down()? Not sure why we have them, legacy probably. The rcu/lock torture module is the only legitimate user of them. But if having those exports gives people the impression its a sane thing to do hotplug from modules, we should just take it out. > Moreover, we also have the hotplug interface exposed to users-space as well > (through sysfs). As long as these interfaces exist, there's always a > potential chance of bringing the CPU up/down. Can you please clear this > thing up for me? Hotplug is an absolute utter slow path. We do our absolute best to put the entire burden on the hotplug path such that we don't perturb normal things. Its primary existence is for physical CPU hotplug, not resource management. Although there seems to be a misguided 'I have this hammer, everthing is a nail' thing going on. I suppose these 'once' things like changing the topology of the machine -- eg. 'unplug' all but one of the SMT threads, are OK as well. And RAS things that take a CPU down when there's 'trouble' is also fine. But anything that does hotplug semi regularly is batshit insane. One of the first things hotplug does is synchronize_rcu(), that can take a _long_ time.
Re: [PATCH 1/1] perf: Add CPU hotplug support for events
On 02/16/2018 12:39 PM, Peter Zijlstra wrote: On Fri, Feb 16, 2018 at 10:06:29AM -0800, Raghavendra Rao Ananta wrote: No this is absolutely disguisting. You can simply keep the events in the dead CPU's context. It's really not that hard. Keeping the events in the dead CPU's context was also an idea that we had. However, detaching that event from the PMU when the CPU is offline would be a pain. Consider the scenario in which an event is about to be destroyed when the CPU is offline (yet still attached to the CPU). During it's destruction, a cross-cpu call is made (from perf_remove_from_context()) to the offlined CPU to detach the event from the CPU's PMU. As the CPU is offline, that would not be possible, and again a separate logic has to be written for cleaning up the events whose CPUs are offlined. That is actually really simple to deal with. The real problems are with semantics, is an event enabled when the CPU is dead? Can you disable/enable an event on a dead CPU. The below patch (_completely_ untested) should do most of it, but needs help with the details. I suspect we want to allow enable/disable on events that are on a dead CPU, and equally I think we want to account the time an enabled event spends on a dead CPU to go towards the 'enabled' bucket. I've gone through your diff, and it gave me a hint of similar texture what we are trying to do (except for maintaining an offline event list). Nevertheless, I tried to test your patch. I created an hw event, and tried to offline the CPU in parallel, and I immediately hit a watchdog soft lockup bug! Tried the same this by first switching off the CPU (without any event created), and I hit into similar issue. I am sure we can fix it, but apart from the "why we are doing hotplug?" question, was was there specifically any issue with our patch? Also, you _still_ don't explain why you care about dead CPUs. I wanted to understand, if we no longer care about hotplugging of CPUs, then why do we still have exported symbols such as cpu_up() and cpu_down()? Moreover, we also have the hotplug interface exposed to users-space as well (through sysfs). As long as these interfaces exist, there's always a potential chance of bringing the CPU up/down. Can you please clear this thing up for me? -- Raghavendra -- Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 1/1] perf: Add CPU hotplug support for events
On 02/16/2018 12:39 PM, Peter Zijlstra wrote: On Fri, Feb 16, 2018 at 10:06:29AM -0800, Raghavendra Rao Ananta wrote: No this is absolutely disguisting. You can simply keep the events in the dead CPU's context. It's really not that hard. Keeping the events in the dead CPU's context was also an idea that we had. However, detaching that event from the PMU when the CPU is offline would be a pain. Consider the scenario in which an event is about to be destroyed when the CPU is offline (yet still attached to the CPU). During it's destruction, a cross-cpu call is made (from perf_remove_from_context()) to the offlined CPU to detach the event from the CPU's PMU. As the CPU is offline, that would not be possible, and again a separate logic has to be written for cleaning up the events whose CPUs are offlined. That is actually really simple to deal with. The real problems are with semantics, is an event enabled when the CPU is dead? Can you disable/enable an event on a dead CPU. The below patch (_completely_ untested) should do most of it, but needs help with the details. I suspect we want to allow enable/disable on events that are on a dead CPU, and equally I think we want to account the time an enabled event spends on a dead CPU to go towards the 'enabled' bucket. I've gone through your diff, and it gave me a hint of similar texture what we are trying to do (except for maintaining an offline event list). Nevertheless, I tried to test your patch. I created an hw event, and tried to offline the CPU in parallel, and I immediately hit a watchdog soft lockup bug! Tried the same this by first switching off the CPU (without any event created), and I hit into similar issue. I am sure we can fix it, but apart from the "why we are doing hotplug?" question, was was there specifically any issue with our patch? Also, you _still_ don't explain why you care about dead CPUs. I wanted to understand, if we no longer care about hotplugging of CPUs, then why do we still have exported symbols such as cpu_up() and cpu_down()? Moreover, we also have the hotplug interface exposed to users-space as well (through sysfs). As long as these interfaces exist, there's always a potential chance of bringing the CPU up/down. Can you please clear this thing up for me? -- Raghavendra -- Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 1/1] perf: Add CPU hotplug support for events
On Fri, Feb 16, 2018 at 10:06:29AM -0800, Raghavendra Rao Ananta wrote: > > No this is absolutely disguisting. You can simply keep the events in the > > dead CPU's context. It's really not that hard. > Keeping the events in the dead CPU's context was also an idea that we had. > However, detaching that event from the PMU when the CPU is offline would be > a pain. Consider the scenario in which an event is about to be destroyed > when the CPU is offline (yet still attached to the CPU). During it's > destruction, a cross-cpu call is made (from perf_remove_from_context()) to > the offlined CPU to detach the event from the CPU's PMU. As the CPU is > offline, that would not be possible, and again a separate logic has to be > written for cleaning up the events whose CPUs are offlined. That is actually really simple to deal with. The real problems are with semantics, is an event enabled when the CPU is dead? Can you disable/enable an event on a dead CPU. The below patch (_completely_ untested) should do most of it, but needs help with the details. I suspect we want to allow enable/disable on events that are on a dead CPU, and equally I think we want to account the time an enabled event spends on a dead CPU to go towards the 'enabled' bucket. > > Also, you _still_ don't explain why you care about dead CPUs. > > > > It's just not only about dead CPUs. It's the fact that the CPUs can come and > go online. The embedded world, specifically Android mobile SoCs, rely on CPU > hotplugs to manage power and thermal constraints. These hotplugs can happen > at a very rapid pace. That's batshit insane... and that sounds like the primary thing you should be fixing. --- include/linux/perf_event.h | 2 + kernel/events/core.c | 109 - 2 files changed, 69 insertions(+), 42 deletions(-) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 7546822a1d74..c7a50fe26672 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -495,6 +495,8 @@ enum perf_event_state { PERF_EVENT_STATE_OFF= -1, PERF_EVENT_STATE_INACTIVE = 0, PERF_EVENT_STATE_ACTIVE = 1, + + PERF_EVENT_STATE_HOTPLUG_OFFSET = -32, }; struct file; diff --git a/kernel/events/core.c b/kernel/events/core.c index 57898102847f..26dae8afe57d 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -265,17 +265,17 @@ static void event_function_call(struct perf_event *event, event_f func, void *da lockdep_assert_held(>mutex); } - if (!task) { - cpu_function_call(event->cpu, event_function, ); - return; - } - if (task == TASK_TOMBSTONE) return; again: - if (!task_function_call(task, event_function, )) - return; + if (task) { + if (!task_function_call(task, event_function, )) + return; + } else { + if (!cpu_function_call(event->cpu, event_function, )) + return; + } raw_spin_lock_irq(>lock); /* @@ -2110,7 +2110,7 @@ group_sched_in(struct perf_event *group_event, struct perf_event *event, *partial_group = NULL; struct pmu *pmu = ctx->pmu; - if (group_event->state == PERF_EVENT_STATE_OFF) + if (group_event->state <= PERF_EVENT_STATE_OFF) return 0; pmu->start_txn(pmu, PERF_PMU_TXN_ADD); @@ -2189,6 +2189,14 @@ static int group_can_go_on(struct perf_event *event, static void add_event_to_ctx(struct perf_event *event, struct perf_event_context *ctx) { + if (!ctx->task) { + struct perf_cpu_context *cpuctx = + container_of(ctx, struct perf_cpu_context, ctx); + + if (!cpuctx->online) + event->state += PERF_EVENT_STATE_HOTPLUG_OFFSET; + } + list_add_event(event, ctx); perf_group_attach(event); } @@ -2352,11 +2360,6 @@ perf_install_in_context(struct perf_event_context *ctx, */ smp_store_release(>ctx, ctx); - if (!task) { - cpu_function_call(cpu, __perf_install_in_context, event); - return; - } - /* * Should not happen, we validate the ctx is still alive before calling. */ @@ -2395,8 +2398,14 @@ perf_install_in_context(struct perf_event_context *ctx, */ smp_mb(); again: - if (!task_function_call(task, __perf_install_in_context, event)) - return; + if (task) { + if (!task_function_call(task, __perf_install_in_context, event)) + return; + } else { + if (!cpu_function_call(cpu, __perf_install_in_context, event)) + return; + } + raw_spin_lock_irq(>lock); task = ctx->task; @@ -10280,16 +10289,7 @@
Re: [PATCH 1/1] perf: Add CPU hotplug support for events
On Fri, Feb 16, 2018 at 10:06:29AM -0800, Raghavendra Rao Ananta wrote: > > No this is absolutely disguisting. You can simply keep the events in the > > dead CPU's context. It's really not that hard. > Keeping the events in the dead CPU's context was also an idea that we had. > However, detaching that event from the PMU when the CPU is offline would be > a pain. Consider the scenario in which an event is about to be destroyed > when the CPU is offline (yet still attached to the CPU). During it's > destruction, a cross-cpu call is made (from perf_remove_from_context()) to > the offlined CPU to detach the event from the CPU's PMU. As the CPU is > offline, that would not be possible, and again a separate logic has to be > written for cleaning up the events whose CPUs are offlined. That is actually really simple to deal with. The real problems are with semantics, is an event enabled when the CPU is dead? Can you disable/enable an event on a dead CPU. The below patch (_completely_ untested) should do most of it, but needs help with the details. I suspect we want to allow enable/disable on events that are on a dead CPU, and equally I think we want to account the time an enabled event spends on a dead CPU to go towards the 'enabled' bucket. > > Also, you _still_ don't explain why you care about dead CPUs. > > > > It's just not only about dead CPUs. It's the fact that the CPUs can come and > go online. The embedded world, specifically Android mobile SoCs, rely on CPU > hotplugs to manage power and thermal constraints. These hotplugs can happen > at a very rapid pace. That's batshit insane... and that sounds like the primary thing you should be fixing. --- include/linux/perf_event.h | 2 + kernel/events/core.c | 109 - 2 files changed, 69 insertions(+), 42 deletions(-) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 7546822a1d74..c7a50fe26672 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -495,6 +495,8 @@ enum perf_event_state { PERF_EVENT_STATE_OFF= -1, PERF_EVENT_STATE_INACTIVE = 0, PERF_EVENT_STATE_ACTIVE = 1, + + PERF_EVENT_STATE_HOTPLUG_OFFSET = -32, }; struct file; diff --git a/kernel/events/core.c b/kernel/events/core.c index 57898102847f..26dae8afe57d 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -265,17 +265,17 @@ static void event_function_call(struct perf_event *event, event_f func, void *da lockdep_assert_held(>mutex); } - if (!task) { - cpu_function_call(event->cpu, event_function, ); - return; - } - if (task == TASK_TOMBSTONE) return; again: - if (!task_function_call(task, event_function, )) - return; + if (task) { + if (!task_function_call(task, event_function, )) + return; + } else { + if (!cpu_function_call(event->cpu, event_function, )) + return; + } raw_spin_lock_irq(>lock); /* @@ -2110,7 +2110,7 @@ group_sched_in(struct perf_event *group_event, struct perf_event *event, *partial_group = NULL; struct pmu *pmu = ctx->pmu; - if (group_event->state == PERF_EVENT_STATE_OFF) + if (group_event->state <= PERF_EVENT_STATE_OFF) return 0; pmu->start_txn(pmu, PERF_PMU_TXN_ADD); @@ -2189,6 +2189,14 @@ static int group_can_go_on(struct perf_event *event, static void add_event_to_ctx(struct perf_event *event, struct perf_event_context *ctx) { + if (!ctx->task) { + struct perf_cpu_context *cpuctx = + container_of(ctx, struct perf_cpu_context, ctx); + + if (!cpuctx->online) + event->state += PERF_EVENT_STATE_HOTPLUG_OFFSET; + } + list_add_event(event, ctx); perf_group_attach(event); } @@ -2352,11 +2360,6 @@ perf_install_in_context(struct perf_event_context *ctx, */ smp_store_release(>ctx, ctx); - if (!task) { - cpu_function_call(cpu, __perf_install_in_context, event); - return; - } - /* * Should not happen, we validate the ctx is still alive before calling. */ @@ -2395,8 +2398,14 @@ perf_install_in_context(struct perf_event_context *ctx, */ smp_mb(); again: - if (!task_function_call(task, __perf_install_in_context, event)) - return; + if (task) { + if (!task_function_call(task, __perf_install_in_context, event)) + return; + } else { + if (!cpu_function_call(cpu, __perf_install_in_context, event)) + return; + } + raw_spin_lock_irq(>lock); task = ctx->task; @@ -10280,16 +10289,7 @@
Re: [PATCH 1/1] perf: Add CPU hotplug support for events
On 02/16/2018 12:21 AM, Peter Zijlstra wrote: On Thu, Feb 15, 2018 at 03:01:41PM -0800, Raghavendra Rao Ananta wrote: Perf framework doesn't allow prevserving CPU events across CPU hotplugs. The events are scheduled out as and when the CPU walks offline. Moreover, the framework also doesn't allow the clients to create events on an offline CPU. As a result, the clients have to keep on monitoring the CPU state until it comes back online. Therefore, introducing the perf framework to support creation and preserving of (CPU) events for offline CPUs. Through this, the CPU's online state would be transparent to the client and it not have to worry about monitoring the CPU's state. Success would be returned to the client even while creating the event on an offline CPU. If during the lifetime of the event the CPU walks offline, the event would be preserved and would continue to count as soon as (and if) the CPU comes back online. Signed-off-by: Raghavendra Rao Ananta--- include/linux/perf_event.h | 7 +++ kernel/events/core.c | 123 + 2 files changed, 97 insertions(+), 33 deletions(-) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 7546822..bc07f16 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -489,6 +489,7 @@ struct perf_addr_filters_head { * enum perf_event_state - the states of a event */ enum perf_event_state { + PERF_EVENT_STATE_DORMANT= -5, PERF_EVENT_STATE_DEAD = -4, PERF_EVENT_STATE_EXIT = -3, PERF_EVENT_STATE_ERROR = -2, @@ -687,6 +688,12 @@ struct perf_event { #endif struct list_head sb_list; + + /* Entry into the list that holds the events whose CPUs +* are offline. These events will be removed from the +* list and installed once the CPU wakes up. +*/ + struct list_headdormant_entry; No this is absolutely disguisting. You can simply keep the events in the dead CPU's context. It's really not that hard. Keeping the events in the dead CPU's context was also an idea that we had. However, detaching that event from the PMU when the CPU is offline would be a pain. Consider the scenario in which an event is about to be destroyed when the CPU is offline (yet still attached to the CPU). During it's destruction, a cross-cpu call is made (from perf_remove_from_context()) to the offlined CPU to detach the event from the CPU's PMU. As the CPU is offline, that would not be possible, and again a separate logic has to be written for cleaning up the events whose CPUs are offlined. Hence, I thought it would be a cleaner way to maintain the events. Also, you _still_ don't explain why you care about dead CPUs. It's just not only about dead CPUs. It's the fact that the CPUs can come and go online. The embedded world, specifically Android mobile SoCs, rely on CPU hotplugs to manage power and thermal constraints. These hotplugs can happen at a very rapid pace. Adjacently, they also rely on many perf event counters for its management. Therefore, there is a need to preserve these events across hotplugs. In such a scenario, a perf client (kernel or user-space) can create events even when the CPU is offline. If the CPU comes online during the lifetime of the event, the registered event can start counting spontaneously. As an extension to this, the events' count can also be preserved across CPU hotplugs. This takes the burden off of the clients to monitor the state of the CPU. -- Raghavendra -- Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 1/1] perf: Add CPU hotplug support for events
On 02/16/2018 12:21 AM, Peter Zijlstra wrote: On Thu, Feb 15, 2018 at 03:01:41PM -0800, Raghavendra Rao Ananta wrote: Perf framework doesn't allow prevserving CPU events across CPU hotplugs. The events are scheduled out as and when the CPU walks offline. Moreover, the framework also doesn't allow the clients to create events on an offline CPU. As a result, the clients have to keep on monitoring the CPU state until it comes back online. Therefore, introducing the perf framework to support creation and preserving of (CPU) events for offline CPUs. Through this, the CPU's online state would be transparent to the client and it not have to worry about monitoring the CPU's state. Success would be returned to the client even while creating the event on an offline CPU. If during the lifetime of the event the CPU walks offline, the event would be preserved and would continue to count as soon as (and if) the CPU comes back online. Signed-off-by: Raghavendra Rao Ananta --- include/linux/perf_event.h | 7 +++ kernel/events/core.c | 123 + 2 files changed, 97 insertions(+), 33 deletions(-) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 7546822..bc07f16 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -489,6 +489,7 @@ struct perf_addr_filters_head { * enum perf_event_state - the states of a event */ enum perf_event_state { + PERF_EVENT_STATE_DORMANT= -5, PERF_EVENT_STATE_DEAD = -4, PERF_EVENT_STATE_EXIT = -3, PERF_EVENT_STATE_ERROR = -2, @@ -687,6 +688,12 @@ struct perf_event { #endif struct list_head sb_list; + + /* Entry into the list that holds the events whose CPUs +* are offline. These events will be removed from the +* list and installed once the CPU wakes up. +*/ + struct list_headdormant_entry; No this is absolutely disguisting. You can simply keep the events in the dead CPU's context. It's really not that hard. Keeping the events in the dead CPU's context was also an idea that we had. However, detaching that event from the PMU when the CPU is offline would be a pain. Consider the scenario in which an event is about to be destroyed when the CPU is offline (yet still attached to the CPU). During it's destruction, a cross-cpu call is made (from perf_remove_from_context()) to the offlined CPU to detach the event from the CPU's PMU. As the CPU is offline, that would not be possible, and again a separate logic has to be written for cleaning up the events whose CPUs are offlined. Hence, I thought it would be a cleaner way to maintain the events. Also, you _still_ don't explain why you care about dead CPUs. It's just not only about dead CPUs. It's the fact that the CPUs can come and go online. The embedded world, specifically Android mobile SoCs, rely on CPU hotplugs to manage power and thermal constraints. These hotplugs can happen at a very rapid pace. Adjacently, they also rely on many perf event counters for its management. Therefore, there is a need to preserve these events across hotplugs. In such a scenario, a perf client (kernel or user-space) can create events even when the CPU is offline. If the CPU comes online during the lifetime of the event, the registered event can start counting spontaneously. As an extension to this, the events' count can also be preserved across CPU hotplugs. This takes the burden off of the clients to monitor the state of the CPU. -- Raghavendra -- Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 1/1] perf: Add CPU hotplug support for events
On Thu, Feb 15, 2018 at 03:01:41PM -0800, Raghavendra Rao Ananta wrote: > Perf framework doesn't allow prevserving CPU events across > CPU hotplugs. The events are scheduled out as and when the > CPU walks offline. Moreover, the framework also doesn't > allow the clients to create events on an offline CPU. As > a result, the clients have to keep on monitoring the CPU > state until it comes back online. > > Therefore, introducing the perf framework to support creation > and preserving of (CPU) events for offline CPUs. Through > this, the CPU's online state would be transparent to the > client and it not have to worry about monitoring the CPU's > state. Success would be returned to the client even while > creating the event on an offline CPU. If during the lifetime > of the event the CPU walks offline, the event would be > preserved and would continue to count as soon as (and if) the > CPU comes back online. > > Signed-off-by: Raghavendra Rao Ananta> --- > include/linux/perf_event.h | 7 +++ > kernel/events/core.c | 123 > + > 2 files changed, 97 insertions(+), 33 deletions(-) > > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 7546822..bc07f16 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -489,6 +489,7 @@ struct perf_addr_filters_head { > * enum perf_event_state - the states of a event > */ > enum perf_event_state { > + PERF_EVENT_STATE_DORMANT= -5, > PERF_EVENT_STATE_DEAD = -4, > PERF_EVENT_STATE_EXIT = -3, > PERF_EVENT_STATE_ERROR = -2, > @@ -687,6 +688,12 @@ struct perf_event { > #endif > > struct list_headsb_list; > + > + /* Entry into the list that holds the events whose CPUs > + * are offline. These events will be removed from the > + * list and installed once the CPU wakes up. > + */ > + struct list_headdormant_entry; No this is absolutely disguisting. You can simply keep the events in the dead CPU's context. It's really not that hard. Also, you _still_ don't explain why you care about dead CPUs.
Re: [PATCH 1/1] perf: Add CPU hotplug support for events
On Thu, Feb 15, 2018 at 03:01:41PM -0800, Raghavendra Rao Ananta wrote: > Perf framework doesn't allow prevserving CPU events across > CPU hotplugs. The events are scheduled out as and when the > CPU walks offline. Moreover, the framework also doesn't > allow the clients to create events on an offline CPU. As > a result, the clients have to keep on monitoring the CPU > state until it comes back online. > > Therefore, introducing the perf framework to support creation > and preserving of (CPU) events for offline CPUs. Through > this, the CPU's online state would be transparent to the > client and it not have to worry about monitoring the CPU's > state. Success would be returned to the client even while > creating the event on an offline CPU. If during the lifetime > of the event the CPU walks offline, the event would be > preserved and would continue to count as soon as (and if) the > CPU comes back online. > > Signed-off-by: Raghavendra Rao Ananta > --- > include/linux/perf_event.h | 7 +++ > kernel/events/core.c | 123 > + > 2 files changed, 97 insertions(+), 33 deletions(-) > > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 7546822..bc07f16 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -489,6 +489,7 @@ struct perf_addr_filters_head { > * enum perf_event_state - the states of a event > */ > enum perf_event_state { > + PERF_EVENT_STATE_DORMANT= -5, > PERF_EVENT_STATE_DEAD = -4, > PERF_EVENT_STATE_EXIT = -3, > PERF_EVENT_STATE_ERROR = -2, > @@ -687,6 +688,12 @@ struct perf_event { > #endif > > struct list_headsb_list; > + > + /* Entry into the list that holds the events whose CPUs > + * are offline. These events will be removed from the > + * list and installed once the CPU wakes up. > + */ > + struct list_headdormant_entry; No this is absolutely disguisting. You can simply keep the events in the dead CPU's context. It's really not that hard. Also, you _still_ don't explain why you care about dead CPUs.
[PATCH 1/1] perf: Add CPU hotplug support for events
Perf framework doesn't allow prevserving CPU events across CPU hotplugs. The events are scheduled out as and when the CPU walks offline. Moreover, the framework also doesn't allow the clients to create events on an offline CPU. As a result, the clients have to keep on monitoring the CPU state until it comes back online. Therefore, introducing the perf framework to support creation and preserving of (CPU) events for offline CPUs. Through this, the CPU's online state would be transparent to the client and it not have to worry about monitoring the CPU's state. Success would be returned to the client even while creating the event on an offline CPU. If during the lifetime of the event the CPU walks offline, the event would be preserved and would continue to count as soon as (and if) the CPU comes back online. Signed-off-by: Raghavendra Rao Ananta--- include/linux/perf_event.h | 7 +++ kernel/events/core.c | 123 + 2 files changed, 97 insertions(+), 33 deletions(-) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 7546822..bc07f16 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -489,6 +489,7 @@ struct perf_addr_filters_head { * enum perf_event_state - the states of a event */ enum perf_event_state { + PERF_EVENT_STATE_DORMANT= -5, PERF_EVENT_STATE_DEAD = -4, PERF_EVENT_STATE_EXIT = -3, PERF_EVENT_STATE_ERROR = -2, @@ -687,6 +688,12 @@ struct perf_event { #endif struct list_headsb_list; + + /* Entry into the list that holds the events whose CPUs +* are offline. These events will be removed from the +* list and installed once the CPU wakes up. +*/ + struct list_headdormant_entry; #endif /* CONFIG_PERF_EVENTS */ }; diff --git a/kernel/events/core.c b/kernel/events/core.c index 96db9ae..5d0a155 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -2329,6 +2329,30 @@ static int __perf_install_in_context(void *info) return ret; } +#if defined CONFIG_HOTPLUG_CPU || defined CONFIG_KEXEC_CORE +static DEFINE_PER_CPU(struct list_head, dormant_event_list); +static DEFINE_PER_CPU(spinlock_t, dormant_event_list_lock); + +static void perf_prepare_install_in_context(struct perf_event *event) +{ + int cpu = event->cpu; + bool prepare_hp_sched = !READ_ONCE(event->ctx->task); + + if (!prepare_hp_sched) + return; + + spin_lock(_cpu(dormant_event_list_lock, cpu)); + if (event->state == PERF_EVENT_STATE_DORMANT) + goto out; + + event->state = PERF_EVENT_STATE_DORMANT; + list_add_tail(>dormant_entry, + _cpu(dormant_event_list, cpu)); +out: + spin_unlock(_cpu(dormant_event_list_lock, cpu)); +} +#endif + /* * Attach a performance event to a context. * @@ -2353,6 +2377,15 @@ static int __perf_install_in_context(void *info) smp_store_release(>ctx, ctx); if (!task) { + struct perf_cpu_context *cpuctx = + container_of(ctx, struct perf_cpu_context, ctx); + +#if defined CONFIG_HOTPLUG_CPU || defined CONFIG_KEXEC_CORE + if (!cpuctx->online) { + perf_prepare_install_in_context(event); + return; + } +#endif cpu_function_call(cpu, __perf_install_in_context, event); return; } @@ -2421,6 +2454,43 @@ static int __perf_install_in_context(void *info) raw_spin_unlock_irq(>lock); } +#if defined CONFIG_HOTPLUG_CPU || defined CONFIG_KEXEC_CORE +static void perf_deferred_install_in_context(int cpu) +{ + struct perf_event *event, *tmp; + struct perf_event_context *ctx; + + /* This function is called twice while coming online. Once for +* CPUHP_PERF_PREPARE and the other for CPUHP_AP_PERF_ONLINE. +* Only during the CPUHP_AP_PERF_ONLINE state, we can confirm +* that CPU PMU is ready and can be installed to. +*/ + if (!cpu_online(cpu)) + return; + + spin_lock(_cpu(dormant_event_list_lock, cpu)); + list_for_each_entry_safe(event, tmp, + _cpu(dormant_event_list, cpu), dormant_entry) { + if (cpu != event->cpu) + continue; + + list_del(>dormant_entry); + event->state = PERF_EVENT_STATE_INACTIVE; + spin_unlock(_cpu(dormant_event_list_lock, cpu)); + + ctx = event->ctx; + perf_event_set_state(event, PERF_EVENT_STATE_INACTIVE); + + mutex_lock(>mutex); + perf_install_in_context(ctx, event, cpu); + mutex_unlock(>mutex); + + spin_lock(_cpu(dormant_event_list_lock, cpu)); + } +
[PATCH 1/1] perf: Add CPU hotplug support for events
Perf framework doesn't allow prevserving CPU events across CPU hotplugs. The events are scheduled out as and when the CPU walks offline. Moreover, the framework also doesn't allow the clients to create events on an offline CPU. As a result, the clients have to keep on monitoring the CPU state until it comes back online. Therefore, introducing the perf framework to support creation and preserving of (CPU) events for offline CPUs. Through this, the CPU's online state would be transparent to the client and it not have to worry about monitoring the CPU's state. Success would be returned to the client even while creating the event on an offline CPU. If during the lifetime of the event the CPU walks offline, the event would be preserved and would continue to count as soon as (and if) the CPU comes back online. Signed-off-by: Raghavendra Rao Ananta --- include/linux/perf_event.h | 7 +++ kernel/events/core.c | 123 + 2 files changed, 97 insertions(+), 33 deletions(-) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 7546822..bc07f16 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -489,6 +489,7 @@ struct perf_addr_filters_head { * enum perf_event_state - the states of a event */ enum perf_event_state { + PERF_EVENT_STATE_DORMANT= -5, PERF_EVENT_STATE_DEAD = -4, PERF_EVENT_STATE_EXIT = -3, PERF_EVENT_STATE_ERROR = -2, @@ -687,6 +688,12 @@ struct perf_event { #endif struct list_headsb_list; + + /* Entry into the list that holds the events whose CPUs +* are offline. These events will be removed from the +* list and installed once the CPU wakes up. +*/ + struct list_headdormant_entry; #endif /* CONFIG_PERF_EVENTS */ }; diff --git a/kernel/events/core.c b/kernel/events/core.c index 96db9ae..5d0a155 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -2329,6 +2329,30 @@ static int __perf_install_in_context(void *info) return ret; } +#if defined CONFIG_HOTPLUG_CPU || defined CONFIG_KEXEC_CORE +static DEFINE_PER_CPU(struct list_head, dormant_event_list); +static DEFINE_PER_CPU(spinlock_t, dormant_event_list_lock); + +static void perf_prepare_install_in_context(struct perf_event *event) +{ + int cpu = event->cpu; + bool prepare_hp_sched = !READ_ONCE(event->ctx->task); + + if (!prepare_hp_sched) + return; + + spin_lock(_cpu(dormant_event_list_lock, cpu)); + if (event->state == PERF_EVENT_STATE_DORMANT) + goto out; + + event->state = PERF_EVENT_STATE_DORMANT; + list_add_tail(>dormant_entry, + _cpu(dormant_event_list, cpu)); +out: + spin_unlock(_cpu(dormant_event_list_lock, cpu)); +} +#endif + /* * Attach a performance event to a context. * @@ -2353,6 +2377,15 @@ static int __perf_install_in_context(void *info) smp_store_release(>ctx, ctx); if (!task) { + struct perf_cpu_context *cpuctx = + container_of(ctx, struct perf_cpu_context, ctx); + +#if defined CONFIG_HOTPLUG_CPU || defined CONFIG_KEXEC_CORE + if (!cpuctx->online) { + perf_prepare_install_in_context(event); + return; + } +#endif cpu_function_call(cpu, __perf_install_in_context, event); return; } @@ -2421,6 +2454,43 @@ static int __perf_install_in_context(void *info) raw_spin_unlock_irq(>lock); } +#if defined CONFIG_HOTPLUG_CPU || defined CONFIG_KEXEC_CORE +static void perf_deferred_install_in_context(int cpu) +{ + struct perf_event *event, *tmp; + struct perf_event_context *ctx; + + /* This function is called twice while coming online. Once for +* CPUHP_PERF_PREPARE and the other for CPUHP_AP_PERF_ONLINE. +* Only during the CPUHP_AP_PERF_ONLINE state, we can confirm +* that CPU PMU is ready and can be installed to. +*/ + if (!cpu_online(cpu)) + return; + + spin_lock(_cpu(dormant_event_list_lock, cpu)); + list_for_each_entry_safe(event, tmp, + _cpu(dormant_event_list, cpu), dormant_entry) { + if (cpu != event->cpu) + continue; + + list_del(>dormant_entry); + event->state = PERF_EVENT_STATE_INACTIVE; + spin_unlock(_cpu(dormant_event_list_lock, cpu)); + + ctx = event->ctx; + perf_event_set_state(event, PERF_EVENT_STATE_INACTIVE); + + mutex_lock(>mutex); + perf_install_in_context(ctx, event, cpu); + mutex_unlock(>mutex); + + spin_lock(_cpu(dormant_event_list_lock, cpu)); + } + spin_unlock(_cpu(dormant_event_list_lock, cpu)); +} +#endif +