From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <netdev-owner@vger.kernel.org>
Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:36178 "EHLO
        mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK)
        by vger.kernel.org with ESMTP id S1751531AbeCVWxc (ORCPT
        <rfc822;netdev@vger.kernel.org>); Thu, 22 Mar 2018 18:53:32 -0400
Subject: Re: [PATCH v3 bpf-next 01/10] treewide: remove struct-pass-by-value
 from tracepoints arguments
To: Linus Torvalds <torvalds@linux-foundation.org>,
        Steven Rostedt <rostedt@goodmis.org>
References: <20180322180157.742725-1-ast@fb.com>
 <20180322180157.742725-2-ast@fb.com>
 <20180322141119.7c876c13@gandalf.local.home>
 <6b1952c4-1612-7abb-49c6-9bbaf6dc6997@fb.com>
 <20180322164831.01519df6@gandalf.local.home>
 <CA+55aFzWKE=6N4_0ihV4JbFcAXJaO_E_=V4q9M2npk3CJM0yrQ@mail.gmail.com>
CC: David Miller <davem@davemloft.net>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Peter Zijlstra <peterz@infradead.org>,
        Network Development <netdev@vger.kernel.org>,
        kernel-team <kernel-team@fb.com>,
        Linux API <linux-api@vger.kernel.org>
From: Alexei Starovoitov <ast@fb.com>
Message-ID: <22c5240e-af3e-fff1-2be0-3c5692d611aa@fb.com>
Date: Thu, 22 Mar 2018 15:52:44 -0700
MIME-Version: 1.0
In-Reply-To: <CA+55aFzWKE=6N4_0ihV4JbFcAXJaO_E_=V4q9M2npk3CJM0yrQ@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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.