Received-SPF: pass (google.com: domain of linux-kernel+bounces-134625-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <d3ef1a5a-a7ee-477c-8697-d64b91726d91@intel.com>
Date: Sun, 7 Apr 2024 12:21:17 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 1/6] x86/resctrl: Move __resctrl_sched_in() out-of-line
To: Peter Newman <peternewman@google.com>
CC: Fenghua Yu <fenghua.yu@intel.com>, James Morse <james.morse@arm.com>,
	Stephane Eranian <eranian@google.com>, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Dave Hansen
	<dave.hansen@linux.intel.com>, <x86@kernel.org>, "H. Peter Anvin"
	<hpa@zytor.com>, Peter Zijlstra <peterz@infradead.org>, Juri Lelli
	<juri.lelli@redhat.com>, Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>, Steven Rostedt
	<rostedt@goodmis.org>, Ben Segall <bsegall@google.com>, Mel Gorman
	<mgorman@suse.de>, Daniel Bristot de Oliveira <bristot@redhat.com>, "Valentin
 Schneider" <vschneid@redhat.com>, Uros Bizjak <ubizjak@gmail.com>, "Mike
 Rapoport" <rppt@kernel.org>, "Kirill A. Shutemov"
	<kirill.shutemov@linux.intel.com>, Rick Edgecombe
	<rick.p.edgecombe@intel.com>, Xin Li <xin3.li@intel.com>, Babu Moger
	<babu.moger@amd.com>, Shaopeng Tan <tan.shaopeng@fujitsu.com>, "Maciej
 Wieczor-Retman" <maciej.wieczor-retman@intel.com>, Jens Axboe
	<axboe@kernel.dk>, Christian Brauner <brauner@kernel.org>, Oleg Nesterov
	<oleg@redhat.com>, Andrew Morton <akpm@linux-foundation.org>, Tycho Andersen
	<tandersen@netflix.com>, Nicholas Piggin <npiggin@gmail.com>, Beau Belgrave
	<beaub@linux.microsoft.com>, "Matthew Wilcox (Oracle)" <willy@infradead.org>,
	<linux-kernel@vger.kernel.org>
