netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
To: Alexandre Ferrieux <alexandre.ferrieux@gmail.com>,
	Pedro Tammela <pctammela@mojatatu.com>,
	edumazet@google.com
Cc: jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us,
	netdev@vger.kernel.org
Subject: Re: [PATCH net] Fix u32's systematic failure to free IDR entries for hnodes.
Date: Tue, 5 Nov 2024 23:42:14 +0000	[thread overview]
Message-ID: <771bd976-e68c-48d0-bfbd-1f1b73d7bb91@linux.dev> (raw)
In-Reply-To: <9dbb815a-0137-4565-ad91-8ed92d53bced@gmail.com>

On 05/11/2024 22:14, Alexandre Ferrieux wrote:
>> On 04/11/2024 22:33, Vadim Fedorenko wrote:
>>>> On 04/11/2024 18:00, Pedro Tammela wrote:
>>>>> 'static inline' is discouraged in .c files
>>>>
>>>> Why ?
>>>>
>>>> It could have been a local macro, but an inline has (a bit) better type
>>>> checking. And I didn't want to add it to a .h that is included by many other
>>>> unrelated components, as it makes no sense to them. So, what is the recommendation ?
>>>
>>> Either move it to some local header file, or use 'static u32
>>> handle2id(u32 h)'
>>> and let compiler decide whether to include it or not.
>>
>> I believe you mean "let the compiler decide whether to _inline_ it or not".
>> Sure, with a sufficiently modern Gcc this will do. However, what about more
>> exotic environments ? Wouldn't it risk a perf regression for style reasons ?
>>
>> And speaking of style, what about the dozens of instances of "static inline" in
>> net/sched/*.c alone ? Why is it a concern suddenly ?
> 
> 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.

I don't really understand what kind of exotic environment you are
thinking about, but modern kernel has proper gcc/clang version
dependency, both of these compilers have good heuristics to identify
which function to inline.


  reply	other threads:[~2024-11-05 23:42 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-04 10:26 [PATCH net] Fix u32's systematic failure to free IDR entries for hnodes Alexandre Ferrieux
2024-11-04 17:00 ` Pedro Tammela
2024-11-04 20:26   ` Alexandre Ferrieux
2024-11-04 21:33     ` Vadim Fedorenko
2024-11-04 21:51       ` Alexandre Ferrieux
2024-11-04 22:33         ` Florian Fainelli
2024-11-05 22:14         ` Alexandre Ferrieux
2024-11-05 23:42           ` Vadim Fedorenko [this message]
2024-11-06 10:15             ` Alexandre Ferrieux
2024-11-06 10:54               ` Vadim Fedorenko
2024-11-10 14:00         ` Simon Horman
2024-11-10 15:40           ` Alexandre Ferrieux
2024-11-11 20:07             ` Simon Horman
  -- strict thread matches above, loose matches on Subject: below --
2024-11-01 18:43 Alexandre Ferrieux
2024-11-04 10:11 ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=771bd976-e68c-48d0-bfbd-1f1b73d7bb91@linux.dev \
    --to=vadim.fedorenko@linux.dev \
    --cc=alexandre.ferrieux@gmail.com \
    --cc=edumazet@google.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=pctammela@mojatatu.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).