Re: [PATCH] perf: Add support for creating offline events

[Peter Zijlstra]

On Tue, Feb 13, 2018 at 02:17:07PM -0800, Sodagudi Prasad wrote:

> > This is horrible.. and you seem to have forgotten to explain why you
> > care about offline CPUs.
> 
> Up to 4.9 kernel, drivers can register cpu hotplug notfiters and drivers are
> able to create perf events dynamically based cpu notifies callback events.
> As cpu hot plug is converted to state machine approach from hot plug
> notifiers,  every driver need to define a cpuhp state and registers with cpu
> hotplug state machine for creating perf events dynamically.
> 
> Qualcomm have use cases to monitor the cpu cycles and other hw events
> continuously on all cpus from kernel and profiling tools.
> So we are thinking that there could be other soc vendors, who are interested
> in perf events preserving across cpu hot plug and perf events creation on
> hot plugged cores.

But _why_ are you hotplugging to begin with? Just don't do that.

Re: [PATCH] perf: Add support for creating offline events

[Peter Zijlstra]

On Tue, Feb 13, 2018 at 02:17:07PM -0800, Sodagudi Prasad wrote:

> > This is horrible.. and you seem to have forgotten to explain why you
> > care about offline CPUs.
> 
> Up to 4.9 kernel, drivers can register cpu hotplug notfiters and drivers are
> able to create perf events dynamically based cpu notifies callback events.
> As cpu hot plug is converted to state machine approach from hot plug
> notifiers,  every driver need to define a cpuhp state and registers with cpu
> hotplug state machine for creating perf events dynamically.
> 
> Qualcomm have use cases to monitor the cpu cycles and other hw events
> continuously on all cpus from kernel and profiling tools.
> So we are thinking that there could be other soc vendors, who are interested
> in perf events preserving across cpu hot plug and perf events creation on
> hot plugged cores.

But _why_ are you hotplugging to begin with? Just don't do that.

Re: [PATCH] perf: Add support for creating offline events

[Sodagudi Prasad]

On 2018-02-13 10:23, Peter Zijlstra wrote:

On Fri, Feb 09, 2018 at 03:07:00PM -0800, Raghavendra Rao Ananta wrote:

Perf framework doesn't allow creation of hardware events if
the requested CPU is offline. However, creation of an event
is achievable if the event is attached to the PMU as soon
as the CPU is online again.

So, introducing a feature that could allow to create events
even when the CPU is offline and return a success to the caller.
If, during the time of event creation, the CPU is found offline,
the event is moved to a new state (PERF_EVENT_STATE_DORMANT). As
and when the CPU is know to be woken up (through hotplug notifiers),
all the dormant events would be attached to the PMU (by
perf_install_in_context()). If during the life time of the event,
the CPU hasn't come online, the dormant event would just be freed.


This is horrible.. and you seem to have forgotten to explain why you
care about offline CPUs.

Hi Peter,

Up to 4.9 kernel, drivers can register cpu hotplug notfiters and drivers 
are able to create perf events dynamically based cpu notifies callback 
events.
As cpu hot plug is converted to state machine approach from hot plug 
notifiers,  every driver need to define a cpuhp state and registers with 
cpu hotplug state machine for creating perf events dynamically.


Qualcomm have use cases to monitor the cpu cycles and other hw events 
continuously on all cpus from kernel and profiling tools.
So we are thinking that there could be other soc vendors, who are 
interested in perf events preserving across cpu hot plug and perf events 
creation on hot plugged cores.


-Thanks, Prasad

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,

Linux Foundation Collaborative Project

Re: [PATCH] perf: Add support for creating offline events

[Sodagudi Prasad]

On 2018-02-13 10:23, Peter Zijlstra wrote:

On Fri, Feb 09, 2018 at 03:07:00PM -0800, Raghavendra Rao Ananta wrote:

Perf framework doesn't allow creation of hardware events if
the requested CPU is offline. However, creation of an event
is achievable if the event is attached to the PMU as soon
as the CPU is online again.

So, introducing a feature that could allow to create events
even when the CPU is offline and return a success to the caller.
If, during the time of event creation, the CPU is found offline,
the event is moved to a new state (PERF_EVENT_STATE_DORMANT). As
and when the CPU is know to be woken up (through hotplug notifiers),
all the dormant events would be attached to the PMU (by
perf_install_in_context()). If during the life time of the event,
the CPU hasn't come online, the dormant event would just be freed.


This is horrible.. and you seem to have forgotten to explain why you
care about offline CPUs.

Hi Peter,

Up to 4.9 kernel, drivers can register cpu hotplug notfiters and drivers 
are able to create perf events dynamically based cpu notifies callback 
events.
As cpu hot plug is converted to state machine approach from hot plug 
notifiers,  every driver need to define a cpuhp state and registers with 
cpu hotplug state machine for creating perf events dynamically.


Qualcomm have use cases to monitor the cpu cycles and other hw events 
continuously on all cpus from kernel and profiling tools.
So we are thinking that there could be other soc vendors, who are 
interested in perf events preserving across cpu hot plug and perf events 
creation on hot plugged cores.


-Thanks, Prasad

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,

Linux Foundation Collaborative Project

Re: [PATCH] perf: Add support for creating offline events

[Peter Zijlstra]

