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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/22/18 1:52 PM, Linus Torvalds wrote:
On Thu, Mar 22, 2018 at 1:48 PM, Steven Rostedt <rostedt@xxxxxxxxxxx> 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux