[FFmpeg-devel] [PATCHv3 0/6] RISC-V Linux perf run-time enablement

[FFmpeg-devel] [PATCHv3 0/6] RISC-V Linux perf run-time enablement

[Rémi Denis-Courmont]

	Hello,

This allows run-time fallback from Linux (or MacOS k)perf to the raw
cycle counter. The main purpose is to deal with RISC-V's hard
deprecation of RDCYCLE access from userspace, all the while preserving
compatibility with existing kernels which do not support Linux perf.
This is also usable as a baseline to fallback further to another
counter, presumably RDTIME (and Arm's equivalent).

Main change since version 2 is retaining inline AV_READ_TIME in
configuration where AV_READ_TIME was/is used.

----------------------------------------------------------------
Rémi Denis-Courmont (6):
      checkasm: make perf macros functional
      checkasm: use pointers for start/stop functions
      checkasm: remove unused variables
      checkasm: make bench clean-up also a function pointer
      checkasm: allow run-time fallback to AV_READ_TIME
      configure: enable Linux perf on RISC-V by default

 configure                 |  2 +-
 tests/checkasm/checkasm.c | 99 +++++++++++++++++++++++++++++++++--------------
 tests/checkasm/checkasm.h | 50 +++++++-----------------
 3 files changed, 85 insertions(+), 66 deletions(-)

-- 
雷米‧德尼-库尔蒙
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/6] 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 30e2dc6fed..63141e1f7a 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -514,11 +514,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);
@@ -652,6 +650,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 2/6] checkasm: use pointers for start/stop functions

[Rémi Denis-Courmont]

Linux and MacOS kernel performance APIs are not always available.
This introduces function pointers so that we can fall back to other
timing functions (in later changesets).

If AV_READ_TIME is the only configured timer, then this sticks to
inline assembler since there will be nothing to fall back to (or from),
and some people are concerned than an indirect function call is too
much overhead.
---
 tests/checkasm/checkasm.c | 25 +++++++++++++++++++++++--
 tests/checkasm/checkasm.h | 24 ++++++------------------
 2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index 63141e1f7a..933d85bac3 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
@@ -313,6 +321,8 @@ static struct {
     /* perf */
     int nop_time;
     int sysfd;
+    uint64_t (*start)(void);
+    uint64_t (*stop)(void);
 
     int cpu_flag;
     const char *cpu_flag_name;
@@ -513,6 +523,12 @@ static int measure_nop_time(void)
     uint16_t nops[10000];
     int i, nop_sum = 0;
     av_unused const int sysfd = state.sysfd;
+#if CONFIG_LINUX_PERF || CONFIG_MACOS_KPERF
+    struct {
+         uint64_t (*start)(void);
+         uint64_t (*stop)(void);
+    } p = { state.start, state.stop }, *perf = &p;
+#endif
 
     for (i = 0; i < 10000; i++) {
         uint64_t t = checkasm_bench_start();
@@ -650,7 +666,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;
 
@@ -659,7 +675,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;
@@ -686,12 +702,15 @@ static int bench_init_linux(void)
         perror("syscall");
         return -1;
     }
+    state.start = checkasm_bench_linux_perf_start;
+    state.stop = checkasm_bench_linux_perf_stop;
     return 0;
 }
 #elif CONFIG_MACOS_KPERF
 static int bench_init_kperf(void)
 {
     ff_kperf_init();
+    state.start = state.stop = ff_kperf_cycles;
     return 0;
 }
 #else
@@ -882,6 +901,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 = state.start;
+    perf->stop = state.stop;
     return perf;
 }
 
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index 8a62b98f3e..3b3a1ab35b 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,16 +227,13 @@ 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()
+#if CONFIG_LINUX_PERF || CONFIG_MACOS_KPERF
+#define checkasm_bench_start() (perf->start)()
+#define checkasm_bench_stop()  (perf->stop)()
 #elif defined (AV_READ_TIME)
 #define checkasm_bench_start() AV_READ_TIME()
 #define checkasm_bench_stop()  AV_READ_TIME()