On Fri, Feb 09, 2018 at 03:07:00PM -0800, Raghavendra Rao Ananta wrote:
> Perf framework doesn't allow creation of hardware events if
> the requested CPU is offline. However, creation of an event
> is achievable if the event is attached to the PMU as soon
> as the CPU is online again.
> 
> So, introducing a feature that could allow to create events
> even when the CPU is offline and return a success to the caller.
> If, during the time of event creation, the CPU is found offline,
> the event is moved to a new state (PERF_EVENT_STATE_DORMANT). As
> and when the CPU is know to be woken up (through hotplug notifiers),
> all the dormant events would be attached to the PMU (by
> perf_install_in_context()). If during the life time of the event,
> the CPU hasn't come online, the dormant event would just be freed.

This is horrible.. and you seem to have forgotten to explain why you
care about offline CPUs.

Re: [PATCH] perf: Add support for creating offline events

[Peter Zijlstra]

On Fri, Feb 09, 2018 at 03:07:00PM -0800, Raghavendra Rao Ananta wrote:
> Perf framework doesn't allow creation of hardware events if
> the requested CPU is offline. However, creation of an event
> is achievable if the event is attached to the PMU as soon
> as the CPU is online again.
> 
> So, introducing a feature that could allow to create events
> even when the CPU is offline and return a success to the caller.
> If, during the time of event creation, the CPU is found offline,
> the event is moved to a new state (PERF_EVENT_STATE_DORMANT). As
> and when the CPU is know to be woken up (through hotplug notifiers),
> all the dormant events would be attached to the PMU (by
> perf_install_in_context()). If during the life time of the event,
> the CPU hasn't come online, the dormant event would just be freed.

This is horrible.. and you seem to have forgotten to explain why you
care about offline CPUs.

Re: [PATCH] perf: Add support for creating offline events

[Raghavendra Rao Ananta]

On 02/13/2018 08:08 AM, Jiri Olsa wrote:

On Mon, Feb 12, 2018 at 02:22:30PM -0800, Raghavendra Rao Ananta wrote:



On 02/12/2018 01:21 PM, Jiri Olsa wrote:

On Mon, Feb 12, 2018 at 10:04:42PM +0100, Jiri Olsa wrote:

On Mon, Feb 12, 2018 at 09:42:05AM -0800, Raghavendra Rao Ananta wrote:

Hi Jiri,

Thank you for the response.

Does perf tool has its own check to see if the CPU was offline during the
lifetime of an event? If so, it might ignore these type of events.


nope, we don't check on that



Initially, I tested the same using perf tool and found similar results.
Then I debugged further and found that the perf core was actually sending
data to the userspace (copy_to_user()) and the corresponding count for the
data. Hence, I tested this further by writing my own userspace application,
and I was able to read the count through this,
even when the CPU was made offline and back online.

Do you think we also have to modify the perf tool accordingly?


hum, I wonder what's wrong.. will check


I think the user space needs to enable the event once the
cpu gets online.. which we dont do and your app does..?

maybe we could add perf_event_attr::enable_on_online ;-)

I'll check what we can do in user space, I guess we can
monitor the cpu state and enable event accordingly

jirka


Yes, probably that's the reason.

In order for an event to get scheduled-in, it expects the event to be at
least in PERF_EVENT_STATE_INACTIVE state. If you notice, in my patch,
when the cpu wakes up, we are initializing the state of the event
(perf_event__state_init()) and then trying to schedule-in. Since the event
was created with a disabled state, it seems that the same this is followed
and the state gets initialized to PERF_EVENT_STATE_OFF. Unfortunately,
events in this state could not be scheduled.

One way for things to get working is, instead of calling
perf_event__state_init() before the event is scheduled-in (when the cpu
wakes up), we can do something like:
perf_event_set_state(event, PERF_EVENT_STATE_INACTIVE);


could you add check in ioctl call that set the inactive state
on the dormant event.. that would start it once the cpu is
online.. as requested


I am a little confused. When you say "check", do you mean a new ioctl 
command?


So the flow (from the user-space perspective) would go something like this?

1. // CPU offline

2. perf_event_open(); // event started as disabled --> added to dormant 
list in the kernel


3. ioctl(SET_INACTIVE); // change the state of the event to inactive

4. // CPU woken up!

5. // schedule the (inactive) event by traversing the dormant list

Is this what you were trying to mention, or am I missing something?

Raghavendra
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] perf: Add support for creating offline events

[Raghavendra Rao Ananta]

On 02/13/2018 08:08 AM, Jiri Olsa wrote:

On Mon, Feb 12, 2018 at 02:22:30PM -0800, Raghavendra Rao Ananta wrote:



On 02/12/2018 01:21 PM, Jiri Olsa wrote:

On Mon, Feb 12, 2018 at 10:04:42PM +0100, Jiri Olsa wrote:

On Mon, Feb 12, 2018 at 09:42:05AM -0800, Raghavendra Rao Ananta wrote:

Hi Jiri,

Thank you for the response.

Does perf tool has its own check to see if the CPU was offline during the
lifetime of an event? If so, it might ignore these type of events.


nope, we don't check on that



Initially, I tested the same using perf tool and found similar results.
Then I debugged further and found that the perf core was actually sending
data to the userspace (copy_to_user()) and the corresponding count for the
data. Hence, I tested this further by writing my own userspace application,
and I was able to read the count through this,
even when the CPU was made offline and back online.

Do you think we also have to modify the perf tool accordingly?


hum, I wonder what's wrong.. will check


I think the user space needs to enable the event once the
cpu gets online.. which we dont do and your app does..?

maybe we could add perf_event_attr::enable_on_online ;-)

I'll check what we can do in user space, I guess we can
monitor the cpu state and enable event accordingly

jirka


