From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:54264 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751595AbeCVTcY (ORCPT ); Thu, 22 Mar 2018 15:32:24 -0400 Subject: Re: [PATCH v3 bpf-next 01/10] treewide: remove struct-pass-by-value from tracepoints arguments To: Steven Rostedt References: <20180322180157.742725-1-ast@fb.com> <20180322180157.742725-2-ast@fb.com> <20180322141119.7c876c13@gandalf.local.home> CC: , , , , , , From: Alexei Starovoitov Message-ID: <6b1952c4-1612-7abb-49c6-9bbaf6dc6997@fb.com> Date: Thu, 22 Mar 2018 12:31:12 -0700 MIME-Version: 1.0 In-Reply-To: <20180322141119.7c876c13@gandalf.local.home> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: On 3/22/18 11:11 AM, Steven Rostedt wrote: > On Thu, 22 Mar 2018 11:01:48 -0700 > Alexei Starovoitov wrote: > >> From: Alexei Starovoitov >> >> Fix all tracepoint arguments to pass structures (large and small) by reference >> instead of by value. >> Avoiding passing large structs by value is a good coding style. >> Passing small structs sometimes is beneficial, but in all cases >> it makes no difference vs readability of the code. >> The subsequent patch enforces that all tracepoints args are either integers >> or pointers and fit into 64-bit. > > But some of these structures are used to force type checking, and are > just the same size as a number. That's why they don't have "struct" in > front of them. Like pmd_t. Will the subsequent patches really break if > the structure itself has one element that is of size long? Just seems > to add extra code to pass in an address to something that fits into a > single register. yeah. C doesn't allow casting of 'struct s { u64 var };' into u64 without massive hacks and aliasing warnings by compiler. CAST_TO_U64 macro in patch 7 will prevent tracepoint arguments to be things like pmd_t. It's not perfect, but doing & of pmd_t variable is imo clean enough as you can see in this patch. The macro can be tweaked to do the cast like *(sizeof(typeof(arg))*)&arg, but there is no way to get rid of compiler warning.