On Tue, Jun 23, 2020 at 11:59:40PM -0700, Andrii Nakryiko wrote: > On Tue, Jun 23, 2020 at 11:47 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Tue, Jun 23, 2020 at 5:34 PM Andrii Nakryiko <andriin@xxxxxx> wrote: > > > > > > Similar message for map creation is extremely useful, so add similar for BPF > > > programs. > > > > 'extremely useful' is quite subjective. > > If we land this patch then everyone will be allowed to add pr_debug() > > everywhere in libbpf with the same reasoning: "it's extremely useful pr_debug". > > We print this for maps, making it clear which maps and with which FD > were created. Having this for programs is just as useful. It doesn't > overwhelm output (and it's debug one either way). "everyone will be > allowed to add pr_debug()" is a big stretch, you can't just sneak in > or force random pr_debug, we do review patches and if something > doesn't make sense we can and we do reject it, regardless of claimed > usefulness by the patch author. > > So far, libbpf debug logs were extremely helpful (subjective, of > course, but what isn't?) to debug "remotely" various issues that BPF > users had. They don't feel overwhelmingly verbose and don't have a lot > of unnecessary info. Adding a few lines (how many BPF programs are > there per each BPF object?) for listing BPF programs is totally ok. None of the above were mentioned in the commit log. And no examples were given where this extra line would actually help. I think libbpf pr_debug is extremely verbose instead of extremely useful. Just typical output: ./test_progs -vv -t lsm libbpf: loading object 'lsm' from buffer libbpf: section(1) .strtab, size 306, link 0, flags 0, type=3 libbpf: skip section(1) .strtab libbpf: section(2) .text, size 0, link 0, flags 6, type=1 libbpf: skip section(2) .text libbpf: section(3) lsm/file_mprotect, size 192, link 0, flags 6, type=1 libbpf: found program lsm/file_mprotect libbpf: section(4) .rellsm/file_mprotect, size 32, link 25, flags 0, type=9 libbpf: section(5) lsm/bprm_committed_creds, size 104, link 0, flags 6, type=1 libbpf: found program lsm/bprm_committed_creds libbpf: section(6) .rellsm/bprm_committed_creds, size 32, link 25, flags 0, type=9 How's above useful for anyone? libbpf says that there are '.strtab' and '.text' sections in the elf file. That's wet water. Any elf file has that. Then it says it's skipping '.text' ? That reads surprising. Why library would skip the code? And so on and so forth. That output is useful to only few core libbpf developers. I don't mind more thought through debug prints, but saying that existing pr_debugs are 'extremely useful' is a stretch.