Yes, probably that's the reason.

In order for an event to get scheduled-in, it expects the event to be at
least in PERF_EVENT_STATE_INACTIVE state. If you notice, in my patch,
when the cpu wakes up, we are initializing the state of the event
(perf_event__state_init()) and then trying to schedule-in. Since the event
was created with a disabled state, it seems that the same this is followed
and the state gets initialized to PERF_EVENT_STATE_OFF. Unfortunately,
events in this state could not be scheduled.

One way for things to get working is, instead of calling
perf_event__state_init() before the event is scheduled-in (when the cpu
wakes up), we can do something like:
perf_event_set_state(event, PERF_EVENT_STATE_INACTIVE);


could you add check in ioctl call that set the inactive state
on the dormant event.. that would start it once the cpu is
online.. as requested


I am a little confused. When you say "check", do you mean a new ioctl 
command?


So the flow (from the user-space perspective) would go something like this?

1. // CPU offline

2. perf_event_open(); // event started as disabled --> added to dormant 
list in the kernel


3. ioctl(SET_INACTIVE); // change the state of the event to inactive

4. // CPU woken up!

5. // schedule the (inactive) event by traversing the dormant list

Is this what you were trying to mention, or am I missing something?

Raghavendra
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] perf: Add support for creating offline events

[Jiri Olsa]

On Mon, Feb 12, 2018 at 02:22:30PM -0800, Raghavendra Rao Ananta wrote:
> 
> 
> On 02/12/2018 01:21 PM, Jiri Olsa wrote:
> > On Mon, Feb 12, 2018 at 10:04:42PM +0100, Jiri Olsa wrote:
> > > On Mon, Feb 12, 2018 at 09:42:05AM -0800, Raghavendra Rao Ananta wrote:
> > > > Hi Jiri,
> > > > 
> > > > Thank you for the response.
> > > > 
> > > > Does perf tool has its own check to see if the CPU was offline during 
> > > > the
> > > > lifetime of an event? If so, it might ignore these type of events.
> > > 
> > > nope, we don't check on that
> > > 
> > > > 
> > > > Initially, I tested the same using perf tool and found similar results.
> > > > Then I debugged further and found that the perf core was actually 
> > > > sending
> > > > data to the userspace (copy_to_user()) and the corresponding count for 
> > > > the
> > > > data. Hence, I tested this further by writing my own userspace 
> > > > application,
> > > > and I was able to read the count through this,
> > > > even when the CPU was made offline and back online.
> > > > 
> > > > Do you think we also have to modify the perf tool accordingly?
> > > 
> > > hum, I wonder what's wrong.. will check
> > 
> > I think the user space needs to enable the event once the
> > cpu gets online.. which we dont do and your app does..?
> > 
> > maybe we could add perf_event_attr::enable_on_online ;-)
> > 
> > I'll check what we can do in user space, I guess we can
> > monitor the cpu state and enable event accordingly
> > 
> > jirka
> > 
> Yes, probably that's the reason.
> 
> In order for an event to get scheduled-in, it expects the event to be at
> least in PERF_EVENT_STATE_INACTIVE state. If you notice, in my patch,
> when the cpu wakes up, we are initializing the state of the event
> (perf_event__state_init()) and then trying to schedule-in. Since the event
> was created with a disabled state, it seems that the same this is followed
> and the state gets initialized to PERF_EVENT_STATE_OFF. Unfortunately,
> events in this state could not be scheduled.
> 
> One way for things to get working is, instead of calling
> perf_event__state_init() before the event is scheduled-in (when the cpu
> wakes up), we can do something like:
> perf_event_set_state(event, PERF_EVENT_STATE_INACTIVE);

could you add check in ioctl call that set the inactive state
on the dormant event.. that would start it once the cpu is
online.. as requested

jirka

Re: [PATCH] perf: Add support for creating offline events

[Jiri Olsa]

On Mon, Feb 12, 2018 at 02:22:30PM -0800, Raghavendra Rao Ananta wrote:
> 
> 
> On 02/12/2018 01:21 PM, Jiri Olsa wrote:
> > On Mon, Feb 12, 2018 at 10:04:42PM +0100, Jiri Olsa wrote:
> > > On Mon, Feb 12, 2018 at 09:42:05AM -0800, Raghavendra Rao Ananta wrote:
> > > > Hi Jiri,
> > > > 
> > > > Thank you for the response.
> > > > 
> > > > Does perf tool has its own check to see if the CPU was offline during 
> > > > the
> > > > lifetime of an event? If so, it might ignore these type of events.
> > > 
> > > nope, we don't check on that
> > > 
> > > > 
> > > > Initially, I tested the same using perf tool and found similar results.
> > > > Then I debugged further and found that the perf core was actually 
> > > > sending
> > > > data to the userspace (copy_to_user()) and the corresponding count for 
> > > > the
> > > > data. Hence, I tested this further by writing my own userspace 
> > > > application,
> > > > and I was able to read the count through this,
> > > > even when the CPU was made offline and back online.
> > > > 
> > > > Do you think we also have to modify the perf tool accordingly?
> > > 
> > > hum, I wonder what's wrong.. will check
> > 
> > I think the user space needs to enable the event once the
> > cpu gets online.. which we dont do and your app does..?
> > 
> > maybe we could add perf_event_attr::enable_on_online ;-)
> > 
> > I'll check what we can do in user space, I guess we can
> > monitor the cpu state and enable event accordingly
> > 
> > jirka
> > 
> Yes, probably that's the reason.
> 
> In order for an event to get scheduled-in, it expects the event to be at
> least in PERF_EVENT_STATE_INACTIVE state. If you notice, in my patch,
> when the cpu wakes up, we are initializing the state of the event
> (perf_event__state_init()) and then trying to schedule-in. Since the event
> was created with a disabled state, it seems that the same this is followed
> and the state gets initialized to PERF_EVENT_STATE_OFF. Unfortunately,
> events in this state could not be scheduled.
> 
> One way for things to get working is, instead of calling
> perf_event__state_init() before the event is scheduled-in (when the cpu
> wakes up), we can do something like:
> perf_event_set_state(event, PERF_EVENT_STATE_INACTIVE);

