Subject: Re: [PATCH net] Fix u32's systematic failure to free IDR entries for hnodes.

From: Vadim Fedorenko <[email-masked]>
Date: Wed, 6 Nov 2024 10:54:21 +0000

On 06/11/2024 10:15, Alexandre Ferrieux wrote:
> On 06/11/2024 00:42, Vadim Fedorenko wrote:
>> On 05/11/2024 22:14, Alexandre Ferrieux wrote:
>>>
>>> Can you please explain *why* in the first place you're saying "'static inline'
>>> is discouraged in .c files" ? I see no trace if this in coding-style.rst, and
>>> the kernel contains hundreds of counter-examples.
>>
>> The biggest reason is because it will mask unused function warnings when
>> "static inline" function will not be used because of some future
>> patches. There is no big reason to refactor old code that's why there
>> are counter-examples in the kernel, but the new code shouldn't have it.
> 
> A macro doesn't elicit unused function warnings either, so this looks like a
> very weak motivation. While coding-style.rst explicitly encourages to use static
> inline instead of macros, as they have better type checking and syntaxic isolation.

Unused macro will not generate any code and will not make build time
longer. But don't get me wrong, I'm not encouraging you to use macro in
this case.

> Regarding old vs new code, below are the last two month's fresh commits of
> "static inline" in *.c. So it looks like the motivation is not shared by other
> maintainers. Do we expect to see "local styles" emerge ?

There are some "local styles" differences in different subsystems. If
you filter out netdev related diffs from awk-magic below, you will find
that is was mostly refactoring of already existing code.

Anyways, you can ignore this suggestion, it's always up to submitter
how to use review feedback given by others.

> 
> $ git log --pretty='%h %as %ae'   -p | gawk
> '/^[0-9a-f]{12}/{c=$0;next}/^diff/{f=$NF;next}/^[+].*static.inline/{if
> (f~/[.]c$/){print c "\t"gensub(/.*\//,"","1",f)}}'
> 
> baa802d2aa5c 2024-10-21 [email-masked]    verifier_const.c
> baa802d2aa5c 2024-10-21 [email-masked]    verifier_const.c
> d1744a4c975b 2024-10-21 [email-masked]    amd.c
> d1744a4c975b 2024-10-21 [email-masked]    amd.c
> a6e0ceb7bf48 2024-10-11 [email-masked]      maple.c
> 78f636e82b22 2024-09-25 [email-masked]    ap_queue.c
> 19773ec99720 2024-10-07 [email-masked]       disk_accounting.c
> 9b23fdbd5d29 2024-09-29 [email-masked]       inode.c
> 9b23fdbd5d29 2024-09-29 [email-masked]       inode.c
> 3d5854d75e31 2024-09-30 [email-masked]  kcore.c
> 3d5854d75e31 2024-09-30 [email-masked]  kcore.c
> 38864eccf78b 2024-09-30 [email-masked]       fsck.c
> d278a9de5e18 2024-10-02 [email-masked]  init.c
> f811b83879fb 2024-10-02 [email-masked]     dm-verity-target.c
> 4c411cca33cf 2024-09-13 [email-masked]        intel_idle.c
> 42268ad0eb41 2024-09-24 [email-masked]   ext.c
> 56bcd0f07fdb 2024-09-05 [email-masked]      localio.c
> 1b11c4d36548 2024-09-01 [email-masked]       ec.c
> 7a51608d0125 2024-09-04 [email-masked]       btree_cache.c
> 7a51608d0125 2024-09-04 [email-masked]       btree_cache.c
> 691f2cba2291 2024-09-05 [email-masked]       btree_cache.c
>