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]:36076 "EHLO
        mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK)
        by vger.kernel.org with ESMTP id S1753715AbeCUWGo (ORCPT
        <rfc822;netdev@vger.kernel.org>); Wed, 21 Mar 2018 18:06:44 -0400
Subject: Re: [PATCH v2 bpf-next 4/8] tracepoint: compute num_args at build
 time
To: Linus Torvalds <torvalds@linux-foundation.org>
References: <20180321185448.2806324-1-ast@fb.com>
 <20180321185448.2806324-5-ast@fb.com>
 <CA+55aFyXFSuyxJrLqixsZ-SBXSOrSJ6Q_oOF04Fd4pzMRbPCEA@mail.gmail.com>
CC: David Miller <davem@davemloft.net>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Peter Zijlstra <peterz@infradead.org>,
        Steven Rostedt <rostedt@goodmis.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: <2559d7cb-ec60-1200-2362-04fa34fd02bb@fb.com>
Date: Wed, 21 Mar 2018 15:05:46 -0700
MIME-Version: 1.0
In-Reply-To: <CA+55aFyXFSuyxJrLqixsZ-SBXSOrSJ6Q_oOF04Fd4pzMRbPCEA@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/21/18 12:44 PM, Linus Torvalds wrote:
> On Wed, Mar 21, 2018 at 11:54 AM, Alexei Starovoitov <ast@fb.com> wrote:
>>
>> add fancy macro to compute number of arguments passed into tracepoint
>> at compile time and store it as part of 'struct tracepoint'.
>
> We should probably do this __COUNT() thing in some generic header, we
> just talked last week about another use case entirely.

ok. Not sure which generic header though.
Should I move it to include/linux/kernel.h ?

> And wouldn't it be nice to just have some generic infrastructure like this:
>
>     /*
>      * This counts to ten.
>      *
>      * Any more than that, and we'd need to take off our shoes
>      */
>     #define __GET_COUNT(_0,_1,_2,_3,_4,_5,_6,_7,_8,_9,_10,_n,...) _n
>     #define __COUNT(...) \
>         __GET_COUNT(__VA_ARGS__,10,9,8,7,6,5,4,3,2,1,0)
>     #define COUNT(...) __COUNT(dummy,##__VA_ARGS__)

since it will be a build time error, it's a good time to discuss
how many arguments we want to support in tracepoints and
in general in other places that would want to use this macro.

Like the only reason my patch is counting till 17 is because of
trace_iwlwifi_dev_ucode_error().
The next offenders are using 12 arguments:
trace_mc_event()
trace_mm_vmscan_lru_shrink_inactive()

Clearly not every efficient usage of it:
         trace_mm_vmscan_lru_shrink_inactive(pgdat->node_id,
                         nr_scanned, nr_reclaimed,
                         stat.nr_dirty,  stat.nr_writeback,
                         stat.nr_congested, stat.nr_immediate,
                         stat.nr_activate, stat.nr_ref_keep,
                         stat.nr_unmap_fail,
                         sc->priority, file);
could have passed &stat instead.

I'd like to refactor that trace_iwlwifi_dev_ucode_error()
and from now on set the limit to 12.
Any offenders should be using tracepoints with <= 12 args
instead of extending the macro.
Does it sound reasonable ?

>     #define __CONCAT(a,b) a##b
>     #define __CONCATENATE(a,b) __CONCAT(a,b)
>
> and then you can do things like:
>
>     #define fn(...) __CONCATENATE(fn,COUNT(__VA_ARGS__))(__VA_ARGS__)
>
> which turns "fn(x,y,z..)" into "fn<N>(x,y,z)".
>
> That can be useful for things like "max(a,b,c,d)" expanding to
> "max4()", and then you can just have the trivial
>
>   #define max3(a,b,c) max2(a,max2(b.c))

I can try that. Not sure my macro-fu is up to that level.
__CAST_TO_U64() macro from the next patch was difficult to make
work across compilers and architectures.