could you add check in ioctl call that set the inactive state
on the dormant event.. that would start it once the cpu is
online.. as requested

jirka

Re: [PATCH] perf: Add support for creating offline events

[Raghavendra Rao Ananta]

On 02/12/2018 01:21 PM, Jiri Olsa wrote:

On Mon, Feb 12, 2018 at 10:04:42PM +0100, Jiri Olsa wrote:

On Mon, Feb 12, 2018 at 09:42:05AM -0800, Raghavendra Rao Ananta wrote:

Hi Jiri,

Thank you for the response.

Does perf tool has its own check to see if the CPU was offline during the
lifetime of an event? If so, it might ignore these type of events.


nope, we don't check on that



Initially, I tested the same using perf tool and found similar results.
Then I debugged further and found that the perf core was actually sending
data to the userspace (copy_to_user()) and the corresponding count for the
data. Hence, I tested this further by writing my own userspace application,
and I was able to read the count through this,
even when the CPU was made offline and back online.

Do you think we also have to modify the perf tool accordingly?


hum, I wonder what's wrong.. will check


I think the user space needs to enable the event once the
cpu gets online.. which we dont do and your app does..?

maybe we could add perf_event_attr::enable_on_online ;-)

I'll check what we can do in user space, I guess we can
monitor the cpu state and enable event accordingly

jirka


Yes, probably that's the reason.

In order for an event to get scheduled-in, it expects the event to be at 
least in PERF_EVENT_STATE_INACTIVE state. If you notice, in my patch,
when the cpu wakes up, we are initializing the state of the event 
(perf_event__state_init()) and then trying to schedule-in. Since the 
event was created with a disabled state, it seems that the same this is 
followed and the state gets initialized to PERF_EVENT_STATE_OFF. 
Unfortunately, events in this state could not be scheduled.


One way for things to get working is, instead of calling 
perf_event__state_init() before the event is scheduled-in (when the cpu 
wakes up), we can do something like:

perf_event_set_state(event, PERF_EVENT_STATE_INACTIVE);

I made this change and ran the same test as yours, and I see things 
working out for us:


# ./perf stat -C 1 -e sched:sched_switch -v -I 1000
failed to read counter sched:sched_switch
#   time counts unit events
 1.000115547sched:sched_switch 


failed to read counter sched:sched_switch
 2.000265492sched:sched_switch 


failed to read counter sched:sched_switch
 3.000379462sched:sched_switch 


failed to read counter sched:sched_switch
 4.000523872sched:sched_switch 


failed to read counter sched:sched_switch
 5.000614808sched:sched_switch

/* CPU bought ONLINE here */

sched:sched_switch: 541 284808940 284808940
 6.000767761541  sched:sched_switch 


sched:sched_switch: 180 1000119686 1000119686
 7.000907234180  sched:sched_switch 


sched:sched_switch: 248 1000129929 1000129929
 8.001026518248  sched:sched_switch 


sched:sched_switch: 253 1000173050 1000173050
 9.001203689253  sched:sched_switch 


sched:sched_switch: 620 1000113378 1000113378
10.001323334620  sched:sched_switch 


sched:sched_switch: 366 1000121839 1000121839
11.001448354366  sched:sched_switch 


sched:sched_switch: 327 1000147664 1000147664
12.001591432327  sched:sched_switch 


^Csched:sched_switch: 272 488810681 488810681
12.490414290272  sched:sched_switch 


sched:sched_switch: 6 75893 75893

Yes, so as you mentioned adding something like 
perf_event_attr::enable_on_online gives us a control as to put the event 
in INACTIVE state.

--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] perf: Add support for creating offline events

[Raghavendra Rao Ananta]

On 02/12/2018 01:21 PM, Jiri Olsa wrote:

On Mon, Feb 12, 2018 at 10:04:42PM +0100, Jiri Olsa wrote:

On Mon, Feb 12, 2018 at 09:42:05AM -0800, Raghavendra Rao Ananta wrote:

Hi Jiri,

Thank you for the response.

Does perf tool has its own check to see if the CPU was offline during the
lifetime of an event? If so, it might ignore these type of events.


nope, we don't check on that



Initially, I tested the same using perf tool and found similar results.
Then I debugged further and found that the perf core was actually sending
data to the userspace (copy_to_user()) and the corresponding count for the
data. Hence, I tested this further by writing my own userspace application,
and I was able to read the count through this,
even when the CPU was made offline and back online.

Do you think we also have to modify the perf tool accordingly?


hum, I wonder what's wrong.. will check


I think the user space needs to enable the event once the
cpu gets online.. which we dont do and your app does..?

maybe we could add perf_event_attr::enable_on_online ;-)

I'll check what we can do in user space, I guess we can
monitor the cpu state and enable event accordingly

jirka


Yes, probably that's the reason.

