Re: [PATCH v2 bpf-next 1/4] bpf: enable program stats
From: Eric Dumazet <hidden>
Date: 2019-02-23 18:36:53
Also in:
bpf
On 02/23/2019 09:44 AM, Alexei Starovoitov wrote: ...
-#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
+DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
+
+#define BPF_PROG_RUN(prog, ctx) ({ \
+ u32 ret; \
+ cant_sleep(); \
+ if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
+ struct bpf_prog_stats *stats; \
+ u64 start = sched_clock(); \
+ ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
+ stats = this_cpu_ptr(prog->aux->stats); \
+ u64_stats_update_begin(&stats->syncp); \
+ stats->cnt++; \
+ stats->nsecs += sched_clock() - start; \
+ u64_stats_update_end(&stats->syncp); \
+ } else { \
+ ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
+ } \
+ ret; })
It seems a cpu running there could still be interrupted (by an interrupt)
and re-enter this section ?
If yes, u64_stats_update_begin() and u64_stats_update_end() are unsafe (on 32bit arches)
u64_stats_update_{begin|end}() assume proper locking, since they use a simple increment.
But then, even on 64bit arches, the stats->{cnt|nsecs} updates are unsafe ?