On Thu, May 8, 2025 at 11:16 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Fri, 2025-05-09 at 01:33 +0200, Kumar Kartikeya Dwivedi wrote: > > [...] > > > > > -#define ___bpf_pick_printk(...) \ > > > > - ___bpf_nth(_, ##__VA_ARGS__, __bpf_vprintk, __bpf_vprintk, __bpf_vprintk, \ > > > > - __bpf_vprintk, __bpf_vprintk, __bpf_vprintk, __bpf_vprintk, \ > > > > - __bpf_vprintk, __bpf_vprintk, __bpf_printk /*3*/, __bpf_printk /*2*/,\ > > > > - __bpf_printk /*1*/, __bpf_printk /*0*/) > > > > +#define ___bpf_pick_printk(choice, choice_3, ...) \ > > > > + ___bpf_nth(_, ##__VA_ARGS__, choice, choice, choice, \ > > > > + choice, choice, choice, choice, \ > > > > + choice, choice, choice_3 /*3*/, choice_3 /*2*/, \ > > > > + choice_3 /*1*/, choice_3 /*0*/) > > > > > > > > /* Helper macro to print out debug messages */ > > > > -#define bpf_printk(fmt, args...) ___bpf_pick_printk(args)(fmt, ##args) > > > > +#define __bpf_trace_printk(fmt, args...) \ > > > > + ___bpf_pick_printk(__bpf_vprintk, __bpf_printk, args)(fmt, ##args) > > > > +#define __bpf_stream_printk(stream, fmt, args...) \ > > > > + ___bpf_pick_printk(__bpf_stream_vprintk, __bpf_stream_vprintk, args)(stream, fmt, ##args) > > > ^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^ > > > These two parameters are identical, > > > why is ___bpf_pick_printk is necessary in such case? > > > > In our case choice and choice_3 are the same, but for bpf_printk > > they're different, I was mostly trying to reuse the pick_printk > > machinery for both (which dispatches correctly to the actual macro). > > > > But ___bpf_pick_printk is a noop if two identical choices are supplied, > so there is nothing to reuse. E.g. nothing breaks after the following change: > > #define __bpf_trace_printk(fmt, args...) \ > ___bpf_pick_printk(__bpf_vprintk, __bpf_printk, args)(fmt, ##args) > -#define __bpf_stream_printk(stream, fmt, args...) \ > - ___bpf_pick_printk(__bpf_stream_vprintk, __bpf_stream_vprintk, args)(stream, fmt, ##args) > > -#define bpf_stream_printk(stream, fmt, args...) __bpf_stream_printk(stream, fmt, ##args) > +#define bpf_stream_printk(stream, fmt, args...) __bpf_stream_vprintk(stream, fmt, ##args) > > #define bpf_printk(arg, args...) __bpf_trace_printk(arg, ##args) > > Which allows to shorten this patch. > Or do I miss something? > +1, we have this ___bpf_pick_printk business because we want to use older bpf_trace_printk() that accepts values directly if possible (for best support of old kernels). With bpf_stream_vprintk() it's always values-in-array approach, so no need for all this extra macro machinery, IMO. > [...] >