In order for an event to get scheduled-in, it expects the event to be at 
least in PERF_EVENT_STATE_INACTIVE state. If you notice, in my patch,
when the cpu wakes up, we are initializing the state of the event 
(perf_event__state_init()) and then trying to schedule-in. Since the 
event was created with a disabled state, it seems that the same this is 
followed and the state gets initialized to PERF_EVENT_STATE_OFF. 
Unfortunately, events in this state could not be scheduled.


One way for things to get working is, instead of calling 
perf_event__state_init() before the event is scheduled-in (when the cpu 
wakes up), we can do something like:

perf_event_set_state(event, PERF_EVENT_STATE_INACTIVE);

I made this change and ran the same test as yours, and I see things 
working out for us:


# ./perf stat -C 1 -e sched:sched_switch -v -I 1000
failed to read counter sched:sched_switch
#   time counts unit events
 1.000115547sched:sched_switch 


failed to read counter sched:sched_switch
 2.000265492sched:sched_switch 


failed to read counter sched:sched_switch
 3.000379462sched:sched_switch 


failed to read counter sched:sched_switch
 4.000523872sched:sched_switch 


failed to read counter sched:sched_switch
 5.000614808sched:sched_switch

/* CPU bought ONLINE here */

sched:sched_switch: 541 284808940 284808940
 6.000767761541  sched:sched_switch 


sched:sched_switch: 180 1000119686 1000119686
 7.000907234180  sched:sched_switch 


sched:sched_switch: 248 1000129929 1000129929
 8.001026518248  sched:sched_switch 


sched:sched_switch: 253 1000173050 1000173050
 9.001203689253  sched:sched_switch 


sched:sched_switch: 620 1000113378 1000113378
10.001323334620  sched:sched_switch 


sched:sched_switch: 366 1000121839 1000121839
11.001448354366  sched:sched_switch 


sched:sched_switch: 327 1000147664 1000147664
12.001591432327  sched:sched_switch 


^Csched:sched_switch: 272 488810681 488810681
12.490414290272  sched:sched_switch 


sched:sched_switch: 6 75893 75893

Yes, so as you mentioned adding something like 
perf_event_attr::enable_on_online gives us a control as to put the event 
in INACTIVE state.

--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] perf: Add support for creating offline events

[Jiri Olsa]

On Mon, Feb 12, 2018 at 10:04:42PM +0100, Jiri Olsa wrote:
> On Mon, Feb 12, 2018 at 09:42:05AM -0800, Raghavendra Rao Ananta wrote:
> > Hi Jiri,
> > 
> > Thank you for the response.
> > 
> > Does perf tool has its own check to see if the CPU was offline during the
> > lifetime of an event? If so, it might ignore these type of events.
> 
> nope, we don't check on that
> 
> > 
> > Initially, I tested the same using perf tool and found similar results.
> > Then I debugged further and found that the perf core was actually sending
> > data to the userspace (copy_to_user()) and the corresponding count for the
> > data. Hence, I tested this further by writing my own userspace application,
> > and I was able to read the count through this,
> > even when the CPU was made offline and back online.
> > 
> > Do you think we also have to modify the perf tool accordingly?
> 
> hum, I wonder what's wrong.. will check

I think the user space needs to enable the event once the
cpu gets online.. which we dont do and your app does..?

maybe we could add perf_event_attr::enable_on_online ;-)

I'll check what we can do in user space, I guess we can
monitor the cpu state and enable event accordingly

jirka

Re: [PATCH] perf: Add support for creating offline events

[Jiri Olsa]

On Mon, Feb 12, 2018 at 10:04:42PM +0100, Jiri Olsa wrote:
> On Mon, Feb 12, 2018 at 09:42:05AM -0800, Raghavendra Rao Ananta wrote:
> > Hi Jiri,
> > 
> > Thank you for the response.
> > 
> > Does perf tool has its own check to see if the CPU was offline during the
> > lifetime of an event? If so, it might ignore these type of events.
> 
> nope, we don't check on that
> 
> > 
> > Initially, I tested the same using perf tool and found similar results.
> > Then I debugged further and found that the perf core was actually sending
> > data to the userspace (copy_to_user()) and the corresponding count for the
> > data. Hence, I tested this further by writing my own userspace application,
> > and I was able to read the count through this,
> > even when the CPU was made offline and back online.
> > 
> > Do you think we also have to modify the perf tool accordingly?
> 
> hum, I wonder what's wrong.. will check

I think the user space needs to enable the event once the
cpu gets online.. which we dont do and your app does..?

maybe we could add perf_event_attr::enable_on_online ;-)

I'll check what we can do in user space, I guess we can
monitor the cpu state and enable event accordingly

jirka

Re: [PATCH] perf: Add support for creating offline events

[Jiri Olsa]

On Mon, Feb 12, 2018 at 09:42:05AM -0800, Raghavendra Rao Ananta wrote:
> Hi Jiri,
> 
> Thank you for the response.
> 
> Does perf tool has its own check to see if the CPU was offline during the
> lifetime of an event? If so, it might ignore these type of events.

nope, we don't check on that

> 
> Initially, I tested the same using perf tool and found similar results.
> Then I debugged further and found that the perf core was actually sending
> data to the userspace (copy_to_user()) and the corresponding count for the
> data. Hence, I tested this further by writing my own userspace application,
> and I was able to read the count through this,
> even when the CPU was made offline and back online.
> 
> Do you think we also have to modify the perf tool accordingly?

hum, I wonder what's wrong.. will check

jirka

Re: [PATCH] perf: Add support for creating offline events

