linux-kernel - Re: [PATCH v6 12/42] x86/resctrl: Move rdt_find_domain() to be visible to arch and fs code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0672ad74-782e-48b9-8591-7e0bdec84845@intel.com>
Date: Thu, 20 Feb 2025 08:01:47 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: Catalin Marinas <catalin.marinas@....com>
CC: James Morse <james.morse@....com>, <x86@...nel.org>,
	<linux-kernel@...r.kernel.org>, Thomas Gleixner <tglx@...utronix.de>, "Ingo
 Molnar" <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, H Peter Anvin
	<hpa@...or.com>, Babu Moger <Babu.Moger@....com>,
	<shameerali.kolothum.thodi@...wei.com>, D Scott Phillips OS
	<scott@...amperecomputing.com>, <carl@...amperecomputing.com>,
	<lcherian@...vell.com>, <bobo.shaobowang@...wei.com>,
	<tan.shaopeng@...itsu.com>, <baolin.wang@...ux.alibaba.com>, Jamie Iles
	<quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
	<peternewman@...gle.com>, <dfustini@...libre.com>, <amitsinght@...vell.com>,
	David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
	"Dave Martin" <dave.martin@....com>, Koba Ko <kobak@...dia.com>, Shanker
 Donthineni <sdonthineni@...dia.com>
Subject: Re: [PATCH v6 12/42] x86/resctrl: Move rdt_find_domain() to be
 visible to arch and fs code

Hi Catalin,

On 2/20/25 2:58 AM, Catalin Marinas wrote:
> On Wed, Feb 19, 2025 at 03:24:06PM -0800, Reinette Chatre wrote:
>> On 2/7/25 10:17 AM, James Morse wrote:
>>> rdt_find_domain() finds a domain given a resource and a cache-id.
>>> This is used by both the architecture code and the filesystem code.
>>>
>>> After the filesystem code moves to live in /fs/, this helper will no
>>> longer be visible.
>>>
>>> Move it to the global header file. As its now globally visible, and
>>> has only a handful of callers, swap the 'rdt' for 'resctrl'.
> [...]
>>> --- a/include/linux/resctrl.h
>>> +++ b/include/linux/resctrl.h
>>> @@ -372,6 +372,40 @@ static inline void resctrl_arch_rmid_read_context_check(void)
>>>  		might_sleep();
>>>  }
>>>  
>>> +/**
>>> + * resctrl_find_domain() - Search for a domain id in a resource domain list.
>>> + * @h:		The domain list to search.
>>> + * @id:		The domain id to search for.
>>> + * @pos:	A pointer to position in the list id should be inserted.
>>> + *
>>> + * Search the domain list to find the domain id. If the domain id is
>>> + * found, return the domain. NULL otherwise.  If the domain id is not
>>> + * found (and NULL returned) then the first domain with id bigger than
>>> + * the input id can be returned to the caller via @pos.
>>> + */
>>> +static inline struct rdt_domain_hdr *resctrl_find_domain(struct list_head *h,
>>> +							 int id,
>>> +							 struct list_head **pos)
>>
>> Could you please provide a motivation for why this needs to be inline now?
> 
> It's in a header now, to avoid the compiler complaining about unused
> static functions wherever this file is included. The alternative is a
> prototype declaration and the actual implementation in a .c file.

resctrl_find_domain() is currently in a .c file (arch/x86/kernel/cpu/resctrl/core.c)
with a prototype declaration (in arch/x86/kernel/cpu/resctrl/internal.h). This patch
switches resctrl_find_domain() to be inline without a motivation.

After a fresh reading of "The inline disease" in Documentation/process/coding-style.rst 
I do see a few red flags related to making this function inline. The function is certainly
larger than the rule of thumb of "3 lines" and considering the number of call sites I do
not see how bloating the kernel is justified.

> 
> (drive-by comment, I don't really understand this subsystem to make a
> meaningful contribution)
> 

Thanks for taking a look. The idea is not unique to resctrl.

Reinette