References: <20240325172707.73966-1-peternewman@google.com>
 <20240325172707.73966-2-peternewman@google.com>
 <e2c479b6-43f0-4e2c-9ccd-2e5f322d1974@intel.com>
 <CALPaoCjWDKM9VYXexDt2+zh2srTotuaErFHXkWzH1w0c_KWDjw@mail.gmail.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <CALPaoCjWDKM9VYXexDt2+zh2srTotuaErFHXkWzH1w0c_KWDjw@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WUlOSVNMM3pvbTA1WFBickpVU2hUTTVUQWFpNHUyT3hiMys3RWFObEFmcWl6?=
 =?utf-8?B?UDlEYmNJa0htQXVnMlhjNVQ2OWt4TUNyRmcxbC9jeWxKL2FBaWV1a0VaOCtX?=
 =?utf-8?B?b3ZXZHhlcGs3TC82dlJmbk5rNHFRUloyREJvRlBKYlkzbExUK2REVjhVLzY2?=
 =?utf-8?B?ajVRNFpYMEZRZ0ExdmZlYkxYSVpSYlVGRmVYcGV4SW1paStHNncvYi9lSi9Q?=
 =?utf-8?B?UDFNRmM3VjM2d2g2TmxZOFUrNk5FL3dvanQwRTkwbnAxSzVZK0VmT1Z5Sm9J?=
 =?utf-8?B?enlINDRQQXdBazZiWXNXdHBQZUNOa0hmUmdiN29BUnZUMkNrQTlHbmdpR2NC?=
 =?utf-8?B?enVCTmFIUVladFVUTG9ZVEpWZXdaaEJxK29KbEJhaHlldGc2TXFsRTRMa3RB?=
 =?utf-8?B?VUc0LzlqeERZb0Q5MHUzWit0OXM0N3BsQXVhdDg1eGJVTkphdmtFRzRnc28v?=
 =?utf-8?B?K0ZlRkhpWkpaN1BjNWxRUkp5c08ydzdranBjUzF4OHQ4QUF4MHN2cnJya0ZM?=
 =?utf-8?B?b29SWXMzcUlVV0ZmWVhyQ3BSL3JjT1ZuSnpPOUEwMkhuV3NnSjV6L3hnY205?=
 =?utf-8?B?L2YzNEg0Mm5TcXJVSjZ0TGpEb1IzdUpRVWdydElrTUJEcG9MRnRkOTllREhR?=
 =?utf-8?B?b2FrM3dHV25jWnAwU085YjdGdlA2cWtkdmhyRVpKaTZKRU9rYzJhSCt5MkJE?=
 =?utf-8?B?NnJrUHhFeGlNOGw0VEpDNUJpOSt4N1ZQMVRVZFQxTzR6eTJTM24remMxWVJJ?=
 =?utf-8?B?OTU4c1JzOW1CTFdDOWVlc3RKZUoxRU5XMzZLeGVJU3dWdU5ZdU5GNG5KUXJT?=
 =?utf-8?B?NUxtM051dERoNXg0WS96Nmc3NzVrS2VCamVLeUl1eXhVT0xGcHZWYVI2cjhK?=
 =?utf-8?B?Y29mdlBWMDA3QzNxQjJqQW9MUjMzWklyK1A2aWRHOHRJbTNIZERlVm9SUW1y?=
 =?utf-8?B?TFphaGR4V3ZVRmtURFBtQS92Y01JWEs0VlVUSi9KR0J0RjdPQ3JjaG5kQ0ZZ?=
 =?utf-8?B?SWhqUjUyNzZ1cVVXR0F2RnhqaFlnOFBZYzBDNWYxcStDWWJWVm9ZL2ZnSXVS?=
 =?utf-8?B?azRJWTRJUzNoOXJ0VUxhRE9UMTVLakR0dGdoUjEvcjdSNW1OV2lmSjE4bFFE?=
 =?utf-8?B?VlFwRVgxeDI2TkpFMDd5eGZ4UjZHTy9RdEpqcnRRVVFCUTZPNFFQVFNaeEFT?=
 =?utf-8?B?cFlNS1VEYTZvSXdxT0pyZGp3RGVjTzNxTXlnZlhWMXBJVGlwR3B3SFVVRjhJ?=
 =?utf-8?B?cXQ0TWtIMXhtV29VYUtlSmhic0J1V3pRc0Z6TjBlWGE1ek9nOGNEZ2YyOXB0?=
 =?utf-8?B?TG10Zlp5V3ZOK2MwcWNvUE9YdkVlVHozeklPaWl0UGFsVzhjMkdPZTllVWlN?=
 =?utf-8?B?OFNqMHNMWGd0VDM3aXY2RFNscjZFblVSVVNkZWFLdGd5Wnp4SzVLRk9XNWg5?=
 =?utf-8?B?ZGw4VE1tMFd4bGdFZGdqL0hhUTU5ZVM0TWtkMWc0UTJBRm83TjFGYkdqbmlX?=
 =?utf-8?B?MldXSXFFKzVMUi9LYXk4d0p1N013M1NHbmRvMEFoSXZMOFA4NG9KUWU5R2Iv?=
 =?utf-8?B?UGsvMEk1bitlaHp1SUtrWCtUZzd2OTAvS28yMVlOL3FPRm1CcTN2N2FpcHdo?=
 =?utf-8?B?emlTSHhXNm1ON3lkSmxQdU1KOXpSc0Z2TVhEdmhKTWdZM1FSYTAzQlEzZ0ox?=
 =?utf-8?B?ZnZmV3ZuWHhtb3YybDNyZkpHWm1YSFBjeGxoc3F0SW5XNVk2eURLdUZMNDBy?=
 =?utf-8?B?a0hDTjFkUlM1d2psb0VvUjUrcWgvRUxYbzhrWFk3U0pTbkFoYm9hSzBLVTdO?=
 =?utf-8?B?SU1XTTNZWEZpQ21nMklsWjBaS3hiQmFxUEFXenVRUy9jRTF4WTlUUFV3WUYz?=
 =?utf-8?B?MjJWTzVjR0t6R3RLa0ZvQkhUZjVrWXJQUFhmTGVRNVlSb1pyMVNBOUI4bEJP?=
 =?utf-8?B?SUFWTmtqMzQ1ekFCVEhKKzJqOVg2OEdFdVhsSkFWaWs5eHpTaTRva3lySXdQ?=
 =?utf-8?B?Z0szalRXNVlIakJ0SXJRWHpzOVdMVGh2U3dIRUJIY3U1VDZIeWNIVHdUMG92?=
 =?utf-8?B?V3RYMEkzaVMvYXRWVzBNQUdIMXRtckF3cVlKbUpKTmE3bzlPdUs0bnhBYTBn?=
 =?utf-8?B?Nm0rdmE2a0cwbzNZR1J4M1pjTm5tczBmeXNoNjRjTWJuOW5KYys2M2RZeDgy?=
 =?utf-8?B?bUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3c160191-9acc-4dfd-b09f-08dc5737e83c
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Apr 2024 19:21:21.2381
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: MA75pkX5NaHmalENePxzBKWE7+2o//UnzGeXczdkigxKuxE5z8PDDUk/h9hQb92gsHUblfOUMHm97s8WCuCOz2cXdKSmtctfK/DkNPsnwv0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4743
X-OriginatorOrg: intel.com

