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: <903b5c02-7d7b-11c7-167b-c4f0f3862ae0@intel.com>
Date:   Mon, 24 Jul 2023 09:12:27 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.13.0
Subject: Re: [PATCH v4 07/19] selftests/resctrl: Refactor remount_resctrl(bool
 mum_resctrlfs) to mount_resctrl()
Content-Language: pl
To:     =?UTF-8?Q?Ilpo_J=c3=a4rvinen?= <ilpo.jarvinen@linux.intel.com>,
        "Reinette Chatre" <reinette.chatre@intel.com>
CC:     <linux-kselftest@vger.kernel.org>, Shuah Khan <shuah@kernel.org>,
        "Shaopeng Tan" <tan.shaopeng@jp.fujitsu.com>,
        Fenghua Yu <fenghua.yu@intel.com>,
        LKML <linux-kernel@vger.kernel.org>
References: <20230713131932.133258-1-ilpo.jarvinen@linux.intel.com>
 <20230713131932.133258-8-ilpo.jarvinen@linux.intel.com>
 <09605219-19db-ba2d-aaad-9e279543f461@intel.com>
 <2c1f4d5f-7d6f-1178-7ec4-7f8b862b26e9@linux.intel.com>
From:   "Wieczor-Retman, Maciej" <maciej.wieczor-retman@intel.com>
In-Reply-To: <2c1f4d5f-7d6f-1178-7ec4-7f8b862b26e9@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?dmxEa25obyt4VklwSVZVaFhmalhsOU40dzNJVzNZVEtoT3J0VGU3ZFFmZU5R?=
 =?utf-8?B?cnZ2cWdJc3d3K2xFSXFIdE5oTkJ4WWFVdDBLZDMrRldsaGZEd09FNDdjYWZn?=
 =?utf-8?B?VGNGTHJ0N25zTE0rZ3B0L3pHNWZWQVZ3MnVvNlNJWlFtRjdlbWc3ZXBjTHJY?=
 =?utf-8?B?cDhwSTZ1elVkVVNIbGdZUUViSUtOeitOME9pSjRsMWZ5amRvb0d1MnFQTW1V?=
 =?utf-8?B?MUE3TmtsQlBId2dTWkFhZEUydSt6TEFUai9XK2oyNHFtdXg5REdxZS9jN29i?=
 =?utf-8?B?dGVUU1FmQjlwcmV5RmVmWXhXUDllV3l5MjdZMmFYOWQrdWRHLzV3ek0zYnZZ?=
 =?utf-8?B?MEk1V1dKMzJFZ1hza2tkaTUzbnhrdzJNREUyd1I0MFdTSzJkRE9hSkNQVWN6?=
 =?utf-8?B?emp1QURiVHR6MFAyY3RaTEhlWlhQTmc2Q2c5YWJERmMzb3hOdFRWUnIwRndQ?=
 =?utf-8?B?RElsQ3pCMHN1UWN3YTRDQ2s4TVdrS2kxdVV5bSsxcjNIMkdIVkdwL3dPaWxw?=
 =?utf-8?B?Z1dxRTR2UGwxYm9yM2ZwSUtKbWZEeFgyV2lMekxHc1pwUVkvQVVUQjV2eGFO?=
 =?utf-8?B?Wkx0ZGdrdXNaRnpscy9IWnNIZjBETHhIWG5vUzlCcHRqUVE5Zm5YTEZsNVQy?=
 =?utf-8?B?Y2ZVM0FKbW04dnUrZTJMTEo1aHdZR1RuOXVHM3JuUjdQNzZPT2JOWUx0WTlw?=
 =?utf-8?B?eEdSQTAzaHdCZnRITjFxeXVobGpHcDVIRjhsQk4weDY5a2RseEZTeU1nT2ZG?=
 =?utf-8?B?eXlzRzVMdXNOYnBuMldjSWk4em1pVUtNTzBKTHoxbWRXdGZMTmdtVXhEZStm?=
 =?utf-8?B?b080MHl2d1VoR1dMa00rYjZVSlFkQ2RJcWkvaWhtaFY2VHdnd1haeWV1enVE?=
 =?utf-8?B?S0ErdWN2MVZVOW1JT0VNUTBUZ0dKWkg5T0E1ZzJWRlRUU0FWM1oyTWNFL2V2?=
 =?utf-8?B?VC9UNjlGWG5QOFp5STZDaCtneThyYTdnR3hCMjVWcWVVcENucm9uVSs0ZlhU?=
 =?utf-8?B?NDB6Z2VKUHdXWEFCR3o3eVhNbjFGNHc1OXp4UEFNbEIzN1JjWGRHNW5mVU90?=
 =?utf-8?B?TVNlTjFmVGZubGJHQ01FZWNlQTZZZHZiSHluUjdKNmhCVjV0RnlvTzYwMTM0?=
 =?utf-8?B?S1ozK2F6UzZXaW4vZmRGNzN2SEZnTjZ6WHE1UWFFbGFVNzJaUTZUMWRSUXh1?=
 =?utf-8?B?TWZJMEErWUx5ZlBwRGJVenN0N0NVRGJIOWtuV2wxSGVjWlBpUjZ6TWZPRTFJ?=
 =?utf-8?B?QzhlR1NQZHMyTm4xb0NOL1ViS3hob2FydUlhRDR0UE4rUng3REhRWkxNN3RZ?=
 =?utf-8?B?ZGZLL1FmQS84ZGhTZFhuQU9vNWZ4MFpBc2xBd0IvU3hNSmhiZkxJQ0ViUzZX?=
 =?utf-8?B?aUZwTXFyQjRMYjFJS3lpN25Sc3E3T1N1aTZuWjhMYnR0NTV6UWgyN1Z3a1lU?=
 =?utf-8?B?WlRIOTRSVC9Ja3VmZ0FBbUI0ZkRlaWM3UENFWERCaytVd1JSamxTWFJGeDk4?=
 =?utf-8?B?R1J2K25nY250SmtLbEFOTzBSYU50Vk5nc2YwMU1FdEE2TVNJY1NOQlVZS2w4?=
 =?utf-8?B?bjNhdkN6OXZyWU9CUFZoWE94bytoU1MzZmxRWUFicm9OMkhDZENFUXFOeWZt?=
 =?utf-8?B?a1AxdU12VU1MNUtIeFUrYkhnSTA2eDYvWm5KVHRzd2FJVzUxdWE4KzFEWUhu?=
 =?utf-8?B?NndZUlVaZXZWME5GVlhGTnJLVWVIVzRUcDJmaWJvSlYwZFU2K2w0Uzc2QWlh?=
 =?utf-8?B?SEFkckdKMUpMcG5KbGk3d3ZGeEpjZlZKNUo3TmdKcTNZMDdxeldzRDh4QlJU?=
 =?utf-8?B?VkN2azhJZHdMODZkV1U2NkcyZWVDSFp1eU1KS1FMdG1Ramc2M0laR0pRK1Vx?=
 =?utf-8?B?aFFmWjhFaXQzSlgrY1Z2Zm1qL0lZV25NMTVVeXFXb2l1djB4eWo5djNGTUpG?=
 =?utf-8?B?bUNBQjhNczVscHU2KzJTMyt0c0F2elFXVmJvRGNaTWtxcXR4amJyRHAwL1dF?=
 =?utf-8?B?czlOSmI3ZFoxRmpRSWVLcXBmNjZ3eUJ4LzBGeWdwUjRlMlBuV25MM0kvR2xR?=
 =?utf-8?B?aEdHMWtDQ1RYMFZrcDEya3p0bDNDZURVZTNZaENzZVhna2FGc1V2VU5nTEg0?=
 =?utf-8?B?MDQ3YmdkOWkxMTJ3ZUtNTlhhRkxvTExGc09zeDZQR0JVYkhjRDlkaVR2SEV0?=
 =?utf-8?Q?+U+mC3qFBkAZthNUwP4oFx4=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: cb7340c1-1924-4f63-28d0-08db8c15596c
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Jul 2023 07:12:32.8251
 (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: PdFuJKbnszvqM1CeafWXUyCUpWhEWiZo7GVGDAMqczI1ZyHqb+HsGzgBiSLnpOaN6UsgQgWUfSNAGv+dQYg5alunHeZ7bTF9xvN/RkI7cD8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7732
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 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
        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 14.07.2023 13:03, Ilpo Järvinen wrote:
> On Thu, 13 Jul 2023, Reinette Chatre wrote:
>> On 7/13/2023 6:19 AM, Ilpo Järvinen wrote:
>>> -int remount_resctrlfs(bool mum_resctrlfs)
>>> +int mount_resctrlfs(void)
>>>  {
>>> -	char mountpoint[256];
>>>  	int ret;
>>>  
>>> -	ret = find_resctrl_mount(mountpoint);
>>> -	if (ret)
>>> -		strcpy(mountpoint, RESCTRL_PATH);
>>> -
>>> -	if (!ret && mum_resctrlfs && umount(mountpoint))
>>> -		ksft_print_msg("Fail: unmounting \"%s\"\n", mountpoint);
>>> -
>>> -	if (!ret && !mum_resctrlfs)
>>> -		return 0;
>>> +	ret = find_resctrl_mount(NULL);
>>> +	if (!ret)
>>> +		return -1;
>>
>> This treats "ret == 0" as a failure. What about -ENXIO? It seems to
>> me that only "ret == -ENOENT" is "success".
> 
> Yes, it's a good catch.
> 

I had an idea about a small redesign of find_resctrl_mount
return values so it is easier to see what the function tries
to accomplish.

When there is an error (-ENXIO for example) it could 
return the negative error value. When no mount is found
it could return a zero (instead of the -ENOENT error code).
Finally when a mount point was found it could return a positive
value (for example return 1). This way errors could be 
separate from regular return values and in my opinion the
function logic would be more transparent.

What do you think about it?

Kind regards
Maciej Wieczór-Retman