Re: [PATCH v2 bpf-next 6/7] bpf: add support for %pT format specifier for bpf_trace_printk() helper

From: Yonghong Song
Date: Mon May 18 2020 - 10:48:23 EST




On 5/18/20 2:10 AM, Alan Maguire wrote:
On Wed, 13 May 2020, Yonghong Song wrote:


+ while (isbtffmt(fmt[i]))
+ i++;

The pointer passed to the helper may not be valid pointer. I think you
need to do a probe_read_kernel() here. Do an atomic memory allocation
here should be okay as this is mostly for debugging only.


Are there other examples of doing allocations in program execution
context? I'd hate to be the first to introduce one if not. I was hoping
I could get away with some per-CPU scratch space. Most data structures
will fit within a small per-CPU buffer, but if multiple copies
are required, performance isn't the key concern. It will make traversing
the buffer during display a bit more complex but I think avoiding
allocation might make that complexity worth it. The other thought I had
was we could carry out an allocation associated with the attach,
but that's messy as it's possible run-time might determine the type for
display (and thus the amount of the buffer we need to copy safely).

percpu buffer definitely better. In fact, I am using percpu buffer
in bpf_seq_printf() helper. Yes, you will need to handling contention
though. I guess we can do the same thing here, return -EBUSY so bpf
program can react properly (retry, or just print error, etc.)
if there is a contention.


Great news about LLVM support for __builtin_btf_type_id()!

Thanks. Hopefully this will make implementation easier.


Thanks!

Alan