2020-05-21 18:17:29

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH V2 15/19] selftests/resctrl: Change return type of umount_resctrlfs() to void

Hi Sai,

On 5/21/2020 10:19 AM, Prakhya, Sai Praneeth wrote:
> Hi Reinette,
>
>> -----Original Message-----
>> From: Reinette Chatre <[email protected]>
>> Sent: Wednesday, May 20, 2020 4:52 PM
>> To: Prakhya, Sai Praneeth <[email protected]>;
>> [email protected]; [email protected]; [email protected]
>> Cc: [email protected]; [email protected]; [email protected]; Luck, Tony
>> <[email protected]>; [email protected]; [email protected];
>> Shankar, Ravi V <[email protected]>; Yu, Fenghua
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]; [email protected]
>> 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