Thread (17 messages) 17 messages, 3 authors, 2018-03-22

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

From: Alexei Starovoitov <hidden>
Date: 2018-03-22 22:53:32

On 3/22/18 1:52 PM, Linus Torvalds wrote:
On Thu, Mar 22, 2018 at 1:48 PM, Steven Rostedt [off-list ref] wrote:
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help