Received-SPF: pass (google.com: domain of linux-kernel+bounces-194526-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <0aa15270-5de9-4fc9-a459-f1665f8d8624@intel.com>
Date: Wed, 29 May 2024 10:45:51 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 08/16] selftests/resctrl: Simplify mem bandwidth file
 code for MBA & MBM tests
To: =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>,
	<linux-kselftest@vger.kernel.org>, Shuah Khan <shuah@kernel.org>, Babu Moger
	<babu.moger@amd.com>, =?UTF-8?Q?Maciej_Wiecz=C3=B3r-Retman?=
	<maciej.wieczor-retman@intel.com>
CC: <linux-kernel@vger.kernel.org>, Fenghua Yu <fenghua.yu@intel.com>, "Shuah
 Khan" <skhan@linuxfoundation.org>
References: <20240520123020.18938-1-ilpo.jarvinen@linux.intel.com>
 <20240520123020.18938-9-ilpo.jarvinen@linux.intel.com>
From: Reinette Chatre <reinette.chatre@intel.com>
Content-Language: en-US
In-Reply-To: <20240520123020.18938-9-ilpo.jarvinen@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
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?WGxHcXQzRVZ3UG5yNUNJeW04UDVoUEJYK0xmbkM3S2hGeXpSUzZBV0lHUi9T?=
 =?utf-8?B?bWQ0RFlLbk5jVHg3NDA4ZXE5UGpmaGkrWmR3UVdRUjdBU3VWWklSZ2x6U1Ra?=
 =?utf-8?B?WGcwUWVKQWZ5M1lLZi93bjBScXU5cWFNSmNxZjZSOGx6SEVmSVNUTVg1Y3lu?=
 =?utf-8?B?UFNsbGJ1bzFBbWYvMnUxbVBEWUVHbzB3UkxUcHR6TGgxVlJvbjVjOG1lRTBw?=
 =?utf-8?B?RWduQ1V2RnNITkNqN1l4a3RWNWFXeE9YT0Iyc0l0U1RWVXZSY1dqVFFkejhI?=
 =?utf-8?B?V3pOZVB2T3Bva0FCVHF3dmRmYnZXNGtRQnFKV1dNL3JUUHBIVVNFVDdvZXJU?=
 =?utf-8?B?cFlOMy8xcGFiYU1Hb1l1eFlBeUo4RTZWam1XaDJSV2t0MitVWTJhYmQ0SHA0?=
 =?utf-8?B?czNYYXlwUzY0ZWt0cXZCRjBia0xSWkNMRmtRMTh0RlROTEtocXltZXVpWTdr?=
 =?utf-8?B?b1cxWDV4L2R4cWJtTXhwU2JkM3JlWHkrUDVSaGFWalF3ZWVGQmJ0VlMrTWJ0?=
 =?utf-8?B?aWtNZklPejF6MHRxdFk1V05LQ2NvWjAzWW02MUJIRkc4djVkNzh0Qm1PWUxU?=
 =?utf-8?B?dGw2bkpMYzNxQXRxVkJ6SUM4OUVIVTBCTko5WWlLcEVqRzY3SnpMZis5MSti?=
 =?utf-8?B?cW01ZHlLWjRqK3FHTTBtcDh5alByRXgzR2lLVENvTnE0RFBiekVDSmxWcFJp?=
 =?utf-8?B?NzJLZk16dU5EZ3R3MTk2UUtOK29CK3g1WmZDQWdNN1Jwc1RGUWowQ1lKN3hP?=
 =?utf-8?B?Q1NuWTU5ckFqdHd0TjdtZjFpaVp4YlM4N1JXT21GMm83SytCTnZqekttaGtK?=
 =?utf-8?B?cHhKOHkvbVFodzVOczJWaXIwOXEvU2FSbDVldzJOQkYzcXhJbE45ZE96elZ1?=
 =?utf-8?B?Smd4cFJzSThESnZYZU1jYjZLZUJRRVZoV3hHbTgrcVlrSUhUZ1ZrWTBGeTNw?=
 =?utf-8?B?eHg1K2hDVXZOeVpJSVNDT0M3YmNrczZjT080WFhUcXhFNGtYQm81bk1LMURU?=
 =?utf-8?B?NjJGNm5uL0xSeEgvaHExQ1VGWFljaGlKYXhwRXhjZE9XUlo4TkxMS0ExZEI2?=
 =?utf-8?B?RWxyYmxYdENIQlc4ckhLL2d5UE0yY0EzSW4vUTNaSm9haE9TREdMWUV4VnU4?=
 =?utf-8?B?UjVlSXk2Q0QyMm10dlJKQ3IydkFwTEhScElXemc0dXF1aWN3WU9FSU9PS3Zw?=
 =?utf-8?B?VytkMy9SN1cxSjllcVVkMWRDOXdlYURuTXJqV2xsYysyWjllWVFBTWZ3S0xE?=
 =?utf-8?B?dk9wdnEwYVNkb1Rmb0pKc0gzZklqM0sva3NYM0JyVVBseVlXaTFyMjJXUDNK?=
 =?utf-8?B?VEdpVExkWTBhd2JONUF5V3diQi8yKzh3M3lWbW8vdWlIckJqYUR5T2ovS1hD?=
 =?utf-8?B?YWpxN3h5Yk5DYk9neDhQWTVvblJuQi81RDVwTFoxWU9ocENpVDV2SlFTdG5C?=
 =?utf-8?B?TDgzd3VPQmZNR2V6R1J3YUpkcTcra3A5cWFyWVg5K0dlcnZCbU5FOWIvdjc5?=
 =?utf-8?B?L2hHV2YvMForWUlkcklCaFFaemRweCtZTXFFSmRNWVlEd2JuM3lEeHBZOHh2?=
 =?utf-8?B?MmJnL01SZk4rQXdmVlI3SDI0d0V2TzRaSWg1TWgvdU1ZWTVyY0RzRERNUVF5?=
 =?utf-8?B?YWpseHNWVmRHekhBRk5vNStnNTNXQmI5cWdoblhFajhXa3NhaUJsVXhwNEpv?=
 =?utf-8?B?MDRaZWV4OEtjNXlNMnRzeStsVzBvUlpLMUtGb09uQjBrWjZJSGFPcTdVaDV4?=
 =?utf-8?B?T1U1MkhrRVpBVFVHTTNUWi9wbTZnTVlCaUhUcEZKU1VscWJoYlZURkVZemNE?=
 =?utf-8?B?QVNPcDlIbVQzMUFHTEhLbVhjcnl1M0ZDZEtjUEY5MHZua1FObEFQdmY1RnBK?=
 =?utf-8?B?enhMdG9NV2VFempTN3duSm1Jb3lzMTY4b1RIWE92em5rcDFxZVpxaXZKNy8x?=
 =?utf-8?B?bkpSZEpKSVEzb2J1RDE3RFBNazNpQlBYSWNxVDJBVVlGb3RmKzNMSkorekI1?=
 =?utf-8?B?Sk9vcUgxaWNhWUE3QkpIZS9nZlFXRjUxNUMrUU5QTWxJYjMzTzdLb3VFVDZp?=
 =?utf-8?B?SFQvZlZ6N1FUVHFKSHo4dUozalVUY2ZkSDFHL0pEM0dNSTRON2x3SnQydTNM?=
 =?utf-8?B?RkFPTnoxQVc2dEtMMEtxOUhXZnZxL3lMdFRYN3ZxM0VZNUVDcG1wTHQrMmNw?=
 =?utf-8?B?VlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ab9ca04e-dab4-48e5-b97f-08dc80072fcf
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 May 2024 17:45:53.6396
 (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: KKEXqbdPtAgJvn5BwcFBl9UsQu1OBiiVZSwE3BTuBnx5D0LXOO48zBUthMAURyrnSjFwe1EG6/8SzG6V8YpIaj0cIp/9b6rBkP/4TP3mmPc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB6446
X-OriginatorOrg: intel.com

Hi Ilpo,

On 5/20/24 5:30 AM, Ilpo Järvinen wrote:
> initialize_mem_bw_resctrl() and set_mbm_path() contain complicated set
> of conditions, each yielding different file to be opened to measure
> memory bandwidth through resctrl FS. In practice, only two of them are
> used. For MBA test, ctrlgrp is always provided, and for MBM test both
> ctrlgrp and mongrp are set.
> 
> The file used differ between MBA/MBM test, however, MBM test
> unnecessarily create monitor group because resctrl FS already provides
> monitoring interface underneath any ctrlgrp too, which is what the MBA
> selftest uses.
> 
> Consolidate memory bandwidth file used to the one used by the MBA
> selftest. Remove all unused branches opening other files to simplify
> the code.
> 
> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---

fyi ... if the write_bm_pid_to_resctrl() fix in patch #16
was closer to this change it would have made this series easier to
review. That fix is clearly needed here and its omission makes this
patch as well as later patches #14 and #15 harder to review. No
need to rework the series at this point. It is just some comment on
how a simple patch ordering change can make a series easier to
understand.

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette