Linux-kselftest Archive on lore.kernel.org
 help / color / Atom feed
From: Reinette Chatre <reinette.chatre@intel.com>
To: "Prakhya, Sai Praneeth" <sai.praneeth.prakhya@intel.com>,
	"shuah@kernel.org" <shuah@kernel.org>,
	"skhan@linuxfoundation.org" <skhan@linuxfoundation.org>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>
Cc: "tglx@linutronix.de" <tglx@linutronix.de>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"bp@alien8.de" <bp@alien8.de>, "Luck, Tony" <tony.luck@intel.com>,
	"babu.moger@amd.com" <babu.moger@amd.com>,
	"james.morse@arm.com" <james.morse@arm.com>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	"Yu, Fenghua" <fenghua.yu@intel.com>,
	"x86@kernel.org" <x86@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"dan.carpenter@oracle.com" <dan.carpenter@oracle.com>,
	"dcb314@hotmail.com" <dcb314@hotmail.com>
Subject: Re: [PATCH V2 15/19] selftests/resctrl: Change return type of umount_resctrlfs() to void
Date: Thu, 21 May 2020 11:15:15 -0700
Message-ID: <2e9ea256-cbf4-8222-c639-74479400fd77@intel.com> (raw)
In-Reply-To: <FFF73D592F13FD46B8700F0A279B802F573B66AE@ORSMSX114.amr.corp.intel.com>

Hi Sai,

On 5/21/2020 10:19 AM, Prakhya, Sai Praneeth wrote:
> Hi Reinette,
> 
>> -----Original Message-----
>> From: Reinette Chatre <reinette.chatre@intel.com>
>> Sent: Wednesday, May 20, 2020 4:52 PM
>> To: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com>;
>> shuah@kernel.org; skhan@linuxfoundation.org; linux-kselftest@vger.kernel.org
>> Cc: tglx@linutronix.de; mingo@redhat.com; bp@alien8.de; Luck, Tony
>> <tony.luck@intel.com>; babu.moger@amd.com; james.morse@arm.com;
>> Shankar, Ravi V <ravi.v.shankar@intel.com>; Yu, Fenghua
>> <fenghua.yu@intel.com>; x86@kernel.org; linux-kernel@vger.kernel;
>> dan.carpenter@oracle.com; dcb314@hotmail.com
>> Subject: Re: [PATCH V2 15/19] selftests/resctrl: Change return type of
>> umount_resctrlfs() to void
>>
>> Hi Sai,
>>
>> On 5/18/2020 3:08 PM, Sai Praneeth Prakhya wrote:
>>> umount_resctrlfs() is used only during tear down path and there is
>>> nothing much to do if unmount of resctrl file system fails, so, all
>>> the callers of this function are not checking for the return value.
>>> Hence, change the return type of this function from int to void.
>>
>> Should the callers be ignoring the return value? From what I can tell the
>> filesystem is unmounted between test runs so I wonder if it may help if the
>> return code is used and the test exits with an appropriate error to user space for
>> possible investigation instead of attempting to run a new test on top of the
>> resctrl filesystem that could potentially be having issues at the time.
> 
> Makes sense to me to check for the return value of umount() and take appropriate
> action rather than ignoring it. But, since this might happen very rarely (I haven't
> noticed umount() failing till now), I am thinking to queue this up for cleanup series.
> What do you think?

That sounds good.

> 
> This bug fixes series will then have patches 16 and 17 because they are fixing a bug
> that could be easily noticed. Please let me know if you think otherwise.

I don't, dropping this change that makes it easy to ignore an error in
this round so that any errors could be dealt with better in a later
patch sounds good to me.

Thank you

Reinette


  reply index

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-18 22:08 [PATCH V2 00/19] Miscellaneous fixes for resctrl selftests Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 01/19] selftests/resctrl: Rename CQM test as CMT test Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 02/19] selftests/resctrl: Fix typo Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 03/19] selftests/resctrl: Fix typo in help text Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 04/19] selftests/resctrl: Declare global variables as extern Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 05/19] selftests/resctrl: Return if resctrl file system is not supported Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 06/19] selftests/resctrl: Check for resctrl mount point only if resctrl FS is supported Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 07/19] selftests/resctrl: Use resctrl/info for feature detection Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 08/19] selftests/resctrl: Ensure sibling CPU is not same as original CPU Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 09/19] selftests/resctrl: Fix missing options "-n" and "-p" Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 10/19] selftests/resctrl: Fix MBA/MBM results reporting format Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 11/19] selftests/resctrl: Abort running tests if not root user Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 12/19] selftests/resctrl: Enable gcc checks to detect buffer overflows Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 13/19] selftests/resctrl: Dynamically select buffer size for CAT test Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 14/19] selftests/resctrl: Skip the test if requested resctrl feature is not supported Sai Praneeth Prakhya
2020-05-20 23:46   ` Reinette Chatre
2020-05-21 17:12     ` Prakhya, Sai Praneeth
2020-05-18 22:08 ` [PATCH V2 15/19] selftests/resctrl: Change return type of umount_resctrlfs() to void Sai Praneeth Prakhya
2020-05-20 23:52   ` Reinette Chatre
2020-05-21 17:19     ` Prakhya, Sai Praneeth
2020-05-21 18:15       ` Reinette Chatre [this message]
2020-05-18 22:08 ` [PATCH V2 16/19] selftests/resctrl: Umount resctrl FS only if mounted Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 17/19] selftests/resctrl: Unmount resctrl FS after running all tests Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 18/19] selftests/resctrl: Fix incorrect parsing of iMC counters Sai Praneeth Prakhya
2020-05-18 22:08 ` [PATCH V2 19/19] selftests/resctrl: Fix checking for < 0 for unsigned values Sai Praneeth Prakhya
2020-05-21 16:12 ` [PATCH V2 00/19] Miscellaneous fixes for resctrl selftests Reinette Chatre
2020-05-21 17:28   ` Prakhya, Sai Praneeth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2e9ea256-cbf4-8222-c639-74479400fd77@intel.com \
    --to=reinette.chatre@intel.com \
    --cc=babu.moger@amd.com \
    --cc=bp@alien8.de \
    --cc=dan.carpenter@oracle.com \
    --cc=dcb314@hotmail.com \
    --cc=fenghua.yu@intel.com \
    --cc=james.morse@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=ravi.v.shankar@intel.com \
    --cc=sai.praneeth.prakhya@intel.com \
    --cc=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-kselftest Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-kselftest/0 linux-kselftest/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-kselftest linux-kselftest/ https://lore.kernel.org/linux-kselftest \
		linux-kselftest@vger.kernel.org
	public-inbox-index linux-kselftest

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kselftest


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git