[FFmpeg-devel] [PATCHv2 0/7] checkasm: RISC-V Linux perf enablement

[FFmpeg-devel] [PATCHv2 0/7] checkasm: RISC-V Linux perf enablement

[Rémi Denis-Courmont]

Changes since v1:
- Purge additional no longer used variable.
- Deinitialise benchmarking through function pointer.
- Cache function pointers outside bench loop.

-- 
レミ・デニ-クールモン
http://www.remlab.net/
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/7] checkasm: improve Linux perf error message

[Rémi Denis-Courmont]

Report the failing system call name, as is convention, rather than just
a rather unhelpful "syscall".
---
 tests/checkasm/checkasm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index 4311a8ffcb..8702c9a30a 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -666,7 +666,7 @@ static int bench_init_linux(void)
 
     state.sysfd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, 0);
     if (state.sysfd == -1) {
-        perror("syscall");
+        perror("perf_event_open");
         return -1;
     }
     return 0;
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/7] checkasm: make perf macros functional

[Rémi Denis-Courmont]

This converts the bench/perf start/stop macros into functional macros,
and for that to work, take the Linux perf code out of line.
---
 tests/checkasm/checkasm.c | 24 ++++++++++++++++++++----
 tests/checkasm/checkasm.h | 36 +++++++++++++-----------------------
 2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index 8702c9a30a..d8f571f301 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -513,11 +513,9 @@ static int measure_nop_time(void)
     int i, nop_sum = 0;
     av_unused const int sysfd = state.sysfd;
 
-    uint64_t t = 0;
     for (i = 0; i < 10000; i++) {
-        PERF_START(t);
-        PERF_STOP(t);
-        nops[i] = t;
+        uint64_t t = checkasm_bench_start();
+        nops[i] = checkasm_bench_stop() - t;
     }
 
     qsort(nops, 10000, sizeof(uint16_t), cmp_nop);
@@ -651,6 +649,24 @@ static void print_cpu_name(void)
 }
 
 #if CONFIG_LINUX_PERF
+uint64_t checkasm_bench_linux_perf_start(void)
+{
+    int fd = state.sysfd;
+
+    ioctl(fd, PERF_EVENT_IOC_RESET, 0);
+    ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);
+    return 0;
+}
+
+uint64_t checkasm_bench_linux_perf_stop(void)
+{
+    uint64_t t;
+    int fd = state.sysfd;
+
+    ioctl(fd, PERF_EVENT_IOC_DISABLE, 0);
+    return (read(fd, &t, sizeof(t)) == sizeof (t)) ? t : 0;
+}
+
 static int bench_init_linux(void)
 {
     struct perf_event_attr attr = {
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index 117d4dd35c..8a62b98f3e 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -238,25 +238,20 @@ typedef struct CheckasmPerf {
     int iterations;
 } CheckasmPerf;
 
-#if defined(AV_READ_TIME) || CONFIG_LINUX_PERF || CONFIG_MACOS_KPERF
-
 #if CONFIG_LINUX_PERF
-#define PERF_START(t) do {                              \
-    ioctl(sysfd, PERF_EVENT_IOC_RESET, 0);              \
-    ioctl(sysfd, PERF_EVENT_IOC_ENABLE, 0);             \
-} while (0)
-#define PERF_STOP(t) do {                               \
-    int ret;                                            \
-    ioctl(sysfd, PERF_EVENT_IOC_DISABLE, 0);            \
-    ret = read(sysfd, &t, sizeof(t));                   \
-    (void)ret;                                          \
-} while (0)
+uint64_t checkasm_bench_linux_perf_start(void);
+uint64_t checkasm_bench_linux_perf_stop(void);
+#define checkasm_bench_start() checkasm_bench_linux_perf_start()
+#define checkasm_bench_stop()  checkasm_bench_linux_perf_stop()
 #elif CONFIG_MACOS_KPERF
-#define PERF_START(t) t = ff_kperf_cycles()
-#define PERF_STOP(t)  t = ff_kperf_cycles() - t
+#define checkasm_bench_start() ff_kperf_cycles()
+#define checkasm_bench_stop()  ff_kperf_cycles()
+#elif defined (AV_READ_TIME)
+#define checkasm_bench_start() AV_READ_TIME()
+#define checkasm_bench_stop()  AV_READ_TIME()
 #else
-#define PERF_START(t) t = AV_READ_TIME()
-#define PERF_STOP(t)  t = AV_READ_TIME() - t
+#define checkasm_bench_start() UINT64_C(0)
+#define checkasm_bench_stop()  UINT64_C(0)
 #endif
 
 /* Benchmark the function */
