On Wed, Feb 08, 2023 at 04:20:13PM -0800, Andrii Nakryiko wrote: SNIP > > > diff --git a/tools/testing/selftests/bpf/progs/cb_refs.c b/tools/testing/selftests/bpf/progs/cb_refs.c > > > index 7653df1bc787..823901c1b839 100644 > > > --- a/tools/testing/selftests/bpf/progs/cb_refs.c > > > +++ b/tools/testing/selftests/bpf/progs/cb_refs.c > > > @@ -2,6 +2,7 @@ > > > #include <vmlinux.h> > > > #include <bpf/bpf_tracing.h> > > > #include <bpf/bpf_helpers.h> > > > +#include "bpf_testmod/bpf_testmod_kfunc.h" > > > > Feel free to ignore if you disagree, but here and elsewhere, should we > > do this: > > > > #include <bpf_testmod/bpf_testmod_kfunc.h> > > > > rather than using #include "bpf_testmod/bpf_testmod_kfunc.h". Doesn't > > matter much, but IMO it's just slightly more readable to use the <> to > > show that we're relying on -I rather than expecting > > bpf_testmod/bpf_testmod_kfunc.h to be found at a path relative to the > > progs. #include "bpf_misc.h" makes more sense because it really is > > located in the progs/ directory. > > We do <> for headers that are expected to be installed in the system > (even if we cheat with -I sometimes). But in this case it's a local > header, so using "" makes more sense to me. But shouldn't it be > "../bpf_testmod/bpf_testmod_kfunc.h"? I think we have -I<..selftests/bpf> so it works.. but right, we want to show it's local header, so "../bpf_testmod/bpf_testmod_kfunc.h" makes sense to me jirka