Re: [PATCH V2 1/5] perf/core: Add PERF_SAMPLE_WEIGHT_STRUCT

2021-01-27 Thread Liang, Kan 




On 1/27/2021 2:03 PM, Peter Zijlstra wrote:

On Wed, Jan 27, 2021 at 07:38:41AM -0800, kan.li...@linux.intel.com wrote:

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index b15e344..13b4019 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -145,12 +145,14 @@ enum perf_event_sample_format {
PERF_SAMPLE_CGROUP  = 1U << 21,
PERF_SAMPLE_DATA_PAGE_SIZE  = 1U << 22,
PERF_SAMPLE_CODE_PAGE_SIZE  = 1U << 23,
+   PERF_SAMPLE_WEIGHT_STRUCT   = 1U << 24,
  
-	PERF_SAMPLE_MAX = 1U << 24,		/* non-ABI */

+   PERF_SAMPLE_MAX = 1U << 25,   /* non-ABI */
  
  	__PERF_SAMPLE_CALLCHAIN_EARLY		= 1ULL << 63, /* non-ABI; internal use */

  };
  
+#define PERF_SAMPLE_WEIGHT_TYPE	(PERF_SAMPLE_WEIGHT | PERF_SAMPLE_WEIGHT_STRUCT)

  /*
   * values to program into branch_sample_type when PERF_SAMPLE_BRANCH is set
   *
@@ -890,7 +892,16 @@ enum perf_event_type {
 *char  data[size];
 *u64   dyn_size; } && PERF_SAMPLE_STACK_USER
 *
-*  { u64   weight;   } && PERF_SAMPLE_WEIGHT
+*  { union perf_sample_weight
+*   {
+*  u64 full; && PERF_SAMPLE_WEIGHT
+*  struct {
+*  u32 low_dword;
+*  u16 high_word;
+*  u16 higher_word;
+*  } && PERF_SAMPLE_WEIGHT_STRUCT
+*   }
+*  }
 *  { u64   data_src; } && PERF_SAMPLE_DATA_SRC
 *  { u64   transaction; } && 
PERF_SAMPLE_TRANSACTION
 *  { u64   abi; # enum perf_sample_regs_abi
@@ -1248,4 +1259,13 @@ struct perf_branch_entry {
reserved:40;
  };
  
+union perf_sample_weight {

+   __u64   full;
+   struct {
+   __u32   low_dword;
+   __u16   high_word;
+   __u16   higher_word;
+   };
+};


*urgh*, my naming lives ... anybody got a better suggestion?


I think we need a generic name here, but the problem is that the 
'weight' field has different meanings among architectures.


The 'weight' fields are to store all kinds of latency on X86.
On PowerPC, it stores MMCRA[TECX/TECM], which doesn't look like a latency.

I don't think I can use the name, 'cache_lat' or 'instruction_lat', 
here. Right?

If so, how about 'var'?

u32 var_1_dw;
u16 var_2_w;
u16 var_3_w;




Also, do we want to care about byte order?


Sure. I will add the big-endian and little-endian support.


Thanks,
Kan

Re: [PATCH V2 1/5] perf/core: Add PERF_SAMPLE_WEIGHT_STRUCT

2021-01-27 Thread Peter Zijlstra 
On Wed, Jan 27, 2021 at 07:38:41AM -0800, kan.li...@linux.intel.com wrote:
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index b15e344..13b4019 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -145,12 +145,14 @@ enum perf_event_sample_format {
>   PERF_SAMPLE_CGROUP  = 1U << 21,
>   PERF_SAMPLE_DATA_PAGE_SIZE  = 1U << 22,
>   PERF_SAMPLE_CODE_PAGE_SIZE  = 1U << 23,
> + PERF_SAMPLE_WEIGHT_STRUCT   = 1U << 24,
>  
> - PERF_SAMPLE_MAX = 1U << 24, /* non-ABI */
> + PERF_SAMPLE_MAX = 1U << 25, /* non-ABI */
>  
>   __PERF_SAMPLE_CALLCHAIN_EARLY   = 1ULL << 63, /* non-ABI; 
> internal use */
>  };
>  
> +#define PERF_SAMPLE_WEIGHT_TYPE  (PERF_SAMPLE_WEIGHT | 
> PERF_SAMPLE_WEIGHT_STRUCT)
>  /*
>   * values to program into branch_sample_type when PERF_SAMPLE_BRANCH is set
>   *
> @@ -890,7 +892,16 @@ enum perf_event_type {
>*char  data[size];
>*u64   dyn_size; } && PERF_SAMPLE_STACK_USER
>*
> -  *  { u64   weight;   } && PERF_SAMPLE_WEIGHT
> +  *  { union perf_sample_weight
> +  *   {
> +  *  u64 full; && PERF_SAMPLE_WEIGHT
> +  *  struct {
> +  *  u32 low_dword;
> +  *  u16 high_word;
> +  *  u16 higher_word;
> +  *  } && PERF_SAMPLE_WEIGHT_STRUCT
> +  *   }
> +  *  }
>*  { u64   data_src; } && PERF_SAMPLE_DATA_SRC
>*  { u64   transaction; } && 
> PERF_SAMPLE_TRANSACTION
>*  { u64   abi; # enum perf_sample_regs_abi
> @@ -1248,4 +1259,13 @@ struct perf_branch_entry {
>   reserved:40;
>  };
>  
> +union perf_sample_weight {
> + __u64   full;
> + struct {
> + __u32   low_dword;
> + __u16   high_word;
> + __u16   higher_word;
> + };
> +};

*urgh*, my naming lives ... anybody got a better suggestion?

Also, do we want to care about byte order?

[PATCH V2 1/5] perf/core: Add PERF_SAMPLE_WEIGHT_STRUCT

2021-01-27 Thread kan . liang 
From: Kan Liang 

Current PERF_SAMPLE_WEIGHT sample type is very useful to expresses the
cost of an action represented by the sample. This allows the profiler
to scale the samples to be more informative to the programmer. It could
also help to locate a hotspot, e.g., when profiling by memory latencies,
the expensive load appear higher up in the histograms. But current
PERF_SAMPLE_WEIGHT sample type is solely determined by one factor. This
could be a problem, if users want two or more factors to contribute to
the weight. For example, Golden Cove core PMU can provide both the
instruction latency and the cache Latency information as factors for the
memory profiling.

For current X86 platforms, although meminfo::latency is defined as a
u64, only the lower 32 bits include the valid data in practice (No
memory access could last than 4G cycles). The higher 32 bits can be used
to store new factors.

Add a new sample type, PERF_SAMPLE_WEIGHT_STRUCT, to indicate the new
sample weight structure. It shares the same space as the
PERF_SAMPLE_WEIGHT sample type.

Users can apply either the PERF_SAMPLE_WEIGHT sample type or the
PERF_SAMPLE_WEIGHT_STRUCT sample type to retrieve the sample weight, but
they cannot apply both sample types simultaneously.

Currently, only X86 and PowerPC use the PERF_SAMPLE_WEIGHT sample type.
- For PowerPC, there is nothing changed for the PERF_SAMPLE_WEIGHT
  sample type. There is no effect for the new PERF_SAMPLE_WEIGHT_STRUCT
  sample type. PowerPC can re-struct the weight field similarly later.
- For X86, the same value will be dumped for the PERF_SAMPLE_WEIGHT
  sample type or the PERF_SAMPLE_WEIGHT_STRUCT sample type for now.
  The following patches will apply the new factors for the
  PERF_SAMPLE_WEIGHT_STRUCT sample type.

Suggested-by: Peter Zijlstra (Intel) 
Signed-off-by: Kan Liang 
---
 arch/powerpc/perf/core-book3s.c |  2 +-
 arch/x86/events/intel/ds.c  | 17 +
 include/linux/perf_event.h  |  4 ++--
 include/uapi/linux/perf_event.h | 24 ++--
 kernel/events/core.c| 11 +++
 5 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 28206b1..869d999 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2195,7 +2195,7 @@ static void record_and_restart(struct perf_event *event, 
unsigned long val,
 
if (event->attr.sample_type & PERF_SAMPLE_WEIGHT &&
ppmu->get_mem_weight)
-   ppmu->get_mem_weight();
+   ppmu->get_mem_weight();
 
if (perf_event_overflow(event, , regs))
power_pmu_stop(event, 0);
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 67dbc91..2f54b1f 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -960,7 +960,8 @@ static void adaptive_pebs_record_size_update(void)
 }
 
 #define PERF_PEBS_MEMINFO_TYPE (PERF_SAMPLE_ADDR | PERF_SAMPLE_DATA_SRC |   \
-   PERF_SAMPLE_PHYS_ADDR | PERF_SAMPLE_WEIGHT | \
+   PERF_SAMPLE_PHYS_ADDR |  \
+   PERF_SAMPLE_WEIGHT_TYPE |\
PERF_SAMPLE_TRANSACTION |\
PERF_SAMPLE_DATA_PAGE_SIZE)
 
@@ -987,7 +988,7 @@ static u64 pebs_update_adaptive_cfg(struct perf_event 
*event)
gprs = (sample_type & PERF_SAMPLE_REGS_INTR) &&
   (attr->sample_regs_intr & PEBS_GP_REGS);
 
-   tsx_weight = (sample_type & PERF_SAMPLE_WEIGHT) &&
+   tsx_weight = (sample_type & PERF_SAMPLE_WEIGHT_TYPE) &&
 ((attr->config & INTEL_ARCH_EVENT_MASK) ==
  x86_pmu.rtm_abort_event);
 
@@ -1369,8 +1370,8 @@ static void setup_pebs_fixed_sample_data(struct 
perf_event *event,
/*
 * Use latency for weight (only avail with PEBS-LL)
 */
-   if (fll && (sample_type & PERF_SAMPLE_WEIGHT))
-   data->weight = pebs->lat;
+   if (fll && (sample_type & PERF_SAMPLE_WEIGHT_TYPE))
+   data->weight.full = pebs->lat;
 
/*
 * data.data_src encodes the data source
@@ -1462,8 +1463,8 @@ static void setup_pebs_fixed_sample_data(struct 
perf_event *event,
 
if (x86_pmu.intel_cap.pebs_format >= 2) {
/* Only set the TSX weight when no memory weight. */
-   if ((sample_type & PERF_SAMPLE_WEIGHT) && !fll)
-   data->weight = intel_get_tsx_weight(pebs->tsx_tuning);
+   if ((sample_type & PERF_SAMPLE_WEIGHT_TYPE) && !fll)
+   data->weight.full = 
intel_get_tsx_weight(pebs->tsx_tuning);
 
if (sample_type & PERF_SAMPLE_TRANSACTION)
data->txn = intel_get_tsx_transaction(pebs->tsx_tuning,