Received-SPF: pass (google.com: domain of linux-kernel+bounces-208967-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <a673b421-0857-4220-a7db-2d58bfc07981@intel.com>
Date: Mon, 10 Jun 2024 14:58:14 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 00/16] selftests/resctrl: resctrl_val() related
 cleanups & improvements
To: Shuah Khan <shuah@kernel.org>, Shuah Khan <skhan@linuxfoundation.org>
CC: Fenghua Yu <fenghua.yu@intel.com>, <linux-kernel@vger.kernel.org>, "Babu
 Moger" <babu.moger@amd.com>, =?UTF-8?Q?Maciej_Wiecz=C3=B3r-Retman?=
	<maciej.wieczor-retman@intel.com>, =?UTF-8?Q?Ilpo_J=C3=A4rvinen?=
	<ilpo.jarvinen@linux.intel.com>, <linux-kselftest@vger.kernel.org>
References: <20240610151457.7305-1-ilpo.jarvinen@linux.intel.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20240610151457.7305-1-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?Q2R3TWgxbGUwOGVYeEVOKzhJamRWK1ZNckQwVjF3Ty9jL3g0Zy9CZTVNcHBG?=
 =?utf-8?B?b0FwVGJ6UGZCem9rL210VHZQOEw4ekY4UGk5OFdHMVoyUFhQbUt4ZDl1NzB2?=
 =?utf-8?B?T3pVUEY4ZFhJdEZaalAxeGJnNWNCUjJ5dlFTS1FndEkxTUFXNyt0Tlp5OEVw?=
 =?utf-8?B?ZEl0anVUSG82N2wxdXFCMG5KN0hHSkU3R01oeHpadU9YUzdtajZ5T3FnUW9R?=
 =?utf-8?B?cGZqRStzOU5OWGVXNVJOSkZyTDZhdVMwUTlLMmdXRGVKTWNHaWNtSUlVUXNm?=
 =?utf-8?B?WnpvOG45bkNOaGVZc0M2M0pLRjViY2QwUkZldkJQUHN6VS9kNEM3MVNsdnQ1?=
 =?utf-8?B?b0RGYVBPbFVpdkh6bHZhZTk0SWVUandyTjY5Qkl3bDIyTnJZYzhEK2p0MjdO?=
 =?utf-8?B?QlZZSG9rZElobFdoeXR5R1AweGZ1eHZMc0ZYdHBIM1lJb2hDbElPMW5ST1E2?=
 =?utf-8?B?c29HRWY0ZVZnUXNnV0lRWTVsN1BWUVJuSkdqc1I1ZGNRZjdPVi95SHg1WER0?=
 =?utf-8?B?cTdneXpXb3lHWUpBSHVpWFJVTEk2bGRuWENzcmd6U0xVN3Y3TGlFQVhKQmxw?=
 =?utf-8?B?NWFsb0Y0c1FJZWNMcGpVdDNqcVBETzVBZnNkb3Q2RXpDb1dNdmRUK21rWDZ0?=
 =?utf-8?B?dlVqZm1EdFpTNkgwbHdLVzZCbC9YcWlkZHM4VG55QUxaVUJDak5qTS92QnhH?=
 =?utf-8?B?WlMxOERKRitsQ0xLdUhsSE5FeE5ZUkVBZnk3Y1Y2YWhNRUU5U1c1WDRxZXVx?=
 =?utf-8?B?OXNDT3NHSm9GRGJmeStBT291ZzRjdEZsSlZ2VUdwWk9ZM0ltQXFFUTBNYVdD?=
 =?utf-8?B?QWNpdllBdFRpYWxnM1pZWFhOY3orbVRVZG1HMXRidUY0UnlkL3o4YU9KMm9a?=
 =?utf-8?B?dkZmK3o4R2R0dWg4MHVhdFVOYnJ1bEdsNS92aGVNOC9UTE9XeGx5U01rUVRF?=
 =?utf-8?B?ODMzUnUxSlRGaGdCZSs3K2c3MEVla3pIYmVnN2IwaGZYSEFtOGJRcUFOVm5Y?=
 =?utf-8?B?T1RXdks1dmpuMGxOazF5UjZ4aXhJek8yaVF2d1dzOXB4T0UzeGNITnFZcXdM?=
 =?utf-8?B?QkZOakx4QlhMa0gzbVlWNWpzM2tsd0xOYTNkNHk3M2J4Q2s0OU1YNEtrTXNl?=
 =?utf-8?B?czhrM1p3aU1sSmUxTkV3b0lOekpXOGlyYkJ3QVRNVzBka25COWc3Y1pTWCtR?=
 =?utf-8?B?Y2kvVHBmNVNVRkhqaUFraGY1ZTJBd3VFcjdBUHUrTk9BOFgvY0t1NVk3RDA1?=
 =?utf-8?B?MGJ0Q1crY0tPS0N1VGI1MW9KVnBRRGFZWlpLaEJiU1hHR0J1amRTSWJqNHFa?=
 =?utf-8?B?VXFCNG8vZTVXSGx5SVFYQ2IyaHlXVTJldVlMNmJDOXJFS1Fvc2xmekU1cHps?=
 =?utf-8?B?MUFzOXBIV0U0QzFXU0pIa1NrbkQzRmlaeWxwL0RrRXg0OHNUVDN6Y3QyWHZh?=
 =?utf-8?B?YlltSFIvZDV4bWhrS0gzMndxR1dPOCtTbmNvZEQzWXF2QTlVZk9kNHZqaURw?=
 =?utf-8?B?cVhKQ1oyUWJKd1Y5S1hPN25uOCs4YVZHUXlLNTRTQkVxRGo0ZFhuYndIQ0pD?=
 =?utf-8?B?RjgwQ08ySWNNZ2VPeDU0VWFMNFpodVdPdjFIUEI4NS8rOHpsYmdlbVBZckUw?=
 =?utf-8?B?NnBNYkVPTkFpZTFqNVNDOEs3NGVWZkM2VXo5QWtSMlBKYTBxTEtmZGRKNXNn?=
 =?utf-8?B?MVkrVFY5LzRiSURReUpXSTY4RTNpN09qTUhrUUtRMVdWT1g1QjhFTWw2d1Qx?=
 =?utf-8?B?VWlDUkNVWXRzZWFRNHhSU0o1YWRJNnhvWjcvVUxIOXVUU2hRbnVHSk5odUd1?=
 =?utf-8?B?aW5JWm94Y0RISG4yaFVNSlNta255blVDc3pnTXlRcEg5NjNBVnJ4L1VMUWU0?=
 =?utf-8?B?UmIza3JQV2kwZElnRW83R21XbnRRL25QS25nVnEzU1I2cVR0cFZSb0JvdnZm?=
 =?utf-8?B?eXNNMlY1NHMxZU40QzcraDVDQWR2U3ZVVTRXN05JSWtwTlhLLzhjMkhTd3B6?=
 =?utf-8?B?a2QzNmw4MkRHa3pQMDBlaVd0TWRLWWNsQkpleC9Fb0t4ODgzL0JHMFJMMnJR?=
 =?utf-8?B?ZTZhOGN1ZnZ3a0VaR2FFK1cwcUlXYXJzUFh2SGp3SGZkWGZzQnI1dFcrcWl2?=
 =?utf-8?B?eVJpWVA5SFhqYWUxTzNORTBHTWgrSkVXUTNwcnVqdlFlekVlRzk4alZlRnhK?=
 =?utf-8?B?b0E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5f76b8e6-0ef1-4972-fc1f-08dc89986ed0
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Jun 2024 21:58:16.9447
 (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: Ffl5TIG8Fm9zpFk3J4jO1c2WPIEOwABLHJ8hxJKXlbN+8OSN5ZMrhVjSsDsBRWa0jUP9HPSGxNP1pofiNBG2M/QV01g3vHfThOwofvht21M=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR11MB4688
X-OriginatorOrg: intel.com

Hi Shuah,

Could you please consider this series for inclusion?

Thank you very much.

Reinette

On 6/10/24 8:14 AM, Ilpo Järvinen wrote:
> Hi all,
> 
> This series does a number of cleanups into resctrl_val() and
> generalizes it by removing test name specific handling from the
> function.
> 
> v7:
> - Truly use "bound to", not bounded to.
> - Fix separator to use 3 dashes
> 
> v6:
> - Adjust closing/rollback of the IMC perf
> - Move the comment in measure_vals() to function level
> - Capitalize MBM
> - binded to -> bound to
> - Language tweak into kerneldoc
> - Removed stale paragraph from commit message
> 
> v5:
> - Open mem bw file only once and use rewind().
> - Add \n to mem bw file read to allow reading fresh values from the file.
> - Return 0 if create_grp() is given NULL grp_name (matches the original
>    behavior). Mention this in function's kerneldoc.
> - Cast pid_t to int before printing with %d.
> - Caps/typo fixes to kerneldoc and commit messages.
> - Use imperative tone in commit messages and improve them based on points
>    that came up during review.
> 
> v4:
> - Merged close fix into IMC READ+WRITE rework patch
> - Add loop to reset imc_counters_config fds to -1 to be able know which
>    need closing
> - Introduce perf_close_imc_mem_bw() to close fds
> - Open resctrl mem bw file (twice) beforehand to avoid opening it during
>    the test
> - Remove MBM .mongrp setup
> - Remove mongrp from CMT test
> 
> v3:
> - Rename init functions to <testname>_init()
> - Replace for loops with READ+WRITE statements for clarity
> - Don't drop Return: entry from perf_open_imc_mem_bw() func comment
> - New patch: Fix closing of IMC fds in case of error
> - New patch: Make "bandwidth" consistent in comments & prints
> - New patch: Simplify mem bandwidth file code
> - Remove wrong comment
> - Changed grp_name check to return -1 on fail (internal sanity check)
> 
> v2:
> - Resolved conflicts with kselftest/next
> - Spaces -> tabs correction
> 
> 
> Ilpo Järvinen (16):
>    selftests/resctrl: Fix closing IMC fds on error and open-code R+W
>      instead of loops
>    selftests/resctrl: Calculate resctrl FS derived mem bw over sleep(1)
>      only
>    selftests/resctrl: Make "bandwidth" consistent in comments & prints
>    selftests/resctrl: Consolidate get_domain_id() into resctrl_val()
>    selftests/resctrl: Use correct type for pids
>    selftests/resctrl: Cleanup bm_pid and ppid usage & limit scope
>    selftests/resctrl: Rename measure_vals() to measure_mem_bw_vals() &
>      document
>    selftests/resctrl: Simplify mem bandwidth file code for MBA & MBM
>      tests
>    selftests/resctrl: Add ->measure() callback to resctrl_val_param
>    selftests/resctrl: Add ->init() callback into resctrl_val_param
>    selftests/resctrl: Simplify bandwidth report type handling
>    selftests/resctrl: Make some strings passed to resctrlfs functions
>      const
>    selftests/resctrl: Convert ctrlgrp & mongrp to pointers
>    selftests/resctrl: Remove mongrp from MBA test
>    selftests/resctrl: Remove mongrp from CMT test
>    selftests/resctrl: Remove test name comparing from
>      write_bm_pid_to_resctrl()
> 
>   tools/testing/selftests/resctrl/cache.c       |  10 +-
>   tools/testing/selftests/resctrl/cat_test.c    |   5 +-
>   tools/testing/selftests/resctrl/cmt_test.c    |  22 +-
>   tools/testing/selftests/resctrl/mba_test.c    |  26 +-
>   tools/testing/selftests/resctrl/mbm_test.c    |  26 +-
>   tools/testing/selftests/resctrl/resctrl.h     |  49 ++-
>   tools/testing/selftests/resctrl/resctrl_val.c | 371 ++++++++----------
>   tools/testing/selftests/resctrl/resctrlfs.c   |  67 ++--
>   8 files changed, 291 insertions(+), 285 deletions(-)
>