Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7aad2648-6adb-ebfc-a7b7-db959c2e85c4@intel.com>
Date:   Fri, 21 Apr 2023 17:20:57 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.10.0
Subject: Re: [PATCH v2 20/24] selftests/resctrl: Move CAT/CMT test global vars
 to func they are used
Content-Language: en-US
To:     =?UTF-8?Q?Ilpo_J=c3=a4rvinen?= <ilpo.jarvinen@linux.intel.com>,
        <linux-kselftest@vger.kernel.org>,
        Fenghua Yu <fenghua.yu@intel.com>,
        "Shuah Khan" <shuah@kernel.org>, <linux-kernel@vger.kernel.org>
CC:     Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
References: <20230418114506.46788-1-ilpo.jarvinen@linux.intel.com>
 <20230418114506.46788-21-ilpo.jarvinen@linux.intel.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20230418114506.46788-21-ilpo.jarvinen@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZHBnd2F5U20vUXZxdGdGbnVoY3dDSUxWQUl3NjRFMUc4a1JQRUlOejV2S0Fh?=
 =?utf-8?B?bTJWMzhlMldqOW9lVmtxNWdMTkFValY5UFMzQWh4dkIyYTEvUG1yUUROcFlp?=
 =?utf-8?B?WVo1Y2l3REozcHRuV3M1aVU3T2dYUUVXL283U05ucXV4MlI1WjNHQzdKMk5X?=
 =?utf-8?B?Q0FiVDlEaXJZMGd2cEUybHA5TUFSWVJSRWpoc0NVQzJCZTJ0R0dZVmRGMXdD?=
 =?utf-8?B?NzRnaVNSTTBROGlQOWlyQTI1U0dTRDNVNUN3aTJTYWliRC9jQXRjNW01TzJo?=
 =?utf-8?B?OUR4UU5QemVEdmhyRURUREZaVFVmeHd4SzJyUnAxTjlaczV6VFJ6aVFURzVZ?=
 =?utf-8?B?ajdDMVRnd08zamZHb3dYRm9wWUUzU3RsNktKTXB6UGlZY0tzQVNCeUlpUDhm?=
 =?utf-8?B?amZnWUxvYTZ1L1ZtZEpiTXh6S3VGTlRPTWpuMnFDeC83RHV4dWM2bUU3cnRw?=
 =?utf-8?B?aHI4cjZWdjNFZkdESExYSWJDT2JGU1h2L2J6VDNqQ1ZacUl0MEF5NDcrU3JI?=
 =?utf-8?B?L0gwdnl1cG12Y0pBdmxNOXFLMUIySmxUeC9MT2ZKYnVnbStrRTdDeUhSZ0xC?=
 =?utf-8?B?RjVVNk9qVnBwREdnbUVzUXFybkRJOEpaMTh0MVRlL2NpQWtSeHhOZmVIOFhs?=
 =?utf-8?B?VmFNeG04TVlacW56N3RKenpaYmFpWXdjOFpDaVRuWS9FWUxvQU1zbEd3WlRG?=
 =?utf-8?B?cjVHcnkxSjBsZWo1aU5jS0tvSUxsdUtVRjBQMGxlTHFTbDhxVFU5akwwaHNO?=
 =?utf-8?B?dytWYkxGV1BaTTdtYUNQdTg5Um1YUFZaUnRzRGR3ZnVIVkVQbUp0dHhmeTNP?=
 =?utf-8?B?MzlJWmhjeENJS0o2YldHTVFySlJOUXVUb3I1cFA0eC91Wk40VHFaT1k1N2JR?=
 =?utf-8?B?ZmJvYjVwdnpDd0JtSUlia1czSkwxaDRTNHgrWDVVWGFVNk1DS1FDVHV1ZGZ1?=
 =?utf-8?B?OTRsRFhxS3V4bDUwZStWUW9MQkJxdHBQMmU0azVmYmNNdTFTUDUyL0F1eWdT?=
 =?utf-8?B?SXIyYVdnakFFYWJjUVBDZWtLRnRpcmt1UGhLMWZJa3BtdE4zUFgydG1XRUpn?=
 =?utf-8?B?NTVOY0kzR3RPSUVBUlJuZHR3eXBTQXd6YVNPQkMzZ1lzSytxYUZ6Qlp1V0Fy?=
 =?utf-8?B?UGxjRmNhMVFreEdkS3lRQSs5bDdqYWpMbkpzRUZYclZCaG1UNG0wL3ZFTXUv?=
 =?utf-8?B?MytkUkJRbVo3QkswQlVYb3RqV2dyRktSQi9rNnRXOUtML1A5bis5eWNBdm15?=
 =?utf-8?B?c1hrNTAwWFBUK0VPdGRkakhQM0dNbFF6UExkNjNWSEhLRnAybVBnVnhZZm5r?=
 =?utf-8?B?UEkrdkE2TEtkUWt3TDcyYXZJcVFFT0JNR2lldzZUVWpqb3ZzejFVWlFQUnRI?=
 =?utf-8?B?eUFINmFQVEZKYmNRTlRYYTJJd2M0YTNOQzhpS282NkJWL3FBallpWndKbkNz?=
 =?utf-8?B?RldySmlxODdPU09vOUFsSFhockNnTGlSajZNSmJvMDJXYWFxbEZZQnQ2VUlZ?=
 =?utf-8?B?RGNlbHBvb2N5aTBXSlBiQUhXb1pjQmZ4ZTV2QnBWWHdHMTIzbk0yU3lyeEZS?=
 =?utf-8?B?VjMxYWNYbDBURkJOOFBSdFV6VEdVZHppakpzaWJ2QjFLSVhLS2o2WEVFVnlY?=
 =?utf-8?B?NTBGNVVzSnA4UXVUYkQxeVdSd0d2amxkWGdWa2g1OE1tNi9haVNwSGVnWmo4?=
 =?utf-8?B?OEpGbFRyejVIU3oxb0psblhQRzZqQWFxRUt3Q0hlY0FMRlVaTTJRSWhhVnQv?=
 =?utf-8?B?cmRIc0phbG9NVWlJUzl6bzM0cWxzaHFxRDRKNkg1UXZSRzRPTllRdFdHRS9y?=
 =?utf-8?B?YVNlRnhYQWlLVjBRaWlZZTRMNW4vY3ErQm9GWk5FejRrRHBSNjFBWFh3TG9M?=
 =?utf-8?B?dzVTN2UveXIyZnc3aFBJNklreVZJVGdkQ2UzNm1lVjR0QjBYV2E0a3VlSlVT?=
 =?utf-8?B?K0c5T05vMUJ0RTkrc1JybjI0RHAxTW1NdDBaRll3YVJ6QzE1Y3dnUW9PemEx?=
 =?utf-8?B?anhnUEhYb3hJUG9VU3lxR3NvdUVWUEErNVpMUVRJR1V1WHRTNGFRUkd3RmlS?=
 =?utf-8?B?a3JGU0xzN0FXczZCV3I0b29Ha25acWxqR00rMmd3MDJ6WkQ0a004NWNrcnpW?=
 =?utf-8?B?VE5OWWlpTUdOMVpzRGNVNEh4bjA4a2NKeFlZbUR5VWNQVWQ1ZU5Bb1hjbEE5?=
 =?utf-8?B?Y2c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 099ebca4-571e-494c-ce30-08db42c77348
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Apr 2023 00:21:00.5336
 (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: Sn0BJB1mJPxT6/QZZv0kFifG55ZSZi+Hvj5K+hNbA05aw41ki8XTt6j6yy9d9LzTydz3hOx0SErgcm1l5l2kxbeINYqvSDcBwEFOLke8G6I=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR11MB6304
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Ilpo,

On 4/18/2023 4:45 AM, Ilpo Järvinen wrote:
> CAT and CMT tests have count_of_bits, long_mask, and cache_size global
> variables that can be moved into the sole using function.
> 
> Make the global variables local variables of the relevant function to
> scope them better.
> 

Could you please move this patch earlier, before usage of long_mask in
earlier patch.

> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  tools/testing/selftests/resctrl/cat_test.c | 7 +++----
>  tools/testing/selftests/resctrl/cmt_test.c | 7 +++----
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index ae21e656cf6e..ef3ba22bdde5 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -17,10 +17,6 @@
>  #define MAX_DIFF_PERCENT	4
>  #define MAX_DIFF		1000000
>  
> -static int count_of_bits;
> -static unsigned long long_mask;
> -static unsigned long cache_size;
> -
>  /*
>   * Change schemata. Write schemata to specified
>   * con_mon grp, mon_grp in resctrl FS.
> @@ -95,6 +91,9 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
>  {
>  	unsigned long l_mask, l_mask_1;
>  	int ret, pipefd[2], sibling_cpu_no;
> +	unsigned long cache_size;
> +	unsigned long long_mask;
> +	int count_of_bits;
>  	char pipe_message;
>  
>  	cache_size = 0;

Seems like this initialization can be moved to the definition?

> diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
> index 087378a775ee..6adee08661e7 100644
> --- a/tools/testing/selftests/resctrl/cmt_test.c
> +++ b/tools/testing/selftests/resctrl/cmt_test.c
> @@ -16,10 +16,6 @@
>  #define MAX_DIFF		2000000
>  #define MAX_DIFF_PERCENT	15
>  
> -static int count_of_bits;
> -static unsigned long long_mask;
> -static unsigned long cache_size;
> -
>  static int cmt_setup(struct resctrl_val_param *p)
>  {
>  	/* Run NUM_OF_RUNS times */
> @@ -74,6 +70,9 @@ void cmt_test_cleanup(void)
>  
>  int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
>  {
> +	unsigned long cache_size;
> +	unsigned long long_mask;
> +	int count_of_bits;
>  	int ret;
>  
>  	cache_size = 0;

Same here.

Reinette