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:   Mon, 7 Aug 2023 13:26:25 +0300 (EEST)
From:   =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>
To:     "Wieczor-Retman, Maciej" <maciej.wieczor-retman@intel.com>
cc:     Reinette Chatre <reinette.chatre@intel.com>,
        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>
Subject: Re: [PATCH v4 07/19] selftests/resctrl: Refactor remount_resctrl(bool
 mum_resctrlfs) to mount_resctrl()
In-Reply-To: <903b5c02-7d7b-11c7-167b-c4f0f3862ae0@intel.com>
Message-ID: <e19fb06b-5dad-e854-3dc-b73c635d7bb1@linux.intel.com>
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>
 <903b5c02-7d7b-11c7-167b-c4f0f3862ae0@intel.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-276558506-1691403989=:9752"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-276558506-1691403989=:9752
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT

On Mon, 24 Jul 2023, Wieczor-Retman, Maciej wrote:
> 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?

Yes, it's a great idea. It would be more logical and thus easier to 
comprehend.

Since this already wast applied, it has to be done in another change.


-- 
 i.

--8323329-276558506-1691403989=:9752--