[Jiri Olsa]

On Mon, Feb 12, 2018 at 09:42:05AM -0800, Raghavendra Rao Ananta wrote:
> Hi Jiri,
> 
> Thank you for the response.
> 
> Does perf tool has its own check to see if the CPU was offline during the
> lifetime of an event? If so, it might ignore these type of events.

nope, we don't check on that

> 
> Initially, I tested the same using perf tool and found similar results.
> Then I debugged further and found that the perf core was actually sending
> data to the userspace (copy_to_user()) and the corresponding count for the
> data. Hence, I tested this further by writing my own userspace application,
> and I was able to read the count through this,
> even when the CPU was made offline and back online.
> 
> Do you think we also have to modify the perf tool accordingly?

hum, I wonder what's wrong.. will check

jirka

Re: [PATCH] perf: Add support for creating offline events

[kbuild test robot]

Hi Raghavendra,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.15]
[also build test ERROR on next-20180212]
[cannot apply to tip/perf/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Raghavendra-Rao-Ananta/perf-Add-support-for-creating-offline-events/20180213-023250
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel//events/core.c: In function 'perf_event_init_cpu':
>> kernel//events/core.c:11152:2: error: implicit declaration of function 
>> 'perf_deferred_install_in_context'; did you mean 'perf_install_in_context'? 
>> [-Werror=implicit-function-declaration]
 perf_deferred_install_in_context(cpu);
 ^~~~
 perf_install_in_context
   cc1: some warnings being treated as errors

vim +11152 kernel//events/core.c

 11131  
 11132  int perf_event_init_cpu(unsigned int cpu)
 11133  {
 11134  struct perf_cpu_context *cpuctx;
 11135  struct perf_event_context *ctx;
 11136  struct pmu *pmu;
 11137  
 11138  perf_swevent_init_cpu(cpu);
 11139  
 11140  mutex_lock(_lock);
 11141  cpumask_set_cpu(cpu, perf_online_mask);
 11142  list_for_each_entry(pmu, , entry) {
 11143  cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
 11144  ctx = >ctx;
 11145  
 11146  mutex_lock(>mutex);
 11147  cpuctx->online = 1;
 11148  mutex_unlock(>mutex);
 11149  }
 11150  mutex_unlock(_lock);
 11151  
 11152  perf_deferred_install_in_context(cpu);
 11153  
 11154  return 0;
 11155  }
 11156  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] perf: Add support for creating offline events

[kbuild test robot]

Hi Raghavendra,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.15]
[also build test ERROR on next-20180212]
[cannot apply to tip/perf/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Raghavendra-Rao-Ananta/perf-Add-support-for-creating-offline-events/20180213-023250
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel//events/core.c: In function 'perf_event_init_cpu':
>> kernel//events/core.c:11152:2: error: implicit declaration of function 
>> 'perf_deferred_install_in_context'; did you mean 'perf_install_in_context'? 
>> [-Werror=implicit-function-declaration]
 perf_deferred_install_in_context(cpu);
 ^~~~
 perf_install_in_context
   cc1: some warnings being treated as errors

vim +11152 kernel//events/core.c

 11131  
 11132  int perf_event_init_cpu(unsigned int cpu)
 11133  {
 11134  struct perf_cpu_context *cpuctx;
 11135  struct perf_event_context *ctx;
 11136  struct pmu *pmu;
 11137  
 11138  perf_swevent_init_cpu(cpu);
 11139  
 11140  mutex_lock(_lock);
 11141  cpumask_set_cpu(cpu, perf_online_mask);
 11142  list_for_each_entry(pmu, , entry) {
 11143  cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
 11144  ctx = >ctx;
 11145  
 11146  mutex_lock(>mutex);
 11147  cpuctx->online = 1;
 11148  mutex_unlock(>mutex);
 11149  }
 11150  mutex_unlock(_lock);
 11151  
 11152  perf_deferred_install_in_context(cpu);
 11153  
 11154  return 0;
 11155  }
 11156  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] perf: Add support for creating offline events

[Raghavendra Rao Ananta]

Hi Jiri,

Thank you for the response.

Does perf tool has its own check to see if the CPU was offline during 
the lifetime of an event? If so, it might ignore these type of events.


Initially, I tested the same using perf tool and found similar results.
Then I debugged further and found that the perf core was actually 
sending data to the userspace (copy_to_user()) and the corresponding 
count for the data. Hence, I tested this further by writing my own 
userspace application, and I was able to read the count through this,

even when the CPU was made offline and back online.

Do you think we also have to modify the perf tool accordingly?

Find an inline comment.

On 02/12/2018 01:43 AM, Jiri Olsa wrote:

On Fri, Feb 09, 2018 at 03:07:00PM -0800, Raghavendra Rao Ananta wrote:

Perf framework doesn't allow creation of hardware events if
the requested CPU is offline. However, creation of an event
is achievable if the event is attached to the PMU as soon
as the CPU is online again.

So, introducing a feature that could allow to create events
even when the CPU is offline and return a success to the caller.
If, during the time of event creation, the CPU is found offline,
the event is moved to a new state (PERF_EVENT_STATE_DORMANT). As
and when the CPU is know to be woken up (through hotplug notifiers),
all the dormant events would be attached to the PMU (by
perf_install_in_context()). If during the life time of the event,
the CPU hasn't come online, the dormant event would just be freed.

Signed-off-by: Raghavendra Rao Ananta 


hum, I tried and for some reason I'm getting zero counts/times
when the cpu 1 is set back online (in 9.second)

