| Message ID | 20250228191220.1488438-1-eddyz87@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | veristat: @files-list.txt notation for object files list | expand |
On Fri, Feb 28, 2025 at 11:13 AM Eduard Zingerman <eddyz87@gmail.com> wrote: > > A few small veristat improvements: > - It is possible to hit command line parameters number limit, > e.g. when running veristat for all object files generated for > test_progs. This patch-set adds an option to read objects files list > from a file. > - Correct usage of strerror() function. > - Avoid printing log lines to CSV output. > All makes sense, and superficially LGTM, but I'd like Mykyta to take a look when he gets a chance, as he's been working with veristat quite a lot recently. One thing I wanted to propose/ask. Do you think it would be useful to allow <object>:<program> pattern to be specified to allow picking just one program out of the object file? I normally do `veristat <object> -f<program>` for this, but being able to do `veristat <obj1>:<prog1> <obj2>:<prog2> ...` seems useful, no? (-f<program> would apply to all objects, btw, which isn't a big problem in practice, but still). Oh, and we could allow globbing in `veristat <obj>:<blah*>`. Thoughts? > Eduard Zingerman (3): > veristat: @files-list.txt notation for object files list > veristat: strerror expects positive number (errno) > veristat: report program type guess results to sdterr > > tools/testing/selftests/bpf/veristat.c | 70 +++++++++++++++++++++----- > 1 file changed, 57 insertions(+), 13 deletions(-) > > -- > 2.48.1 >
On Fri, 2025-02-28 at 14:47 -0800, Andrii Nakryiko wrote: > On Fri, Feb 28, 2025 at 11:13 AM Eduard Zingerman <eddyz87@gmail.com> wrote: > > > > A few small veristat improvements: > > - It is possible to hit command line parameters number limit, > > e.g. when running veristat for all object files generated for > > test_progs. This patch-set adds an option to read objects files list > > from a file. > > - Correct usage of strerror() function. > > - Avoid printing log lines to CSV output. > > > > All makes sense, and superficially LGTM, but I'd like Mykyta to take a > look when he gets a chance, as he's been working with veristat quite a > lot recently. Thanks. I'll wait for Mykytas comments before sending v2 with -err. > One thing I wanted to propose/ask. Do you think it would be useful to > allow <object>:<program> pattern to be specified to allow picking just > one program out of the object file? I normally do `veristat <object> > -f<program>` for this, but being able to do `veristat <obj1>:<prog1> > <obj2>:<prog2> ...` seems useful, no? (-f<program> would apply to all > objects, btw, which isn't a big problem in practice, but still). Oh, > and we could allow globbing in `veristat <obj>:<blah*>`. > > Thoughts? Tbh I don't remember myself ever needing this, -f was sufficient. Every time I used -f, it was to do <object>:<program> for a single program. On the other hand, this looks like a nice generalization. This does not seem to be too complicated, so I'd say lets add it, the use case will find us eventually. One thing I do want is multi-threading. E.g. it takes about 2 minutes to process all .bpf.o from selftests/bpf/cpuv4/, and it can be slashed to 10s of seconds. Per-object this should be straightforward. Per-program this would need to wait for Mykyta's work on prepare object, as far as I understand. I can add the per-object version over the weekend if you are ok with such granularity.
On Fri, Feb 28, 2025 at 3:00 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > On Fri, 2025-02-28 at 14:47 -0800, Andrii Nakryiko wrote: > > On Fri, Feb 28, 2025 at 11:13 AM Eduard Zingerman <eddyz87@gmail.com> wrote: > > > > > > A few small veristat improvements: > > > - It is possible to hit command line parameters number limit, > > > e.g. when running veristat for all object files generated for > > > test_progs. This patch-set adds an option to read objects files list > > > from a file. > > > - Correct usage of strerror() function. > > > - Avoid printing log lines to CSV output. > > > > > > > All makes sense, and superficially LGTM, but I'd like Mykyta to take a > > look when he gets a chance, as he's been working with veristat quite a > > lot recently. > > Thanks. I'll wait for Mykytas comments before sending v2 with -err. > > > One thing I wanted to propose/ask. Do you think it would be useful to > > allow <object>:<program> pattern to be specified to allow picking just > > one program out of the object file? I normally do `veristat <object> > > -f<program>` for this, but being able to do `veristat <obj1>:<prog1> > > <obj2>:<prog2> ...` seems useful, no? (-f<program> would apply to all > > objects, btw, which isn't a big problem in practice, but still). Oh, > > and we could allow globbing in `veristat <obj>:<blah*>`. > > > > Thoughts? > > Tbh I don't remember myself ever needing this, -f was sufficient. > Every time I used -f, it was to do <object>:<program> for a single program. > On the other hand, this looks like a nice generalization. > This does not seem to be too complicated, so I'd say lets add it, > the use case will find us eventually. > > One thing I do want is multi-threading. > E.g. it takes about 2 minutes to process all .bpf.o from > selftests/bpf/cpuv4/, and it can be slashed to 10s of seconds. > Per-object this should be straightforward. > Per-program this would need to wait for Mykyta's work on prepare object, > as far as I understand. > I can add the per-object version over the weekend if you are ok with > such granularity. > Yeah, I was hoping for bpf_object__prepare() to be used by veristat to speed up mass-processing. I think it's fine to parallelize, but this will be awkward with verbose/error logging, so think how you'll handle that, ok? Ideally we'll parallelize at program level, so you can start with per-object parallelism, but just anticipate that it will actually be prepared object + bpf_program eventually. Just write it in the way that would accommodate that easily, once we have all the pieces in libbpf.
On 28/02/2025 19:12, Eduard Zingerman wrote: > A few small veristat improvements: > - It is possible to hit command line parameters number limit, > e.g. when running veristat for all object files generated for > test_progs. This patch-set adds an option to read objects files list > from a file. > - Correct usage of strerror() function. > - Avoid printing log lines to CSV output. > > Eduard Zingerman (3): > veristat: @files-list.txt notation for object files list > veristat: strerror expects positive number (errno) > veristat: report program type guess results to sdterr > > tools/testing/selftests/bpf/veristat.c | 70 +++++++++++++++++++++----- > 1 file changed, 57 insertions(+), 13 deletions(-) Patch set looks good to me. Acked-by: Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>
On Fri, 2025-02-28 at 23:33 +0000, Mykyta Yatsenko wrote: > On 28/02/2025 19:12, Eduard Zingerman wrote: > > A few small veristat improvements: > > - It is possible to hit command line parameters number limit, > > e.g. when running veristat for all object files generated for > > test_progs. This patch-set adds an option to read objects files list > > from a file. > > - Correct usage of strerror() function. > > - Avoid printing log lines to CSV output. > > > > Eduard Zingerman (3): > > veristat: @files-list.txt notation for object files list > > veristat: strerror expects positive number (errno) > > veristat: report program type guess results to sdterr > > > > tools/testing/selftests/bpf/veristat.c | 70 +++++++++++++++++++++----- > > 1 file changed, 57 insertions(+), 13 deletions(-) > Patch set looks good to me. > Acked-by: Mykyta Yatsenko <mykyta.yatsenko5@gmail.com> > Thank you for taking a look!