[PATCH v3 0/3] perf/core: expose thread context switch out event type to user space

[PATCH v3 0/3] perf/core: expose thread context switch out event type to user space

[Alexey Budankov]

Implementation of exposing context-switch-out type event as a part 
of PERF_RECORD_SWITCH[_CPU_WIDE] record.

Introduced types of events assumed to be:

a) preempt: when task->state == TASK_RUNNING, encoding is implemented using
   new PERF_RECORD_MISC_SWITCH_OUT_PREEMPT bit
   event_header->misc &= 
	PERF_RECORD_MISC_SWITCH_OUT|PERF_RECORD_MISC_SWITCH_OUT_PREEMPT

b) !preempt: encoding is implemented the only PERF_RECORD_MISC_SWITCH_OUT bit
   event_header->misc &= PERF_RECORD_MISC_SWITCH_OUT

Perf tool report and script commands output has been extended to decode 
new PREEMPT bit and the updated output looks like in the examples below.

The documentation has been updated to mention PREEMPT switch out events 
and its decoding symbols in perf script output.

The changes have been manually tested on Fedora 27 with the patched kernel:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core

tools/perf/perf report -D -i perf.data | grep _SWITCH

0 768361415226 0x27f076 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:     8/8    
4 768362216813 0x28f45e [0x28]: PERF_RECORD_SWITCH_CPU_WIDE OUT          next pid/tid:     0/0    
4 768362217824 0x28f486 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:  4073/4073 
0 768362414027 0x27f0ce [0x28]: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt  next pid/tid:     8/8    
0 768362414367 0x27f0f6 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:     0/0    

perf script --show-switch-events -F +misc -I -i perf.data:

          hdparm  4073 [004] U       762.198265:     380194 cycles:ppp:      7faf727f5a23 strchr (/usr/lib64/ld-2.26.so)
          hdparm  4073 [004] K       762.198366:     441572 cycles:ppp:  ffffffffb9218435 alloc_set_pte (/lib/modules/4.16.0-rc6+/build/vmlinux)
          hdparm  4073 [004] S       762.198391: PERF_RECORD_SWITCH_CPU_WIDE OUT          next pid/tid:     0/0    
         swapper     0 [004]         762.198392: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:  4073/4073 
         swapper     0 [004] Sp      762.198477: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt  next pid/tid:  4073/4073 
          hdparm  4073 [004]         762.198478: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:     0/0    
         swapper     0 [007] K       762.198514:    2303073 cycles:ppp:  ffffffffb98b0c66 intel_idle (/lib/modules/4.16.0-rc6+/build/vmlinux)
         swapper     0 [007] Sp      762.198561: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt  next pid/tid:  1134/1134 
  kworker/u16:18  1134 [007]         762.198562: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:     0/0    
  kworker/u16:18  1134 [007] S       762.198567: PERF_RECORD_SWITCH_CPU_WIDE OUT          next pid/tid:     0/0    

---
 Alexey Budankov (3):
	perf/core: store context switch out type into Perf trace
	perf report: extend raw dump (-D) out with switch out event type
	perf script: extend misc field decoding with switch out event type

 include/uapi/linux/perf_event.h          |  4 ++++
 kernel/events/core.c                     |  4 +++-
 tools/include/uapi/linux/perf_event.h    |  4 ++++
 tools/perf/Documentation/perf-script.txt | 17 +++++++++--------
 tools/perf/builtin-script.c              |  5 ++++-
 tools/perf/util/event.c                  |  4 +++-
 6 files changed, 27 insertions(+), 11 deletions(-)

[PATCH v3 1/3] perf/core: store context switch out type into Perf trace

[Alexey Budankov]

Store thread context-switch-out event type into Perf trace as a part of 
PERF_RECORD_SWITCH[_CPU_WIDE] records.

Introduced types of switch-out events assumed to be 
a) preempt: task->state == TASK_RUNNING and b) !preempt;

