On Wed, Nov 18, 2020 at 10:19 AM Martin KaFai Lau <kafai@xxxxxx> wrote:
>
> On Tue, Nov 17, 2020 at 02:56:36PM +0000, Daniel T. Lee wrote:
> > Under the samples/bpf directory, similar tracing helpers are
> > fragmented around. To keep consistent of tracing programs, this commit
> > moves the helper and define locations to increase the reuse of each
> > helper function.
> >
> > Signed-off-by: Daniel T. Lee <danieltimlee@xxxxxxxxx>
> >
> > ---
> > samples/bpf/Makefile | 2 +-
> > samples/bpf/hbm.c | 51 ++++-----------------
> > tools/testing/selftests/bpf/trace_helpers.c | 33 ++++++++++++-
> > tools/testing/selftests/bpf/trace_helpers.h | 3 ++
> > 4 files changed, 45 insertions(+), 44 deletions(-)
> >
> > [...]
> >
> > -#define DEBUGFS "/sys/kernel/debug/tracing/"
> Is this change needed?
This macro can be used in other samples such as the 4th' patch of this
patchset, task_fd_query.
> > -
> > #define MAX_SYMS 300000
> > static struct ksym syms[MAX_SYMS];
> > static int sym_cnt;
> > @@ -136,3 +134,34 @@ void read_trace_pipe(void)
> > }
> > }
> > }
> > +
> > +void read_trace_pipe2(char *filename)
> > +{
> > + int trace_fd;
> > + FILE *outf;
> > +
> > + trace_fd = open(DEBUGFS "trace_pipe", O_RDONLY, 0);
> > + if (trace_fd < 0) {
> > + printf("Error opening trace_pipe\n");
> > + return;
> > + }
> > +
> > + outf = fopen(filename, "w");
> > + if (!outf)
> > + printf("Error creating %s\n", filename);
> > +
> > + while (1) {
> > + static char buf[4096];
> > + ssize_t sz;
> > +
> > + sz = read(trace_fd, buf, sizeof(buf) - 1);
> > + if (sz > 0) {
> > + buf[sz] = 0;
> > + puts(buf);
> > + if (outf) {
> > + fprintf(outf, "%s\n", buf);
> > + fflush(outf);
> > + }
> > + }
> > + }
> It needs a fclose().
>
> IIUC, this function will never return. I am not sure
> this is something that should be made available to selftests.
Actually, read_trace_pipe and read_trace_pipe2 are helpers that are
only used under samples directory. Since these helpers are not used
in selftests, What do you think about moving these helpers to
something like samples/bpf/trace_pipe.h?
Thanks for your time and effort for the review.
--
Best,
Daniel T. Lee