Hi Peter,

On 4/5/2024 3:04 PM, Peter Newman wrote:
> Hi Reinette,
> 
> On Thu, Apr 4, 2024 at 4:09 PM Reinette Chatre
> <reinette.chatre@intel.com> wrote:
>>
>> Hi Peter,
>>
>> On 3/25/2024 10:27 AM, Peter Newman wrote:
>>> __resctrl_sched_in() is unable to dereference a struct rdtgroup pointer
>>> when defined inline because rdtgroup is a private structure defined in
>>> internal.h.
>>
>> Being inline has nothing to do with whether it can reference a struct rdtgroup
>> pointer, no?
> 
> No, but it has a lot to do with whether it can de-reference a struct
> rdtgroup pointer in order to obtain a CLOSID or RMID, as doing so
> would pull the definitions for struct rdtgroup and struct mongroup
> into an external header. Before doing so, I would want to make sure
> implementing  __resctrl_sched_in() inline is actually adding value.

I expect that each architecture would need architecture specific task
switching code and by pointing to rdtgroup from the task_struct every
architecture would need to know how to reach the CLOSID/RMID within.

Having architectures reach into the private fs data is not ideal
so we should consider to make just a portion of rdtgroup information
required to support switching code accessible to the architectures, not the
entire rdtgroup and mongroup structures.

..

>>> +static inline void resctrl_sched_in(struct task_struct *tsk)
>>> +{
>>> +#ifdef CONFIG_X86_CPU_RESCTRL
>>> +     if (static_branch_likely(&rdt_enable_key))
>>> +             __resctrl_sched_in(tsk);
>>> +#endif
>>> +}
>>> +
>>
>> include/linux/resctrl.h should rather be divided to accommodate code
>> as below:
>>
>> #ifdef CONFIG_X86_CPU_RESCTRL
>>
>> static inline void resctrl_sched_in(struct task_struct *tsk)
>> {
>>         if (static_branch_likely(&rdt_enable_key))
>>                 __resctrl_sched_in(tsk);
>> }
>>
>> #else
>>
>> static inline void resctrl_sched_in(struct task_struct *tsk) {}
>>
>> #endif
>>
>> so that core code does not get anything unnecessary when CONFIG_X86_CPU_RESCTRL
>> is not set.
> 
> Will do.

I think this needs more thought. rdt_enable_key is x86 specific now and should not
be in the fs code. Every architecture will have its own task switch code, with
__resctrl_sched_in() belonging to x86 and thus not something to be directly called
from the fs code.

Reinette