@@ -270,12 +265,12 @@ typedef struct CheckasmPerf {
             int ti, tcount = 0;\
             uint64_t t = 0; \
             for (ti = 0; ti < BENCH_RUNS; ti++) {\
-                PERF_START(t);\
+                t = checkasm_bench_start(); \
                 tfunc(__VA_ARGS__);\
                 tfunc(__VA_ARGS__);\
                 tfunc(__VA_ARGS__);\
                 tfunc(__VA_ARGS__);\
-                PERF_STOP(t);\
+                t = checkasm_bench_stop() - t;\
                 if (t*tcount <= tsum*4 && ti > 0) {\
                     tsum += t;\
                     tcount++;\
@@ -286,11 +281,6 @@ typedef struct CheckasmPerf {
             perf->iterations++;\
         }\
     } while (0)
-#else
-#define bench_new(...) while(0)
-#define PERF_START(t)  while(0)
-#define PERF_STOP(t)   while(0)
-#endif
 
 #define DECL_CHECKASM_CHECK_FUNC(type) \
 int checkasm_check_##type(const char *const file, const int line, \
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/7] checkasm: use pointers for start/stop functions

[Rémi Denis-Courmont]

This makes all calls to the bench start and stop functions via
function pointers. While the primary goal is to support run-time
selection of the performance measurement back-end in later commits,
this has the side benefit of containing platform dependencies in to
checkasm.c and out of checkasm.h.
---
 tests/checkasm/checkasm.c | 33 ++++++++++++++++++++++++++++-----
 tests/checkasm/checkasm.h | 33 ++++++---------------------------
 2 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index d8f571f301..e921257257 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -57,6 +57,14 @@
 #if HAVE_UNISTD_H
 #include <unistd.h>
 #endif
+#if CONFIG_LINUX_PERF
+#include <sys/ioctl.h>
+#include <asm/unistd.h>
+#include <linux/perf_event.h>
+#endif
+#if CONFIG_MACOS_KPERF
+#include "libavutil/macos_kperf.h"
+#endif
 
 #if !HAVE_ISATTY
 #define isatty(fd) 1
@@ -506,6 +514,9 @@ static int cmp_nop(const void *a, const void *b)
     return *(const uint16_t*)a - *(const uint16_t*)b;
 }
 
+static uint64_t (*checkasm_bench_start)(void);
+static uint64_t (*checkasm_bench_stop)(void);
+
 /* Measure the overhead of the timing code (in decicycles) */
 static int measure_nop_time(void)
 {
@@ -649,7 +660,7 @@ static void print_cpu_name(void)
 }
 
 #if CONFIG_LINUX_PERF
-uint64_t checkasm_bench_linux_perf_start(void)
+static uint64_t checkasm_bench_linux_perf_start(void)
 {
     int fd = state.sysfd;
 
@@ -658,7 +669,7 @@ uint64_t checkasm_bench_linux_perf_start(void)
     return 0;
 }
 
-uint64_t checkasm_bench_linux_perf_stop(void)
+static uint64_t checkasm_bench_linux_perf_stop(void)
 {
     uint64_t t;
     int fd = state.sysfd;
@@ -685,24 +696,34 @@ static int bench_init_linux(void)
         perror("perf_event_open");
         return -1;
     }
+    checkasm_bench_start = checkasm_bench_linux_perf_start;
+    checkasm_bench_stop = checkasm_bench_linux_perf_stop;
     return 0;
 }
 #elif CONFIG_MACOS_KPERF
 static int bench_init_kperf(void)
 {
     ff_kperf_init();
+    checkasm_bench_start = checkasm_bench_stop = ff_kperf_cycles;
     return 0;
 }
-#else
+#elif defined (AV_READ_TIME)
+static uint64_t ff_read_time(void)
+{
+    return AV_READ_TIME();
+}
+
 static int bench_init_ffmpeg(void)
 {
-#ifdef AV_READ_TIME
     printf("benchmarking with native FFmpeg timers\n");
+    checkasm_bench_start = checkasm_bench_stop = ff_read_time;
     return 0;
+}
 #else
+static int bench_init_ffmpeg(void)
+{
     fprintf(stderr, "checkasm: --bench is not supported on your system\n");
     return -1;
-#endif
 }
 #endif
 
@@ -869,6 +890,8 @@ CheckasmPerf *checkasm_get_perf_context(void)
     CheckasmPerf *perf = &state.current_func_ver->perf;
     memset(perf, 0, sizeof(*perf));
     perf->sysfd = state.sysfd;
+    perf->start = checkasm_bench_start;
+    perf->stop = checkasm_bench_start;
     return perf;
 }
 
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index 8a62b98f3e..8f966f49af 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -26,15 +26,6 @@
 #include <stdint.h>
 #include "config.h"
 
