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;
Date:   Thu, 31 Aug 2023 10:43:52 +0200
From:   Maciej =?utf-8?Q?Wiecz=C3=B3r-Retman?= 
        <maciej.wieczor-retman@intel.com>
To:     Reinette Chatre <reinette.chatre@intel.com>
CC:     <linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>,
        <shuah@kernel.org>, <fenghua.yu@intel.com>,
        <ilpo.jarvinen@linux.intel.com>
Subject: Re: [PATCH v2 2/2] selftests/resctrl: Move run_benchmark() to a more
 fitting file
Message-ID: <v6bjvvjofvgly4k2jzvikr43mjz3ugzic2umlobjlkzchqmww3@sbw5xz6rbd4r>
References: <cover.1693213468.git.maciej.wieczor-retman@intel.com>
 <e527be8b871212823ff83f3800b6eecc2a75e455.1693213468.git.maciej.wieczor-retman@intel.com>
 <16eccb6b-a968-c6ee-dfb3-221ed8ec023c@intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <16eccb6b-a968-c6ee-dfb3-221ed8ec023c@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?mWP3NTzwr9esGuMDc9leTvZAChAkiiJLy21kjusyUt35eKo+PbBUbCZcEp?=
 =?iso-8859-1?Q?6PhyOxDfXLAjdoG1RfeW5iOWv0pWTk1U1yYJFEOw1erZ+H34wYV/T0dc4r?=
 =?iso-8859-1?Q?6nQQzulIywIqpi+gDUTDQ7nlnFdQEBKsSCniB+n6s5umKJWIn9LY/da0Vj?=
 =?iso-8859-1?Q?mUDq0BU5LPyTo8idsXxBwX4jnZnbjgY69nfXuNHOl0xRnGASbV0vBrNE2t?=
 =?iso-8859-1?Q?A7O1oiAbOG4VmhPqlsW6TJBYubkZlz69FFPrXdAEd7Mohv9vnDCELq1aBV?=
 =?iso-8859-1?Q?NM9gQVZhCsdQiRM/mniXKCon/ZeBI/dKUxJrM36GU7M6j0iF4pX4cSeMRk?=
 =?iso-8859-1?Q?OiW5Rf+WeStDjTaGqSxwIiVW3wD/gPerNejLBzL0OXYqhj4XZho38ecb0E?=
 =?iso-8859-1?Q?BlOk2eAfHg+AGvP8dSskX8wC2NEaYnAYTqUZ72OR8qu0Iyl+ig8a3LV+HO?=
 =?iso-8859-1?Q?T2W6VJdd42L9XJ0hZ+l1KQUrXSQeNBjzxAnuWYfqGQaSie8rTExRjlblbA?=
 =?iso-8859-1?Q?pNdSvq3WnPbmk0DdEj1lpkKRcJyzbWKFiAVXuzlZzP3M+rF0VlbqlZiJEJ?=
 =?iso-8859-1?Q?UFx4Y/2Nqt13ncVpLZNBCKX/ZACtikTcosLcSX5Ipa9CLCyNNznxHB5Jpq?=
 =?iso-8859-1?Q?XegvBIVZ4RynhbE9HdgJQZEdIKYMwtkFGvj19KsD2bR1EDl1GzXE6MTziZ?=
 =?iso-8859-1?Q?LlS9MEdVxpc1uxEHFf2Ep/u/6uA/+gsWc3NLgYYX/g8BFwYu2OFCQ3z5yH?=
 =?iso-8859-1?Q?x0d6wFHqWfiH2CBdTLUNpot3Tg2iq6Y2h043EfZngBibpqbQZ28Je7ovT/?=
 =?iso-8859-1?Q?3GlYkdQSkR2rM7znm7G/u3DTg4vnUPW+guKwrdJ51cwaOsiB6js1J//aN6?=
 =?iso-8859-1?Q?5O0JaSvtbiJkZYndxtnxOGSfnFOgzj60xwDKsQraN5Wp8sygSZflDmSRgB?=
 =?iso-8859-1?Q?FenWXMQkXXb9LYEesNjDpGVOLygdwf0cJVCKZiurJii35Xz8n+hR2o4xh8?=
 =?iso-8859-1?Q?uF2Upyy5UrqChxSsGWsw5F3Wyiv4La7fDs135gyyE0Dz8KiPM0tzhJLPdm?=
 =?iso-8859-1?Q?o7SdVhUmBBiy21tPsSbK0UTFjKQObOKkddz19rgOj4VuF2ENcpPbGmjdAq?=
 =?iso-8859-1?Q?hEgr7SwqAMxzF6Lat50zECJ9HE53ivLK19LyvKrer/u2iN/3gJSBNzyAWp?=
 =?iso-8859-1?Q?nJGoYrXt65xRPB5D8tXyGD2/LBtldzzepnyJOBUe1uAjrWu0reeQiggwRq?=
 =?iso-8859-1?Q?bnif8US2v5+lQaV7LWqV7JzOPwUOXvWMMKjZp7wbLC/+zwl1HcOSwC5ofB?=
 =?iso-8859-1?Q?FT9Xa5TmCeJuifEqPt16i08QHRYyFr+GdMRjcpFDAQyAreBEaVgy/Su+GQ?=
 =?iso-8859-1?Q?PsGM+4v/aK8DmZduB8UycyDoXRdCTF3d/j283zSyxjwnLMiFpmmq4SegZE?=
 =?iso-8859-1?Q?mEa3YnfnMa1c7VxwZsHtbrKOheF/EyYrzm0ebTDp2ahujIeeu3Cal+3YOZ?=
 =?iso-8859-1?Q?PeZuyFzlEJe/W+i6yeMKZ9aNfMdwH5aR6yQjdqf9hYSAvDpZFG2Yh6igtY?=
 =?iso-8859-1?Q?p0659fTnwMNhOFYarABCiTBWz0wgrWG6TmmfflMGPwZ3iFV9v+btTRfeLr?=
 =?iso-8859-1?Q?A3RlcQ2Sl7Y0c4HV2cfGvrqh5i8s1Sw8IA1MNqJJTOOfFx4FA7vpQbJyjy?=
 =?iso-8859-1?Q?YgOMMSadx+zVJwnSlzc=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 38d299cf-8df3-460a-c437-08dba9fe6dae
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Aug 2023 08:44:03.6656
 (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: 2iJ6D1mZ0/Tu++g1m6cO0uflCyxWPPHeoHENkkGUQguU9EJ+0yMWWgtPB0zI0UVZZLZxt0j33rFcAUdfJQ6wQl3hAuYuzR3F8RO5J8PpTDw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5939
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE 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,

On 2023-08-30 at 13:51:29 -0700, Reinette Chatre wrote:
>Hi Maciej,
>
>On 8/28/2023 2:56 AM, Wieczor-Retman, Maciej wrote:
>> Resctrlfs.c file contains mostly functions that interact in some way
>> with resctrl FS entries while functions inside resctrl_val.c deal with
>> measurements and benchmarking.
>> 
>> Run_benchmark() function is located in resctrlfs.c file even though it's
>> purpose is not interacting with the resctrl FS but to execute cache
>> checking logic.
>
>It looks like your editor may be automatically capitalize first words
>of sentences like Resctrlfs.c and Run_benchmark() above.

I'll fix it.

>Also please note that when using () to indicate a function it is not
>necessary to say it is a function. For example above can just be:
>"run_benchmark() is located ..." ... similarly you can just say
>"resctrlfs.c contains ...".

Thanks for the tip, will apply it from now on.

>> 
>> Move run_benchmark() to resctrl_val.c just before resctrl_val() function
>> that makes use of run_benchmark().
>> 
>> Remove return comment from kernel-doc since the function is type void.
>> 
>> Changelog v2:
>> - Add dots at the end of patch msg sentences.
>> - Remove "Return: void" from run_benchmark() kernel-doc comment.
>> 
>
>same comment about changelog.

It'll be fixed next time.

>> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
>> ---
>>  tools/testing/selftests/resctrl/resctrl_val.c | 50 ++++++++++++++++++
>>  tools/testing/selftests/resctrl/resctrlfs.c   | 52 -------------------
>>  2 files changed, 50 insertions(+), 52 deletions(-)
>> 
>> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
>> index f0f6c5f6e98b..5c8dc0a7bab9 100644
>> --- a/tools/testing/selftests/resctrl/resctrl_val.c
>> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
>> @@ -621,6 +621,56 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start)
>>  	return 0;
>>  }
>>  
>> +/*
>> + * run_benchmark - Run a specified benchmark or fill_buf (default benchmark)
>> + *		   in specified signal. Direct benchmark stdio to /dev/null.
>> + * @signum:	signal number
>> + * @info:	signal info
>> + * @ucontext:	user context in signal handling
>> + */
>> +void run_benchmark(int signum, siginfo_t *info, void *ucontext)
>
>Can run_benchmark() now be made static and its declaration removed from
>the header file?

Thanks for noticing. Yes, from my side it seems turning it into static
is okay. I tried it out on Ilpo's branches that I know he's currently
working on and there were no errors either.

-- 
Kind regards
Maciej Wiecz?r-Retman