Re: [PATCH v3 bpf-next 01/10] treewide: remove struct-pass-by-value from tracepoints arguments

[Alexei Starovoitov <ast@fb.com>]

On 3/22/18 1:52 PM, Linus Torvalds wrote:
> On Thu, Mar 22, 2018 at 1:48 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>>
>> OK, but instead of changing it to pass by reference, why not just pass
>> the value. That is:
>>
>>  static void xen_set_pte_atomic(pte_t *ptep, pte_t pte)
>>  {
>> -       trace_xen_mmu_set_pte_atomic(ptep, pte);
>> +       trace_xen_mmu_set_pte_atomic(ptep, native_pte_val(pte));
>>         set_64bit((u64 *)ptep, native_pte_val(pte));
>>  }
>
> That looks simple and clean, and makes sense since the function itself
> then uses that value anyway.
>
> Certainly simpler than my monster define.

memcpy to avoid warning is a nice trick.
Generate code looks good on variety of compilers and architectures.
It also works with s390 tracepoints that pass this struct:
struct subchannel_id {
         u32 cssid : 8;
         u32 : 4;
         u32 m : 1;
         u32 ssid : 2;
         u32 one : 1;
         u32 sch_no : 16;
} __attribute__ ((packed, aligned(4)));

by value into a bunch of trace_* functions.
So at the end this 'monster define' will reduce patch 1 a lot.

Passing less than 8 byte structs by value is ok.
If we disallow them for tracepoints it would be seen as annoying
programming convention, because one dude (me) couldn't figure out
how to cast them.
Since you found the way to do this cast, let's go with it.
It will be hidden in one file (regardless whether it's ugly or not)
whereas small struct dereferences will be everywhere,
so one hack is better.