From 6b4c95b2d6a05ea181442e99c28c008ac7a39c17 Mon Sep 17 00:00:00 2001 From: Kajetan Puchalski Date: Wed, 12 Apr 2023 16:39:12 +0100 Subject: [PATCH 1/4] lisa._assets.kmodules.sched_tp: parsec.h: Add private field to parse_buffer This commit adds a 'private' field to parse_buffer in parsec.h. This field can be used to pass down any data that could be relevant to the parser's output but is not contained in the input itself. It also adds a 'private' variable inside the SEQUENCE parser which serves as a handle to that data. Authored-by: Douglas Raillard Signed-off-by: Kajetan Puchalski --- lisa/_assets/kmodules/sched_tp/parsec.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/lisa/_assets/kmodules/sched_tp/parsec.h b/lisa/_assets/kmodules/sched_tp/parsec.h index ac47f73ea..19f1807f0 100644 --- a/lisa/_assets/kmodules/sched_tp/parsec.h +++ b/lisa/_assets/kmodules/sched_tp/parsec.h @@ -13,11 +13,13 @@ * @size: Size of the data buffer. * @capacity: Maximum size of the data pointed. This can be larger than @size * when describing a window into the buffer. + * @private: Pointer to additional data to be passed down the parser chain. */ typedef struct parse_buffer { u8 *data; size_t size; size_t capacity; + void *private; } parse_buffer; /** @@ -55,11 +57,13 @@ typedef struct parse_buffer { /** * charp2parse_buffer() - Convert a null-terminated string to struct parse_buffer. * @s: Null terminated string. + * @private: Pointer to additional data to be passed down the parser chain. */ -static inline parse_buffer charp2parse_buffer(char *s) +static inline parse_buffer charp2parse_buffer(char *s, void *private) { size_t size = strlen(s) + 1; return (parse_buffer){ .data = (u8 *)s, + .private = private, .size = size, .capacity = size }; } @@ -149,6 +153,7 @@ static inline PARSE_RESULT(parse_buffer) parse_buffer)){ .tag = PARSE_SUCCESS, .remainder = (parse_buffer){ + .private = input->private, .data = input->data + len, .size = input->size - @@ -158,6 +163,7 @@ static inline PARSE_RESULT(parse_buffer) len, }, .value = { + .private = input->private, .data = input->data, .size = len, .capacity = @@ -184,6 +190,7 @@ static inline PARSE_RESULT(u8) .tag = PARSE_SUCCESS, .remainder = (parse_buffer){ + .private = input->private, .data = input->data + 1, .size = input->size - 1, .capacity = @@ -228,7 +235,8 @@ static inline PARSE_RESULT(u8) return (PARSE_RESULT(u8)){ .tag = PARSE_SUCCESS, .remainder = - (parse_buffer){ .data = input->data + 1, + (parse_buffer){ .private = input->private, + .data = input->data + 1, .size = input->size - 1, .capacity = input->capacity - 1 }, @@ -423,6 +431,7 @@ static inline PARSE_RESULT(u8) parse_not_char(parse_buffer *input, u8 c) .tag = PARSE_SUCCESS, \ .value = \ (parse_buffer){ \ + .private = input->private, \ .data = input->data, \ .size = res.remainder \ .data - \ @@ -543,8 +552,8 @@ static inline PARSE_RESULT(u8) parse_not_char(parse_buffer *input, u8 c) * @type: Return type of the sequence. * @name: Name of the new parser to create. * @body: Statement expr containing the custom logic. The last statement will be - * the value returned by the parser. - * + * the value returned by the parser. Private data from parse_buffer can be + * accessed inside the body through the `private` variable. */ #define SEQUENCE(type, name, body, ...) \ static inline PARSE_RESULT(type) \ @@ -552,6 +561,8 @@ static inline PARSE_RESULT(u8) parse_not_char(parse_buffer *input, u8 c) { \ parse_buffer __seq_remainder = *input; \ parse_buffer __seq_unmodified_input = *input; \ + void *private = input->private; \ + (void)private; \ type __seq_value = (body); \ return (PARSE_RESULT(type)){ \ .tag = PARSE_SUCCESS, \ -- GitLab From c9bac56ed52bcb698dabb407a56422bbc0c797d6 Mon Sep 17 00:00:00 2001 From: Kajetan Puchalski Date: Wed, 12 Apr 2023 15:05:40 +0100 Subject: [PATCH 2/4] lisa._assets.kmodules.sched_tp: Add device field to pixel6_emeter FEATURE Use the previously added 'private' field in order to pass down the ID of the device from which power meter measurements are coming and include that information in the pixel6_emeter trace event. Signed-off-by: Kajetan Puchalski --- .../_assets/kmodules/sched_tp/ftrace_events.h | 12 +++++++----- lisa/_assets/kmodules/sched_tp/pixel6.c | 19 +++++++++++++++---- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lisa/_assets/kmodules/sched_tp/ftrace_events.h b/lisa/_assets/kmodules/sched_tp/ftrace_events.h index 153978f5d..6bae72509 100644 --- a/lisa/_assets/kmodules/sched_tp/ftrace_events.h +++ b/lisa/_assets/kmodules/sched_tp/ftrace_events.h @@ -336,25 +336,27 @@ TRACE_EVENT(sched_cpu_capacity, #define PIXEL6_EMETER_CHAN_NAME_MAX_SIZE 64 TRACE_EVENT(pixel6_emeter, - TP_PROTO(unsigned long ts, unsigned int chan, char *chan_name, unsigned long value), - TP_ARGS(ts, chan, chan_name, value), + TP_PROTO(unsigned long ts, unsigned int device, unsigned int chan, char *chan_name, unsigned long value), + TP_ARGS(ts, device, chan, chan_name, value), TP_STRUCT__entry( __field(unsigned long, ts ) __field(unsigned long, value ) + __field(unsigned int, device ) __field(unsigned int, chan ) __array(char, chan_name, PIXEL6_EMETER_CHAN_NAME_MAX_SIZE ) ), TP_fast_assign( - __entry->ts = ts; + __entry->ts = ts; + __entry->device = device; __entry->chan = chan; __entry->value = value; strlcpy(__entry->chan_name, chan_name, PIXEL6_EMETER_CHAN_NAME_MAX_SIZE); ), - TP_printk("ts=%lu chan=%u chan_name=%s value=%lu", - __entry->ts, __entry->chan, __entry->chan_name, __entry->value) + TP_printk("ts=%lu device=%u chan=%u chan_name=%s value=%lu", + __entry->ts, __entry->device, __entry->chan, __entry->chan_name, __entry->value) ); #endif /* _FTRACE_EVENTS_H */ diff --git a/lisa/_assets/kmodules/sched_tp/pixel6.c b/lisa/_assets/kmodules/sched_tp/pixel6.c index 291ca9bc0..5c43cf4c0 100644 --- a/lisa/_assets/kmodules/sched_tp/pixel6.c +++ b/lisa/_assets/kmodules/sched_tp/pixel6.c @@ -24,11 +24,15 @@ static PARSE_RESULT(int) parse_content(parse_buffer *); typedef struct sample { unsigned long ts; unsigned long value; + unsigned int device; unsigned int chan; char chan_name[PIXEL6_EMETER_CHAN_NAME_MAX_SIZE]; } sample_t; DEFINE_PARSE_RESULT_TYPE(sample_t); +typedef struct emeter_buffer_private { + unsigned int device; +} emeter_buffer_private_t; static struct file *open_file(int *error, const char *path, umode_t mode) { @@ -78,15 +82,20 @@ static int free_p6_emeter_data(struct p6_emeter_data *data) { return ret; } -static void process_content(unsigned char *content, size_t content_capacity) +static void process_content(unsigned char *content, size_t content_capacity, unsigned int device) { size_t size = strlen(content) + 1; + emeter_buffer_private_t private = { + .device = device, + }; parse_buffer input = { .data = (u8 *)content, .size = size, .capacity = content_capacity, + .private = &private, }; + PARSE_RESULT(int) res = parse_content(&input); if (!IS_SUCCESS(res)) pr_err("Failed to parse content\n"); @@ -103,7 +112,7 @@ static int p6_emeter_worker(void *data) { pr_err("Could not read " POWER_METER_SAMPLE_FILE ": %ld\n", count); } else { content[count] = '\0'; - process_content(content, ARRAY_SIZE(content)); + process_content(content, ARRAY_SIZE(content), 0); } /* Schedule the next run using the same delay as previously */ @@ -172,6 +181,8 @@ TAKEWHILE(u8, parse_name, parse_name_char); SEQUENCE(sample_t, parse_sample, ({ sample_t value; + value.device = ((emeter_buffer_private_t*)private)->device; + /* CH42 */ PARSE(parse_string, "CH"); value.chan = PARSE(parse_ulong); @@ -201,9 +212,9 @@ LEFT(sample_t, int, parse_sample_line, parse_sample, count_whitespaces) int process_sample(int nr, sample_t sample) { - /* pr_info("parsed: chan=%u, ts=%lu chan_name=%s value=%lu\n", sample.chan, */ + /* pr_info("parsed: device=%u chan=%u, ts=%lu chan_name=%s value=%lu\n", sample.device, sample.chan, */ /* sample.ts, sample.chan_name, sample.value); */ - trace_pixel6_emeter(sample.ts, sample.chan, sample.chan_name, sample.value); + trace_pixel6_emeter(sample.ts, sample.device, sample.chan, sample.chan_name, sample.value); return nr + 1; } -- GitLab From d91e8e4d4e2fc94a53d4badb3468ca78b098bfe3 Mon Sep 17 00:00:00 2001 From: Kajetan Puchalski Date: Wed, 12 Apr 2023 17:01:27 +0100 Subject: [PATCH 3/4] lisa._assets.kmodules.sched_tp: Extend pixel6_emeter to support all channels FEATURE The Pixel 6 power meter is split into two devices (device0 and device1) containing 8 power channels each. Under the current implementation, only device0 is being parsed by the lisa sched_tp parser for pixel6_emeter. This commit extends it to support both device files and all power channels in the process. Signed-off-by: Kajetan Puchalski --- lisa/_assets/kmodules/sched_tp/pixel6.c | 51 +++++++++++++++++-------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/lisa/_assets/kmodules/sched_tp/pixel6.c b/lisa/_assets/kmodules/sched_tp/pixel6.c index 5c43cf4c0..5ce37e673 100644 --- a/lisa/_assets/kmodules/sched_tp/pixel6.c +++ b/lisa/_assets/kmodules/sched_tp/pixel6.c @@ -16,8 +16,10 @@ #define POWER_METER_SAMPLING_RATE_MS 50 #define POWER_METER_SAMPLE_FILE_MAX_SIZE 1024 -#define POWER_METER_SAMPLE_FILE "/sys/bus/iio/devices/iio:device0/energy_value" -#define POWER_METER_RATE_FILE "/sys/bus/iio/devices/iio:device0/sampling_rate" +#define POWER_METER_SAMPLE_FILE_0 "/sys/bus/iio/devices/iio:device0/energy_value" +#define POWER_METER_RATE_FILE_0 "/sys/bus/iio/devices/iio:device0/sampling_rate" +#define POWER_METER_SAMPLE_FILE_1 "/sys/bus/iio/devices/iio:device1/energy_value" +#define POWER_METER_RATE_FILE_1 "/sys/bus/iio/devices/iio:device1/sampling_rate" static PARSE_RESULT(int) parse_content(parse_buffer *); @@ -64,10 +66,10 @@ static void write_str(struct file *file, char *str) } } - struct p6_emeter_data { struct work_item *work; - struct file *sample_file; + struct file *sample_file_0; + struct file *sample_file_1; }; static int free_p6_emeter_data(struct p6_emeter_data *data) { @@ -75,7 +77,8 @@ static int free_p6_emeter_data(struct p6_emeter_data *data) { if (data) { ret |= destroy_work(data->work); - close_file(&ret, data->sample_file); + close_file(&ret, data->sample_file_0); + close_file(&ret, data->sample_file_1); kfree(data); } @@ -101,19 +104,26 @@ static void process_content(unsigned char *content, size_t content_capacity, uns pr_err("Failed to parse content\n"); } -static int p6_emeter_worker(void *data) { - struct feature *feature = data; - struct p6_emeter_data *p6_emeter_data = feature->data; - char content[POWER_METER_SAMPLE_FILE_MAX_SIZE]; +static void read_and_process(char *buffer, unsigned int size, struct file *file, char *name, + unsigned int device) { ssize_t count = 0; - count = kernel_read(p6_emeter_data->sample_file, content, POWER_METER_SAMPLE_FILE_MAX_SIZE - 1, 0); + count = kernel_read(file, buffer, POWER_METER_SAMPLE_FILE_MAX_SIZE - 1, 0); if (count < 0 || count >= POWER_METER_SAMPLE_FILE_MAX_SIZE) { - pr_err("Could not read " POWER_METER_SAMPLE_FILE ": %ld\n", count); + pr_err("Could not read %s : %ld\n", name, count); } else { - content[count] = '\0'; - process_content(content, ARRAY_SIZE(content), 0); + buffer[count] = '\0'; + process_content(buffer, size, device); } +} + +static int p6_emeter_worker(void *data) { + struct feature *feature = data; + struct p6_emeter_data *p6_emeter_data = feature->data; + char content[POWER_METER_SAMPLE_FILE_MAX_SIZE]; + + read_and_process(content, ARRAY_SIZE(content), p6_emeter_data->sample_file_0, POWER_METER_SAMPLE_FILE_0, 0); + read_and_process(content, ARRAY_SIZE(content), p6_emeter_data->sample_file_1, POWER_METER_SAMPLE_FILE_1, 1); /* Schedule the next run using the same delay as previously */ return WORKER_SAME_DELAY; @@ -138,13 +148,22 @@ static int enable_p6_emeter(struct feature* feature) { /* Note that this is the hardware sampling rate. Software will only see *an updated value every 8 hardware periods */ - rate_file = open_file(&ret, POWER_METER_RATE_FILE, O_WRONLY); + rate_file = open_file(&ret, POWER_METER_RATE_FILE_0, O_WRONLY); write_str(rate_file, "500\n"); close_file(&ret, rate_file); HANDLE_ERR(ret); - sample_file = open_file(&ret, POWER_METER_SAMPLE_FILE, O_RDONLY); - data->sample_file = sample_file; + rate_file = open_file(&ret, POWER_METER_RATE_FILE_1, O_WRONLY); + write_str(rate_file, "500\n"); + close_file(&ret, rate_file); + HANDLE_ERR(ret); + + sample_file = open_file(&ret, POWER_METER_SAMPLE_FILE_0, O_RDONLY); + data->sample_file_0 = sample_file; + HANDLE_ERR(ret); + + sample_file = open_file(&ret, POWER_METER_SAMPLE_FILE_1, O_RDONLY); + data->sample_file_1 = sample_file; HANDLE_ERR(ret); data->work = start_work(p6_emeter_worker, msecs_to_jiffies(POWER_METER_SAMPLING_RATE_MS), feature); -- GitLab From 145feff736adaec18ea2d7f19c616ab9cf9af279 Mon Sep 17 00:00:00 2001 From: Kajetan Puchalski Date: Wed, 12 Apr 2023 17:17:23 +0100 Subject: [PATCH 4/4] lisa.analysis.pixel6: Add GPU to emeter channel names FEATURE Now that we have the GPU power meter available in pixel6_emeter, add it to EMETER_CHAN_NAMES to use with trace analysis functions too. Also since we have more than CPU clusters, prefix every cluster channel with CPU-* for clarity. Signed-off-by: Kajetan Puchalski --- lisa/analysis/pixel6.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lisa/analysis/pixel6.py b/lisa/analysis/pixel6.py index 09f85443b..78029998f 100644 --- a/lisa/analysis/pixel6.py +++ b/lisa/analysis/pixel6.py @@ -36,9 +36,10 @@ class Pixel6Analysis(TraceAnalysisBase): name = 'pixel6' EMETER_CHAN_NAMES = { - 'S4M_VDD_CPUCL0': 'little', - 'S3M_VDD_CPUCL1': 'mid', - 'S2M_VDD_CPUCL2': 'big', + 'S4M_VDD_CPUCL0': 'CPU-Little', + 'S3M_VDD_CPUCL1': 'CPU-Mid', + 'S2M_VDD_CPUCL2': 'CPU-Big', + 'S2S_VDD_G3D': 'GPU', } ############################################################################### -- GitLab