On Sat, Feb 23, 2019 at 10:36:48AM -0800, Eric Dumazet wrote:
>
>
> 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 ?
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.
