Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <31abaa2d-f93b-e2df-4605-04c276ea7397@rasmusvillemoes.dk>
Date:   Wed, 12 Apr 2023 23:09:55 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v1 1/1] kernel.h: Split out COUNT_ARGS() and CONCATENATE()
Content-Language: en-US, da
To:     Andrew Morton <akpm@linux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     linux-kernel@vger.kernel.org
References: <20230411102454.85898-1-andriy.shevchenko@linux.intel.com>
 <20230411152119.c9bb83a8566e37f6d8dbc590@linux-foundation.org>
 <ZDaqi1jE0Fkg5BSp@smile.fi.intel.com>
 <20230412115531.17bb1aed232952c79aba0f2a@linux-foundation.org>
From:   Rasmus Villemoes <linux@rasmusvillemoes.dk>
In-Reply-To: <20230412115531.17bb1aed232952c79aba0f2a@linux-foundation.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12/04/2023 20.55, Andrew Morton wrote:
> On Wed, 12 Apr 2023 15:56:43 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
>> On Tue, Apr 11, 2023 at 03:21:19PM -0700, Andrew Morton wrote:
>>> On Tue, 11 Apr 2023 13:24:54 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>>>
>>>> kernel.h is being used as a dump for all kinds of stuff for a long time.
>>>> The COUNT_ARGS() and CONCATENATE() macros may be used in some places
>>>> without need of the full kernel.h dependency train with it.
>>>>
>>>> Here is the attempt on cleaning it up by splitting out these macros().
>>>>
>>>> --- a/include/linux/kernel.h
>>>> +++ b/include/linux/kernel.h
>>>> @@ -13,6 +13,7 @@
>>>>  
>>>>  #include <linux/stdarg.h>
>>>>  #include <linux/align.h>
>>>> +#include <linux/args.h>
>>>
>>> A more energetic patch would have included args.h into each file which
>>> calls COUNT_ARGS() and CONCATENATE(), and not included args.h into
>>> kernel.h.  And that appears to be very easy - only bpf uses these things?
>>>
>>> In fact these macros are so weird and ugly I'd be inclined to move them
>>> into some bpf header so we don't have to see them again.  No
>>> args.h, which might avoid encouraging others to use them.
>>
>> We have more users than one 
> 
> I cant find any?

True, git grep COUNT_ARGS doesn't find anything other than the
bpf_probe.h user.

>> and a couple of users that reimplement this macro
>> under different names.
>>
> Where are these?

Amusingly, bpf have at least

tools/lib/bpf/bpf_core_read.h:#define ___narg(...) ___nth(_,
##__VA_ARGS__, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
tools/lib/bpf/bpf_helpers.h:    ___bpf_nth(_, ##__VA_ARGS__, 12, 11, 10,
9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
tools/lib/bpf/bpf_tracing.h:#define ___bpf_narg(...) ___bpf_nth(_,
##__VA_ARGS__, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)

OK, so that's under tools/, but we really should be able to make a
cpp-tricks-header that's usable by the kernel itself as well as tools.

There's also

include/linux/arm-smccc.h:      ___count_args(__VA_ARGS__, 7, 6, 5, 4,
3, 2, 1, 0)
arch/x86/include/asm/rmwcc.h:#define RMWcc_ARGS(X...) __RMWcc_ARGS(,
##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)

and some efi_nargs that seems a little more elaborate (though I doubt
they couldn't just make do with the standard macro).

There's probably even more, this is just what grepping for a typical
implementation showed.

> What the heck does it do

It counts the number of arguments given to a variadic macro.

> and why is it so ugly

Because cpp.

> and why isn't it documented.  Shudder.

Yeah, some comments on how it works and its limitation(s) would probably
be in order.

Rasmus