Received-SPF: pass (google.com: domain of linux-kernel+bounces-195877-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <b53645d0-2061-43ec-8524-d60c090b896b@intel.com>
Date: Thu, 30 May 2024 13:21:34 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v19 09/20] x86/resctrl: Add new fields to struct rmid_read
 for summation of domains
To: Tony Luck <tony.luck@intel.com>, Fenghua Yu <fenghua.yu@intel.com>,
	"Maciej Wieczor-Retman" <maciej.wieczor-retman@intel.com>, Peter Newman
	<peternewman@google.com>, James Morse <james.morse@arm.com>, Babu Moger
	<babu.moger@amd.com>, Drew Fustini <dfustini@baylibre.com>, Dave Martin
	<Dave.Martin@arm.com>
CC: <x86@kernel.org>, <linux-kernel@vger.kernel.org>,
	<patches@lists.linux.dev>
References: <20240528222006.58283-1-tony.luck@intel.com>
 <20240528222006.58283-10-tony.luck@intel.com>
From: Reinette Chatre <reinette.chatre@intel.com>
Content-Language: en-US
In-Reply-To: <20240528222006.58283-10-tony.luck@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NzlDL002K2drVlUzdjVSRVQvVXNJa0ZUR3BFTThRMTFITC9jUXJUS090T2M4?=
 =?utf-8?B?NUpwbmF6Z0J3amRTR2R4UjB1RHBOMkNENTNvcm1VS3NYSWJkNDRIbzg3QnNs?=
 =?utf-8?B?bTdkNzlLN0xLV0M0cDllWUE5MFBMR05SNS92T2w3TTNJd2R6SGozZmJsczBQ?=
 =?utf-8?B?NXMraWMyamhLZDRKdXNzWGxEYVJxTzhwbmxrODR3eWRqdXZHSFZEWC9GajNm?=
 =?utf-8?B?K1pmMlJGSFNVWDFpQll2Tk4zdVFhcmJLYUJoaEtuMjM5MnM4T0MrT2lXVGEv?=
 =?utf-8?B?ZElTanlhQWhUM0VYNDFpMnNWdTNtWHQzYjFYeFJCVTQ4M1FzWDhLOXNJNzdO?=
 =?utf-8?B?dVMyNzIxeEpiakxJTHkxSk5MWmV5UW9WamJYVzJVQ3lGMmVZQVZ6VUlKMGpa?=
 =?utf-8?B?Q1graDJSSWI1THIzU1EzVGZZZWh1Mi95SmExRkJ6b25Tc0dZM3JwcGZKcHpJ?=
 =?utf-8?B?ZDRWRkV5MEI4M05wL08zdjg3aUNmZjhhV3dzYXFXK1J3cEpLQUJEeTR0aUFt?=
 =?utf-8?B?cURoWUF6MWc1eTJvWG9CZUIxUVhSMU1SWm9rU2hOVkYwZmxEcWthSnZaNEtD?=
 =?utf-8?B?dSt2Nk15WDhmeU1sTThMek0zMjM3SGRvU1dpZ2NlSmFnUjNyMVp2a0VMa05s?=
 =?utf-8?B?dEVOWHVkaTBDRkNlakhBV054d0t3WnQzWldqUHh6d1Z6QUNqUzdaMkE2NTZS?=
 =?utf-8?B?b0xva3YwY3dKQmZDTldYbHc1NElEeW5sOWQzeXZzQTVPUStuWWs1bk5aWHZB?=
 =?utf-8?B?WjlyQ2RSN3hOd1AzWEJiTEpEWEY3N21zQVVtNm9IeWxWZ2Q2R0h5MHdCM240?=
 =?utf-8?B?RU5veUxMeVlwSnkxWUxsTExCWWdpSDZFVzRWZFNSSEF4SXJVKzgvZHR2TXJD?=
 =?utf-8?B?NWdJMC92MTQwOUFGMXJvWTVCWlQ0QlZCUDFHU285R01SWnpMTVV0T1YwY3NG?=
 =?utf-8?B?THpkdmt2bi8xellyT0ZPSU5uSkJqSGp6dk9kbmNNMS9rOHBBREI3Y01CMFYy?=
 =?utf-8?B?OG1NMTZ4UFNBbmcyNnFBdHNXakVZM29zbUhzcE1jV0VlV3lVaGJJbUtUWDd5?=
 =?utf-8?B?aTlFWFNzRGwxTUhJYWFRQmhLZ0xOY1hTcDRyRHVDMkVHZ082NTNrRmR3YXFx?=
 =?utf-8?B?Y0ZQTzM1TkNQanBHcHEvc3A0c1d6OHprL3VhSVJxKzBEczg4eExLZ3NhY3Nt?=
 =?utf-8?B?RUZlMGJtMmZ4aE5iek12aGVmT2hYdmZUenZYZGUwWEZKY2pTUkZaZTdwY2hY?=
 =?utf-8?B?QmNGL0NDRjFWYk5rbk4ydE5WNnVOelpXS3J5YzZKUGVSTm4zODZjMjRBeS9E?=
 =?utf-8?B?eW9IUm5pTllVaml1aHA0aCtkUTFFMVR1Z1hvNUtxbGpKcXJZYVNtNVBGdkFG?=
 =?utf-8?B?SXkzUDNQaWphbUoxcERianJ2QituMEhiV1p3bzhvZFh5MG9FUmx1MzY0cGJD?=
 =?utf-8?B?N250dDBxQVRIRkJiK2JKWTlMNjlKb2VrSGFWbXptMlUwbWpxL3VGU1dlN2xF?=
 =?utf-8?B?OXpVcGFoSEpvbzJSNmdtU2g5dmNNVExpdC9jdzZycDdrdlUxbkRZNWR1OUwy?=
 =?utf-8?B?V1Y5UlZqdjYzUXNuQjNEU0NHSWdFMXdwdUtXNmlaOE1vOGxCUW9tRit4RThI?=
 =?utf-8?B?YjREMXdLMDNBc3RMVHM5d0NTek9ON0VjTDNPL29QaFpxdUtvcUl0WDJ0N1N5?=
 =?utf-8?B?cVF4bHFnM0h0TzlFT0lhVFJFbSttY1dmcXByWnEvamQrVGxzSzZvY2xydmwy?=
 =?utf-8?B?T0lSOGlTZGoxZE50cDJhdUtjYzlKZkRxMFMveEFoaFZIbzZLcjhGYTFOc0xz?=
 =?utf-8?B?WTVBKzBUNm1DcEpNdkFyMWwxSGlnQjkvU2k1VEQxVXI3NWRNck1KU3lpZThC?=
 =?utf-8?B?cVBxK1IzMTBwaFduT1FiOXhrWUxWZjMyQWhBZndnM08yUFZDa2t2dWR3dmhU?=
 =?utf-8?B?VnpYakhXN3JwbHhNZ2NEMzdzSEQva0dBdHpHTkJtQ0xjTXplZm01bXRHd3Zh?=
 =?utf-8?B?d1hOUURsOTYyeFpGazQyMlc4MW83RzFCTFBtbzRXMS9veFRPTnRYMUIwbnZn?=
 =?utf-8?B?Q1IwSnAxN0tFSGFZV2Z5dlBEMUlRVlU0ZDYrT09nUmhycE9sRDlZblNZZWpH?=
 =?utf-8?B?azMvUE1NcHRzY0J5UTh2T25oM1NNUHozYUtzaG5ZUldTeXdHUUhrYUdkbTht?=
 =?utf-8?B?Z3c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c8c631a2-2810-44ab-7df4-08dc80e61b22
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2024 20:21:36.6876
 (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: LzQojXfwQkRcW2DafeXTbqG4MUEon6fpx8AK8R8hDuSv6yZyCx5CiCWKwEeT0Fv048rR0le2i00DXc29j6l/RrSeuLIHwLGt3twbhJHyaCc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6453
X-OriginatorOrg: intel.com

Hi Tony,

On 5/28/24 3:19 PM, Tony Luck wrote:
> rdtgroup_mondata_show() calls mon_event_read() which calls
> mon_event_count() which packages up all the required details into an

No, mon_event_read() does the "packaging".

> rmid_read structure passed across the smp_call*() infrastructure.
> 
> Legacy files reporting for a single domain pass that domain in the
> rmid_read structure. Files that need to sum multiple domains have
> meta data that provides the L3 cache ID for domains that must be
> summed.
> 
> Add the sumdomains and cacheinfo fields to the rmid_read structure.

This just describes the code and that can be seen from patch. Please
check all changelogs in series for this.

> 
> Add kerneldoc comments for the rmid_read structure.

Same.

> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>   arch/x86/kernel/cpu/resctrl/internal.h | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index eb70d3136ced..d8156d22cbdc 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -145,12 +145,28 @@ union mon_data_bits {
>   	} u;
>   };
>   
> +/**
> + * struct rmid_read - Data passed across smp_call*() to read event count
> + * @rgrp:	Resctrl group (provides RMID value)

Provides much more than RMID so either make that accurate or drop the annotation.

> + * @r:		Resource
> + * @d:		Domain
> + * @evtid:	Which monitor event to read
> + * @first:	When true this just requests initialization of an MBM counter

Seems strange. Perhaps just "Initializes MBM counter when true."

> + * @sumdomains:	When false just return monitor count from domain @d. When true,
> + *		sum all domains in @r sharing L3 @ci.id
> + * @ci:		See @sumdomains
> + * @err:	Used to return error indication
> + * @val:	Used to return value of event counter
> + * @arch_mon_ctx: hardware monitor allocated for this read request (MPAM only)

Stay consistent with descriptions starting with upper case.

> + */
>   struct rmid_read {
>   	struct rdtgroup		*rgrp;
>   	struct rdt_resource	*r;
>   	struct rdt_mon_domain	*d;
>   	enum resctrl_event_id	evtid;
>   	bool			first;
> +	bool			sumdomains;
> +	struct cacheinfo	*ci;
>   	int			err;
>   	u64			val;
>   	void			*arch_mon_ctx;

Reinette