New !preempt event type is encoded using new 
PERF_RECORD_MISC_SWITCH_OUT_PREEMPT bit extending 
existing PERF_RECORD_MISC_SWITCH_OUT bit of switch out event:

    misc &= PERF_RECORD_MISC_SWITCH_OUT | PERF_RECORD_MISC_SWITCH_OUT_PREEMPT
	
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 include/uapi/linux/perf_event.h       | 4 ++++
 kernel/events/core.c                  | 4 +++-
 tools/include/uapi/linux/perf_event.h | 4 ++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 912b85b52344..cd6ad7e13824 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -655,6 +655,10 @@ struct perf_event_mmap_page {
  * perf_event_attr::precise_ip.
  */
 #define PERF_RECORD_MISC_EXACT_IP		(1 << 14)
+/*
+ * Indicates that thread was preempted in TASK_RUNNING state
+ */
+#define PERF_RECORD_MISC_SWITCH_OUT_PREEMPT	(1 << 14)
 /*
  * Reserve the last bit to indicate some extended misc field
  */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 74a6e8f12a3c..0d39192215bc 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7556,6 +7556,8 @@ static void perf_event_switch(struct task_struct *task,
 			      struct task_struct *next_prev, bool sched_in)
 {
 	struct perf_switch_event switch_event;
+	__u16 switch_type = sched_in ? 0 : PERF_RECORD_MISC_SWITCH_OUT |
+		(task->state == TASK_RUNNING ? PERF_RECORD_MISC_SWITCH_OUT_PREEMPT : 0);
 
 	/* N.B. caller checks nr_switch_events != 0 */
 
@@ -7565,7 +7567,7 @@ static void perf_event_switch(struct task_struct *task,
 		.event_id	= {
 			.header = {
 				/* .type */
-				.misc = sched_in ? 0 : PERF_RECORD_MISC_SWITCH_OUT,
+				.misc = switch_type,
 				/* .size */
 			},
 			/* .next_prev_pid */
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index 912b85b52344..cd6ad7e13824 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -655,6 +655,10 @@ struct perf_event_mmap_page {
  * perf_event_attr::precise_ip.
  */
 #define PERF_RECORD_MISC_EXACT_IP		(1 << 14)
+/*
+ * Indicates that thread was preempted in TASK_RUNNING state
+ */
+#define PERF_RECORD_MISC_SWITCH_OUT_PREEMPT	(1 << 14)
 /*
  * Reserve the last bit to indicate some extended misc field
  */

[PATCH v3 2/3] perf report: extend raw dump (-D) out with switch out event type

[Alexey Budankov]

Print additional 'preempt' tag for PERF_RECORD_SWITCH[_CPU_WIDE] OUT records when
event header misc field contains PERF_RECORD_MISC_SWITCH_OUT_PREEMPT bit set 
designating preemption context switch out event:

tools/perf/perf report -D -i perf.data | grep _SWITCH

0 768361415226 0x27f076 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:     8/8    
4 768362216813 0x28f45e [0x28]: PERF_RECORD_SWITCH_CPU_WIDE OUT          next pid/tid:     0/0    
4 768362217824 0x28f486 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:  4073/4073 
0 768362414027 0x27f0ce [0x28]: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt  next pid/tid:     8/8    
0 768362414367 0x27f0f6 [0x28]: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:     0/0    

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 tools/perf/util/event.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index f0a6cbd033cc..98ff3a6a3d50 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -1421,7 +1421,9 @@ size_t perf_event__fprintf_itrace_start(union perf_event *event, FILE *fp)
 size_t perf_event__fprintf_switch(union perf_event *event, FILE *fp)
 {
 	bool out = event->header.misc & PERF_RECORD_MISC_SWITCH_OUT;
-	const char *in_out = out ? "OUT" : "IN ";
+	const char *in_out = !out ? "IN         " :
+		!(event->header.misc & PERF_RECORD_MISC_SWITCH_OUT_PREEMPT) ?
+				    "OUT        " : "OUT preempt";
 
 	if (event->header.type == PERF_RECORD_SWITCH)
 		return fprintf(fp, " %s\n", in_out);

[PATCH v3 3/3] perf script: extend misc field decoding with switch out event type

[Alexey Budankov]

Append 'p' sign to 'S' tag designating the type of context switch out event so 
'Sp' means preemption context switch. Documentation is extended to cover 
new presentation changes.

perf script --show-switch-events -F +misc -I -i perf.data:

          hdparm  4073 [004] U       762.198265:     380194 cycles:ppp:      7faf727f5a23 strchr (/usr/lib64/ld-2.26.so)
          hdparm  4073 [004] K       762.198366:     441572 cycles:ppp:  ffffffffb9218435 alloc_set_pte (/lib/modules/4.16.0-rc6+/build/vmlinux)
          hdparm  4073 [004] S       762.198391: PERF_RECORD_SWITCH_CPU_WIDE OUT          next pid/tid:     0/0    
         swapper     0 [004]         762.198392: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:  4073/4073 
         swapper     0 [004] Sp      762.198477: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt  next pid/tid:  4073/4073 
          hdparm  4073 [004]         762.198478: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:     0/0    
         swapper     0 [007] K       762.198514:    2303073 cycles:ppp:  ffffffffb98b0c66 intel_idle (/lib/modules/4.16.0-rc6+/build/vmlinux)
         swapper     0 [007] Sp      762.198561: PERF_RECORD_SWITCH_CPU_WIDE OUT preempt  next pid/tid:  1134/1134 
  kworker/u16:18  1134 [007]         762.198562: PERF_RECORD_SWITCH_CPU_WIDE IN           prev pid/tid:     0/0    
  kworker/u16:18  1134 [007] S       762.198567: PERF_RECORD_SWITCH_CPU_WIDE OUT          next pid/tid:     0/0    

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 tools/perf/Documentation/perf-script.txt | 17 +++++++++--------
 tools/perf/builtin-script.c              |  5 ++++-
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 36ec0257f8d3..afdafe2110a1 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -228,14 +228,15 @@ OPTIONS
 	For sample events it's possible to display misc field with -F +misc option,
 	following letters are displayed for each bit:
 
-	  PERF_RECORD_MISC_KERNEL        K
-	  PERF_RECORD_MISC_USER          U
-	  PERF_RECORD_MISC_HYPERVISOR    H
-	  PERF_RECORD_MISC_GUEST_KERNEL  G
-	  PERF_RECORD_MISC_GUEST_USER    g
-	  PERF_RECORD_MISC_MMAP_DATA*    M
-	  PERF_RECORD_MISC_COMM_EXEC     E
-	  PERF_RECORD_MISC_SWITCH_OUT    S
+	  PERF_RECORD_MISC_KERNEL               K
+	  PERF_RECORD_MISC_USER                 U
+	  PERF_RECORD_MISC_HYPERVISOR           H
+	  PERF_RECORD_MISC_GUEST_KERNEL         G
+	  PERF_RECORD_MISC_GUEST_USER           g
+	  PERF_RECORD_MISC_MMAP_DATA*           M
+	  PERF_RECORD_MISC_COMM_EXEC            E
+	  PERF_RECORD_MISC_SWITCH_OUT           S
+	  PERF_RECORD_MISC_SWITCH_OUT_PREEMPT   Sp
 
 	  $ perf script -F +misc ...
 	   sched-messaging  1414 K     28690.636582:       4590 cycles ...
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 313c42423393..0f916e8e1835 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -657,8 +657,11 @@ static int perf_sample__fprintf_start(struct perf_sample *sample,
 			break;
 		case PERF_RECORD_SWITCH:
 		case PERF_RECORD_SWITCH_CPU_WIDE:
-			if (has(SWITCH_OUT))
+			if (has(SWITCH_OUT)) {
 				ret += fprintf(fp, "S");
+				if (sample->misc & PERF_RECORD_MISC_SWITCH_OUT_PREEMPT)
+					ret += fprintf(fp, "p");
+			}
 		default:
 			break;
 		}

Re: [PATCH v3 1/3] perf/core: store context switch out type into Perf trace

[Alexander Shishkin]

On Mon, Mar 26, 2018 at 12:20:32PM +0300, Alexey Budankov wrote:
> 
> Store thread context-switch-out event type into Perf trace as a part of 
> PERF_RECORD_SWITCH[_CPU_WIDE] records.
> 
> Introduced types of switch-out events assumed to be 
> a) preempt: task->state == TASK_RUNNING and b) !preempt;
> 
> New !preempt event type is encoded using new 
> PERF_RECORD_MISC_SWITCH_OUT_PREEMPT bit extending 
> existing PERF_RECORD_MISC_SWITCH_OUT bit of switch out event:
> 
>     misc &= PERF_RECORD_MISC_SWITCH_OUT | PERF_RECORD_MISC_SWITCH_OUT_PREEMPT

I'd like to offer some suggestions as to how to make the commit message
friendlier for reviewing.

Generally, for every patch, we want to explain the following: what we want,
why we want it and how we want to go about getting it. We also would prefer
to do it in english rather than in C, because for the latter we can just
look at the code.

So, my understanding of this patch translates into something like this.

What: we want to tell apart preemting and non-preempting context switches.
Why: I'm guessing it tells us something about the kind of workloads that
are running on the machine. This doesn't seem to be mentioned anywhere.
How: we add a new bit to the event header to indicate that the corresponding
sched-out is preempting.

> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
>  include/uapi/linux/perf_event.h       | 4 ++++
>  kernel/events/core.c                  | 4 +++-
>  tools/include/uapi/linux/perf_event.h | 4 ++++

The last one probably wants to be a separate patch.

>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 912b85b52344..cd6ad7e13824 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -655,6 +655,10 @@ struct perf_event_mmap_page {
>   * perf_event_attr::precise_ip.
>   */
>  #define PERF_RECORD_MISC_EXACT_IP		(1 << 14)
> +/*
> + * Indicates that thread was preempted in TASK_RUNNING state
> + */
> +#define PERF_RECORD_MISC_SWITCH_OUT_PREEMPT	(1 << 14)
>  /*
>   * Reserve the last bit to indicate some extended misc field
>   */
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 74a6e8f12a3c..0d39192215bc 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7556,6 +7556,8 @@ static void perf_event_switch(struct task_struct *task,
>  			      struct task_struct *next_prev, bool sched_in)
>  {
>  	struct perf_switch_event switch_event;
> +	__u16 switch_type = sched_in ? 0 : PERF_RECORD_MISC_SWITCH_OUT |
> +		(task->state == TASK_RUNNING ? PERF_RECORD_MISC_SWITCH_OUT_PREEMPT : 0);

This is also hard on the eyes. Can't we just

if (!sched_in) {
	misc = SWITCH_OUT;
	if (task->state == TASK_RUNNING)
		misc |= SWITCH_OUT_PREEMPT;
}

?

Thanks,
--
Alex

Re: [PATCH v3 1/3] perf/core: store context switch out type into Perf trace

[Alexey Budankov]

On 29.03.2018 16:31, Alexander Shishkin wrote:
> On Mon, Mar 26, 2018 at 12:20:32PM +0300, Alexey Budankov wrote:
>>
>> Store thread context-switch-out event type into Perf trace as a part of 
>> PERF_RECORD_SWITCH[_CPU_WIDE] records.
>>
>> Introduced types of switch-out events assumed to be 
>> a) preempt: task->state == TASK_RUNNING and b) !preempt;
>>
>> New !preempt event type is encoded using new 
>> PERF_RECORD_MISC_SWITCH_OUT_PREEMPT bit extending 
>> existing PERF_RECORD_MISC_SWITCH_OUT bit of switch out event:
>>
>>     misc &= PERF_RECORD_MISC_SWITCH_OUT | PERF_RECORD_MISC_SWITCH_OUT_PREEMPT
> 
> I'd like to offer some suggestions as to how to make the commit message
> friendlier for reviewing.
> 
> Generally, for every patch, we want to explain the following: what we want,
> why we want it and how we want to go about getting it. We also would prefer
> to do it in english rather than in C, because for the latter we can just
> look at the code.

It makes sense. Thanks for sharing your opinion and valuable guidance.

> 
> So, my understanding of this patch translates into something like this.
> 
> What: we want to tell apart preemting and non-preempting context switches.
> Why: I'm guessing it tells us something about the kind of workloads that
> are running on the machine. This doesn't seem to be mentioned anywhere.

My bad. That is missing from the description but you got the rationale just right.

> How: we add a new bit to the event header to indicate that the corresponding
> sched-out is preempting.
> 
>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
>> ---
>>  include/uapi/linux/perf_event.h       | 4 ++++
>>  kernel/events/core.c                  | 4 +++-
>>  tools/include/uapi/linux/perf_event.h | 4 ++++
> 
> The last one probably wants to be a separate patch.

Yes. It could be. Tried to minimize amount of patches in the series, 
at the same time remembering about successful git bisect integration.

> 
>>  3 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
>> index 912b85b52344..cd6ad7e13824 100644
>> --- a/include/uapi/linux/perf_event.h
>> +++ b/include/uapi/linux/perf_event.h
>> @@ -655,6 +655,10 @@ struct perf_event_mmap_page {
>>   * perf_event_attr::precise_ip.
>>   */
>>  #define PERF_RECORD_MISC_EXACT_IP		(1 << 14)
>> +/*
>> + * Indicates that thread was preempted in TASK_RUNNING state
>> + */
>> +#define PERF_RECORD_MISC_SWITCH_OUT_PREEMPT	(1 << 14)
>>  /*
>>   * Reserve the last bit to indicate some extended misc field
>>   */
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index 74a6e8f12a3c..0d39192215bc 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -7556,6 +7556,8 @@ static void perf_event_switch(struct task_struct *task,
>>  			      struct task_struct *next_prev, bool sched_in)
>>  {
>>  	struct perf_switch_event switch_event;
>> +	__u16 switch_type = sched_in ? 0 : PERF_RECORD_MISC_SWITCH_OUT |
>> +		(task->state == TASK_RUNNING ? PERF_RECORD_MISC_SWITCH_OUT_PREEMPT : 0);
> 
> This is also hard on the eyes. Can't we just
> 
> if (!sched_in) {
> 	misc = SWITCH_OUT;
> 	if (task->state == TASK_RUNNING)
> 		misc |= SWITCH_OUT_PREEMPT;
> }
> 
> ?

Sure. Let me take care of all your comments.

Thanks,
Alexey

> 
> Thanks,
> --
> Alex
> 
>