[root@ibm-x3650m4-02 perf]# ./perf stat -C 1 -e sched:sched_switch -v -I 1000
failed to read counter sched:sched_switch
#   time counts unit events
  1.000921624sched:sched_switch
failed to read counter sched:sched_switch
  2.001725364sched:sched_switch
failed to read counter sched:sched_switch
  3.002685350sched:sched_switch
failed to read counter sched:sched_switch
  4.003463851sched:sched_switch
failed to read counter sched:sched_switch
  5.004651601sched:sched_switch
failed to read counter sched:sched_switch
  6.005338294sched:sched_switch
failed to read counter sched:sched_switch
  7.006351155sched:sched_switch
failed to read counter sched:sched_switch
  8.007239698sched:sched_switch
sched:sched_switch: 0 0 0
  9.008665621sched:sched_switch
sched:sched_switch: 0 0 0
 10.009570492sched:sched_switch
sched:sched_switch: 0 0 0
 11.010811591sched:sched_switch
sched:sched_switch: 0 0 0
 12.011614182sched:sched_switch
sched:sched_switch: 0 0 0
 13.012299851sched:sched_switch

looks like the dormant event wasn't scheduled in properly

also while at it, could we also handle cpu going offline case,
so the event would survive until it's back online

Sure, I have a plan for that one as well, but wondering how this goes first.


jirka



Thank you.

Raghavendra

--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] perf: Add support for creating offline events

[Raghavendra Rao Ananta]

Hi Jiri,

Thank you for the response.

Does perf tool has its own check to see if the CPU was offline during 
the lifetime of an event? If so, it might ignore these type of events.


Initially, I tested the same using perf tool and found similar results.
Then I debugged further and found that the perf core was actually 
sending data to the userspace (copy_to_user()) and the corresponding 
count for the data. Hence, I tested this further by writing my own 
userspace application, and I was able to read the count through this,

even when the CPU was made offline and back online.

Do you think we also have to modify the perf tool accordingly?

Find an inline comment.

On 02/12/2018 01:43 AM, Jiri Olsa wrote:

On Fri, Feb 09, 2018 at 03:07:00PM -0800, Raghavendra Rao Ananta wrote:

Perf framework doesn't allow creation of hardware events if
the requested CPU is offline. However, creation of an event
is achievable if the event is attached to the PMU as soon
as the CPU is online again.

So, introducing a feature that could allow to create events
even when the CPU is offline and return a success to the caller.
If, during the time of event creation, the CPU is found offline,
the event is moved to a new state (PERF_EVENT_STATE_DORMANT). As
and when the CPU is know to be woken up (through hotplug notifiers),
all the dormant events would be attached to the PMU (by
perf_install_in_context()). If during the life time of the event,
the CPU hasn't come online, the dormant event would just be freed.

Signed-off-by: Raghavendra Rao Ananta 


hum, I tried and for some reason I'm getting zero counts/times
when the cpu 1 is set back online (in 9.second)

[root@ibm-x3650m4-02 perf]# ./perf stat -C 1 -e sched:sched_switch -v -I 1000
failed to read counter sched:sched_switch
#   time counts unit events
  1.000921624sched:sched_switch
failed to read counter sched:sched_switch
  2.001725364sched:sched_switch
failed to read counter sched:sched_switch
  3.002685350sched:sched_switch
failed to read counter sched:sched_switch
  4.003463851sched:sched_switch
failed to read counter sched:sched_switch
  5.004651601sched:sched_switch
failed to read counter sched:sched_switch
  6.005338294sched:sched_switch
failed to read counter sched:sched_switch
  7.006351155sched:sched_switch
failed to read counter sched:sched_switch
  8.007239698sched:sched_switch
sched:sched_switch: 0 0 0
  9.008665621sched:sched_switch
sched:sched_switch: 0 0 0
 10.009570492sched:sched_switch
sched:sched_switch: 0 0 0
 11.010811591sched:sched_switch
sched:sched_switch: 0 0 0
 12.011614182sched:sched_switch
sched:sched_switch: 0 0 0
 13.012299851sched:sched_switch

looks like the dormant event wasn't scheduled in properly

also while at it, could we also handle cpu going offline case,
so the event would survive until it's back online

Sure, I have a plan for that one as well, but wondering how this goes first.


jirka



Thank you.

Raghavendra

--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] perf: Add support for creating offline events

[Jiri Olsa]

On Fri, Feb 09, 2018 at 03:07:00PM -0800, Raghavendra Rao Ananta wrote:
> Perf framework doesn't allow creation of hardware events if
> the requested CPU is offline. However, creation of an event
> is achievable if the event is attached to the PMU as soon
> as the CPU is online again.
> 
> So, introducing a feature that could allow to create events
> even when the CPU is offline and return a success to the caller.
> If, during the time of event creation, the CPU is found offline,
> the event is moved to a new state (PERF_EVENT_STATE_DORMANT). As
> and when the CPU is know to be woken up (through hotplug notifiers),
> all the dormant events would be attached to the PMU (by
> perf_install_in_context()). If during the life time of the event,
> the CPU hasn't come online, the dormant event would just be freed.
> 
> Signed-off-by: Raghavendra Rao Ananta 

hum, I tried and for some reason I'm getting zero counts/times
when the cpu 1 is set back online (in 9.second)

[root@ibm-x3650m4-02 perf]# ./perf stat -C 1 -e sched:sched_switch -v -I 1000
failed to read counter sched:sched_switch
#   time counts unit events
 1.000921624sched:sched_switch 
 