-#if CONFIG_LINUX_PERF
-#include <unistd.h> // read(3)
-#include <sys/ioctl.h>
-#include <asm/unistd.h>
-#include <linux/perf_event.h>
-#elif CONFIG_MACOS_KPERF
-#include "libavutil/macos_kperf.h"
-#endif
-
 #include "libavutil/avstring.h"
 #include "libavutil/cpu.h"
 #include "libavutil/internal.h"
@@ -236,41 +227,29 @@ typedef struct CheckasmPerf {
     int sysfd;
     uint64_t cycles;
     int iterations;
+    uint64_t (*start)(void);
+    uint64_t (*stop)(void);
 } CheckasmPerf;
 
-#if CONFIG_LINUX_PERF
-uint64_t checkasm_bench_linux_perf_start(void);
-uint64_t checkasm_bench_linux_perf_stop(void);
-#define checkasm_bench_start() checkasm_bench_linux_perf_start()
-#define checkasm_bench_stop()  checkasm_bench_linux_perf_stop()
-#elif CONFIG_MACOS_KPERF
-#define checkasm_bench_start() ff_kperf_cycles()
-#define checkasm_bench_stop()  ff_kperf_cycles()
-#elif defined (AV_READ_TIME)
-#define checkasm_bench_start() AV_READ_TIME()
-#define checkasm_bench_stop()  AV_READ_TIME()
-#else
-#define checkasm_bench_start() UINT64_C(0)
-#define checkasm_bench_stop()  UINT64_C(0)
-#endif
-
 /* Benchmark the function */
 #define bench_new(...)\
     do {\
         if (checkasm_bench_func()) {\
             struct CheckasmPerf *perf = checkasm_get_perf_context();\
             av_unused const int sysfd = perf->sysfd;\
+            uint64_t (*const start)(void) = perf->start;\
+            uint64_t (*const stop)(void) = perf->stop;\
             func_type *tfunc = func_new;\
             uint64_t tsum = 0;\
             int ti, tcount = 0;\
             uint64_t t = 0; \
             for (ti = 0; ti < BENCH_RUNS; ti++) {\
-                t = checkasm_bench_start(); \
+                t = start(); \
                 tfunc(__VA_ARGS__);\
                 tfunc(__VA_ARGS__);\
                 tfunc(__VA_ARGS__);\
                 tfunc(__VA_ARGS__);\
-                t = checkasm_bench_stop() - t;\
+                t = stop() - t;\
                 if (t*tcount <= tsum*4 && ti > 0) {\
                     tsum += t;\
                     tcount++;\
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/7] checkasm: remove unused variables

[Rémi Denis-Courmont]

---
 tests/checkasm/checkasm.c | 2 --
 tests/checkasm/checkasm.h | 2 --
 2 files changed, 4 deletions(-)

diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index e921257257..c90f361ff7 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -522,7 +522,6 @@ static int measure_nop_time(void)
 {
     uint16_t nops[10000];
     int i, nop_sum = 0;
-    av_unused const int sysfd = state.sysfd;
 
     for (i = 0; i < 10000; i++) {
         uint64_t t = checkasm_bench_start();
@@ -889,7 +888,6 @@ CheckasmPerf *checkasm_get_perf_context(void)
 {
     CheckasmPerf *perf = &state.current_func_ver->perf;
     memset(perf, 0, sizeof(*perf));
-    perf->sysfd = state.sysfd;
     perf->start = checkasm_bench_start;
     perf->stop = checkasm_bench_start;
     return perf;
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index 8f966f49af..6a3f826a7f 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -224,7 +224,6 @@ void *checkasm_get_wrapper(void);
 #endif
 
 typedef struct CheckasmPerf {
-    int sysfd;
     uint64_t cycles;
     int iterations;
     uint64_t (*start)(void);
@@ -236,7 +235,6 @@ typedef struct CheckasmPerf {
     do {\
         if (checkasm_bench_func()) {\
             struct CheckasmPerf *perf = checkasm_get_perf_context();\
-            av_unused const int sysfd = perf->sysfd;\
             uint64_t (*const start)(void) = perf->start;\
             uint64_t (*const stop)(void) = perf->stop;\
             func_type *tfunc = func_new;\
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/7] checkasm: make bench clean-up also a function pointer

[Rémi Denis-Courmont]

Because we might as well at this point. This incidentally fixes a corner
case bug that state.sys_fd == 0 could be either the initial value from
.bss or a valid file descriptor (in which case it didn't get closed).
---
 tests/checkasm/checkasm.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index c90f361ff7..2e327de3a4 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -319,7 +319,9 @@ static struct {
 
     /* perf */
     int nop_time;
+#ifdef CONFIG_LINUX_PERF
     int sysfd;
+#endif
 
     int cpu_flag;
     const char *cpu_flag_name;
@@ -516,6 +518,7 @@ static int cmp_nop(const void *a, const void *b)
 
 static uint64_t (*checkasm_bench_start)(void);
 static uint64_t (*checkasm_bench_stop)(void);
+static void (*checkasm_bench_close)(void);
 
 /* Measure the overhead of the timing code (in decicycles) */
 static int measure_nop_time(void)
@@ -677,6 +680,11 @@ static uint64_t checkasm_bench_linux_perf_stop(void)
     return (read(fd, &t, sizeof(t)) == sizeof (t)) ? t : 0;
 }
 
+static void checkasm_bench_linux_perf_close(void)
+{
+    close(state.sysfd);
+}
+
 static int bench_init_linux(void)
 {
     struct perf_event_attr attr = {
@@ -697,6 +705,7 @@ static int bench_init_linux(void)
     }
     checkasm_bench_start = checkasm_bench_linux_perf_start;
     checkasm_bench_stop = checkasm_bench_linux_perf_stop;
+    checkasm_bench_close = checkasm_bench_linux_perf_close;
     return 0;
 }
 #elif CONFIG_MACOS_KPERF
@@ -743,14 +752,6 @@ static int bench_init(void)
     return 0;
 }
 
-static void bench_uninit(void)
-{
-#if CONFIG_LINUX_PERF
-    if (state.sysfd > 0)
-        close(state.sysfd);
-#endif
-}
-
 int main(int argc, char *argv[])
 {
     unsigned int seed = av_get_random_seed();
@@ -805,7 +806,8 @@ int main(int argc, char *argv[])
     }
 
     destroy_func_tree(state.funcs);
-    bench_uninit();
+    if (checkasm_bench_close != NULL)
+        checkasm_bench_close();
     return ret;
 }
 
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 6/7] checkasm: allow run-time fallback to AV_READ_TIME

[Rémi Denis-Courmont]

On Linux RISC-V, depending on kernel version and configuration, the
cycle counters may be available:
- via Linux perf (as on Arm),
- directly with RDCYCLE (which is considered legacy),
- neither (if admistratively disabled), or
- or both (in legacy compatibility mode).

This allows to try Linux perf, with a fallback to RDCYCLE. Without
this, bench will stop working in upcoming kernel releases.
---
 tests/checkasm/checkasm.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index 2e327de3a4..ad2dc9ab5a 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -708,7 +708,8 @@ static int bench_init_linux(void)
     checkasm_bench_close = checkasm_bench_linux_perf_close;
     return 0;
 }
-#elif CONFIG_MACOS_KPERF
+#endif
+#if CONFIG_MACOS_KPERF
 static int bench_init_kperf(void)
 {
     ff_kperf_init();
@@ -727,25 +728,27 @@ static int bench_init_ffmpeg(void)
     checkasm_bench_start = checkasm_bench_stop = ff_read_time;
     return 0;
 }
-#else
-static int bench_init_ffmpeg(void)
-{
-    fprintf(stderr, "checkasm: --bench is not supported on your system\n");
-    return -1;
-}
 #endif
 
 static int bench_init(void)
 {
+    int ret = -1;
 #if CONFIG_LINUX_PERF
-    int ret = bench_init_linux();
-#elif CONFIG_MACOS_KPERF
-    int ret = bench_init_kperf();
-#else
-    int ret = bench_init_ffmpeg();
+    if (ret < 0)
+        ret = bench_init_linux();
+#endif
+#if CONFIG_MACOS_KPERF
+    if (ret < 0)
+        ret = bench_init_kperf();
 #endif
+#ifdef AV_READ_TIME
     if (ret < 0)
-        return ret;
+        ret = bench_init_ffmpeg();
+#endif
+    if (ret < 0) {
+        fputs("checkasm: --bench is not supported on your system\n", stderr);
+        return -1;
+    }
 
     state.nop_time = measure_nop_time();
     printf("nop: %d.%d\n", state.nop_time/10, state.nop_time%10);
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 7/7] configure: enable Linux perf on RISC-V by default

[Rémi Denis-Courmont]

Now that checkasm can automatically fall back to RDCYCLE, it is safe to
enable this for forward compatibility with kernel versions mitigating
the "Cycle Drift" side channel attack.

Where available, this should also reduce measurement noise, since
kernel and hypervisor cycles should no longer be counted.
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 0ab0761011..3be074e15b 100755
--- a/configure
+++ b/configure
@@ -5754,7 +5754,7 @@ case $target_os in
         ;;
     linux)
         enable section_data_rel_ro
-        enabled_any arm aarch64 && enable_weak linux_perf
+        enabled_any arm aarch64 riscv && enable_weak linux_perf
         ;;
     irix*)
         target_os=irix
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".