@@ -258,7 +246,7 @@ uint64_t checkasm_bench_linux_perf_stop(void);
 #define bench_new(...)\
     do {\
         if (checkasm_bench_func()) {\
-            struct CheckasmPerf *perf = checkasm_get_perf_context();\
+            struct CheckasmPerf *restrict perf = checkasm_get_perf_context();\
             av_unused const int sysfd = perf->sysfd;\
             func_type *tfunc = func_new;\
             uint64_t tsum = 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 3/6] 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 933d85bac3..987162e6b5 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;
 #if CONFIG_LINUX_PERF || CONFIG_MACOS_KPERF
     struct {
          uint64_t (*start)(void);
@@ -900,7 +899,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 = state.start;
     perf->stop = state.stop;
     return perf;
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index 3b3a1ab35b..80e6561e69 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);
@@ -247,7 +246,6 @@ typedef struct CheckasmPerf {
     do {\
         if (checkasm_bench_func()) {\
             struct CheckasmPerf *restrict perf = checkasm_get_perf_context();\
-            av_unused const int sysfd = perf->sysfd;\
             func_type *tfunc = func_new;\
             uint64_t tsum = 0;\
             int ti, tcount = 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 4/6] checkasm: make bench clean-up also a function pointer

[Rémi Denis-Courmont]

There are no performance concerns with cleaning up, so might as well
indirect it. This incidentally fixes a corner case (and
inconsequential) bug whereby the Linux perf descriptor is zero and did
not 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 987162e6b5..fad660a924 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -320,9 +320,12 @@ static struct {
 
     /* perf */
     int nop_time;
+#if CONFIG_LINUX_PERF
     int sysfd;
+#endif
     uint64_t (*start)(void);
     uint64_t (*stop)(void);
+    void (*close)(void);
 
     int cpu_flag;
     const char *cpu_flag_name;
@@ -683,6 +686,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 = {
@@ -703,6 +711,7 @@ static int bench_init_linux(void)
     }
     state.start = checkasm_bench_linux_perf_start;
     state.stop = checkasm_bench_linux_perf_stop;
+    state.close = checkasm_bench_linux_perf_close;
     return 0;
 }
 #elif CONFIG_MACOS_KPERF
@@ -742,14 +751,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
-}
-
 static int usage(const char *path)
 {
     fprintf(stderr,
@@ -816,7 +817,8 @@ int main(int argc, char *argv[])
     }
 
     destroy_func_tree(state.funcs);
-    bench_uninit();
+    if (state.close != NULL)
+        state.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 5/6] 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, benchmarking would breakdown in upcoming Linux RISC-V kernel
releases. This also paves the way for falling back from the cycle
counter on Arm or RISC-V, whence they too are disabled.
---
 tests/checkasm/checkasm.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index fad660a924..3c8aa8631e 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -714,37 +714,43 @@ static int bench_init_linux(void)
     state.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();
     state.start = state.stop = ff_kperf_cycles;
     return 0;
 }
-#else
+#endif
+#ifdef AV_READ_TIME
 static int bench_init_ffmpeg(void)
 {
-#ifdef AV_READ_TIME
     printf("benchmarking with native FFmpeg timers\n");
+    state.start = state.stop = AV_READ_TIME;
     return 0;
-#else
-    fprintf(stderr, "checkasm: --bench is not supported on your system\n");
-    return -1;
-#endif
 }
 #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 6/6] 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 b018abf139..c60e8d3438 100755
--- a/configure
+++ b/configure
@@ -5753,7 +5753,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".

Re: [FFmpeg-devel] [PATCH 2/6] checkasm: use pointers for start/stop functions

[Lynne]

Jul 19, 2023, 21:56 by remi@remlab.net:

> Linux and MacOS kernel performance APIs are not always available.
> This introduces function pointers so that we can fall back to other
> timing functions (in later changesets).
>
> If AV_READ_TIME is the only configured timer, then this sticks to
> inline assembler since there will be nothing to fall back to (or from),
> and some people are concerned than an indirect function call is too
> much overhead.
> ---
> tests/checkasm/checkasm.c | 25 +++++++++++++++++++++++--
> tests/checkasm/checkasm.h | 24 ++++++------------------
> 2 files changed, 29 insertions(+), 20 deletions(-)
>
> diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
> index 63141e1f7a..933d85bac3 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
> @@ -313,6 +321,8 @@ static struct {
> /* perf */
> int nop_time;
> int sysfd;
> +    uint64_t (*start)(void);
> +    uint64_t (*stop)(void);
>

Could these be ifdef's and macro'd out on x86/arm?
Perhaps behind a CONFIG_CPU_TIMERS, which would
be disabled if linux perf is active.
_______________________________________________
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".