failed to read counter sched:sched_switch
 2.001725364sched:sched_switch 
 
failed to read counter sched:sched_switch
 3.002685350sched:sched_switch 
 
failed to read counter sched:sched_switch
 4.003463851sched:sched_switch 
 
failed to read counter sched:sched_switch
 5.004651601sched:sched_switch 
 
failed to read counter sched:sched_switch
 6.005338294sched:sched_switch 
 
failed to read counter sched:sched_switch
 7.006351155sched:sched_switch 
 
failed to read counter sched:sched_switch
 8.007239698sched:sched_switch 
 
sched:sched_switch: 0 0 0
 9.008665621sched:sched_switch 
 
sched:sched_switch: 0 0 0
10.009570492sched:sched_switch 
 
sched:sched_switch: 0 0 0
11.010811591sched:sched_switch 
 
sched:sched_switch: 0 0 0
12.011614182sched:sched_switch 
 
sched:sched_switch: 0 0 0
13.012299851sched:sched_switch 
 

looks like the dormant event wasn't scheduled in properly

also while at it, could we also handle cpu going offline case,
so the event would survive until it's back online

jirka

Re: [PATCH] perf: Add support for creating offline events

[Jiri Olsa]

On Fri, Feb 09, 2018 at 03:07:00PM -0800, Raghavendra Rao Ananta wrote:

SNIP

>  
>   if (!task) {
> +#if defined CONFIG_HOTPLUG_CPU || defined CONFIG_KEXEC_CORE
> + struct perf_cpu_context *cpuctx =
> + container_of(ctx, struct perf_cpu_context, ctx);
> +
> + if (!cpuctx->online) {
> + perf_prepare_install_in_context(event);
> + return;
> + }
> +#endif
>   cpu_function_call(cpu, __perf_install_in_context, event);
>   return;
>   }
> @@ -2421,6 +2443,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(_event_list_lock);
> + list_for_each_entry_safe(event, tmp, _event_list,
> + dormant_entry) {
> + if (cpu != event->cpu)
> + continue;

I wonder having per cpu lists would be better here,
could be quite busy for big number of CPUs

jirka

Re: [PATCH] perf: Add support for creating offline events

[Jiri Olsa]

On Fri, Feb 09, 2018 at 03:07:00PM -0800, Raghavendra Rao Ananta wrote:
> Perf framework doesn't allow creation of hardware events if
> the requested CPU is offline. However, creation of an event
> is achievable if the event is attached to the PMU as soon
> as the CPU is online again.
> 
> So, introducing a feature that could allow to create events
> even when the CPU is offline and return a success to the caller.
> If, during the time of event creation, the CPU is found offline,
> the event is moved to a new state (PERF_EVENT_STATE_DORMANT). As
> and when the CPU is know to be woken up (through hotplug notifiers),
> all the dormant events would be attached to the PMU (by
> perf_install_in_context()). If during the life time of the event,
> the CPU hasn't come online, the dormant event would just be freed.
> 
> Signed-off-by: Raghavendra Rao Ananta 

hum, I tried and for some reason I'm getting zero counts/times
when the cpu 1 is set back online (in 9.second)

[root@ibm-x3650m4-02 perf]# ./perf stat -C 1 -e sched:sched_switch -v -I 1000
failed to read counter sched:sched_switch
#   time counts unit events
 1.000921624sched:sched_switch 
 
failed to read counter sched:sched_switch
 2.001725364sched:sched_switch 
 
failed to read counter sched:sched_switch
 3.002685350sched:sched_switch 
 
failed to read counter sched:sched_switch
 4.003463851sched:sched_switch 
 
failed to read counter sched:sched_switch
 5.004651601sched:sched_switch 
 
failed to read counter sched:sched_switch
 6.005338294sched:sched_switch 
 
failed to read counter sched:sched_switch
 7.006351155sched:sched_switch 
 
failed to read counter sched:sched_switch
 8.007239698sched:sched_switch 
 
sched:sched_switch: 0 0 0
 9.008665621sched:sched_switch 
 
sched:sched_switch: 0 0 0
10.009570492sched:sched_switch 
 
sched:sched_switch: 0 0 0
11.010811591sched:sched_switch 
 
sched:sched_switch: 0 0 0
12.011614182sched:sched_switch 
 
sched:sched_switch: 0 0 0
13.012299851sched:sched_switch 
 

looks like the dormant event wasn't scheduled in properly

also while at it, could we also handle cpu going offline case,
so the event would survive until it's back online

jirka

Re: [PATCH] perf: Add support for creating offline events

[Jiri Olsa]

On Fri, Feb 09, 2018 at 03:07:00PM -0800, Raghavendra Rao Ananta wrote:

SNIP

>  
>   if (!task) {
> +#if defined CONFIG_HOTPLUG_CPU || defined CONFIG_KEXEC_CORE
> + struct perf_cpu_context *cpuctx =
> + container_of(ctx, struct perf_cpu_context, ctx);
> +
> + if (!cpuctx->online) {
> + perf_prepare_install_in_context(event);
> + return;
> + }
> +#endif
>   cpu_function_call(cpu, __perf_install_in_context, event);
>   return;
>   }
> @@ -2421,6 +2443,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(_event_list_lock);
> + list_for_each_entry_safe(event, tmp, _event_list,
> + dormant_entry) {
> + if (cpu != event->cpu)
> + continue;

I wonder having per cpu lists would be better here,
could be quite busy for big number of CPUs

jirka