Re: [PATCH v2 bpf-next 1/4] bpf: enable program stats

On Sat, Feb 23, 2019 at 10:36:48AM -0800, Eric Dumazet wrote:

On 02/23/2019 09:44 AM, Alexei Starovoitov wrote:

...

quoted

 
-#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 ?

No. It cannot reenter for the same program.
socket filter prog can be interrupted by kprobe prog, but they are different
progs and different stats.