[RESEND PATCH] kvm/selftests: Close opened file descriptor in stable_tsc_check_supported()

[RESEND PATCH] kvm/selftests: Close opened file descriptor in stable_tsc_check_supported()

[Hao Ge]

Close the "current_clocksource" file descriptor before
returning or exiting from stable_tsc_check_supported()
in vmx_nested_tsc_scaling_test

Signed-off-by: Hao Ge <gehao@kylinos.cn>
---
 .../selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c    | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
index d427eb146bc5..fa03c8d1ce4e 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
@@ -126,12 +126,16 @@ static void stable_tsc_check_supported(void)
 		goto skip_test;
 
 	if (fgets(buf, sizeof(buf), fp) == NULL)
-		goto skip_test;
+		goto close_fp;
 
 	if (strncmp(buf, "tsc", sizeof(buf)))
-		goto skip_test;
+		goto close_fp;
 
+	fclose(fp);
 	return;
+
+close_fp:
+	fclose(fp);
 skip_test:
 	print_skip("Kernel does not use TSC clocksource - assuming that host TSC is not stable");
 	exit(KSFT_SKIP);
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

Re: [RESEND PATCH] kvm/selftests: Close opened file descriptor in stable_tsc_check_supported()

[Sean Christopherson]

This is not a RESEND, it is a new version.  From Documentation/process/submitting-patches.rst:

  Don't add "RESEND" when you are submitting a modified version of your
  patch or patch series - "RESEND" only applies to resubmission of a
  patch or patch series which have not been modified in any way from the
  previous submission.

And the "in any way" really does mean in _any_ way.  E.g. if a patch is rebased,
the version needs to be bumped.  RESEND should only ever be used when sending
literally the same patch/email file, e.g. if something went awry in the delivery
of the email, or you forgot to Cc the right people, tec.

On Wed, Apr 05, 2023, Hao Ge wrote:
> Close the "current_clocksource" file descriptor before

Wrap closer to ~75 chars, wrapping at ~55 is too aggressive.

> returning or exiting from stable_tsc_check_supported()
> in vmx_nested_tsc_scaling_test


Vipin provided his Reviewed-by, that should have been captured here.  Please read
through Documentation/process/submitting-patches.rst, guidance on "using" the
various tags is also provided there.  And if you have time, pretty much all of
Documentation/process/ is worth reading.

No need to send a new version, all of this is easy to fixup when applying.

Re: [RESEND PATCH] kvm/selftests: Close opened file descriptor in stable_tsc_check_supported()

[Sean Christopherson]

On Wed, Apr 05, 2023, Hao Ge wrote:
> Close the "current_clocksource" file descriptor before
> returning or exiting from stable_tsc_check_supported()
> in vmx_nested_tsc_scaling_test
> 
> Signed-off-by: Hao Ge <gehao@kylinos.cn>
> ---
>  .../selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c    | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
> index d427eb146bc5..fa03c8d1ce4e 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
> @@ -126,12 +126,16 @@ static void stable_tsc_check_supported(void)
>  		goto skip_test;
>  
>  	if (fgets(buf, sizeof(buf), fp) == NULL)
> -		goto skip_test;
> +		goto close_fp;
>  
>  	if (strncmp(buf, "tsc", sizeof(buf)))
> -		goto skip_test;
> +		goto close_fp;
>  
> +	fclose(fp);
>  	return;
> +
> +close_fp:
> +	fclose(fp);
>  skip_test:
>  	print_skip("Kernel does not use TSC clocksource - assuming that host TSC is not stable");
>  	exit(KSFT_SKIP);

Actually, this can be streamlined by having the helper return a bool and punting
the skip logic to TEST_REQUIRE.  I'll still apply this patch first, but I'll post
a patch on top to yield:

static bool system_has_stable_tsc(void)
{
	bool tsc_is_stable;
	FILE *fp;
	char buf[4];

	fp = fopen("/sys/devices/system/clocksource/clocksource0/current_clocksource", "r");
	if (fp == NULL)
		return false;

	tsc_is_stable = fgets(buf, sizeof(buf), fp) &&
			!strncmp(buf, "tsc", sizeof(buf));

	fclose(fp);
	return tsc_is_stable;
}

Re: [RESEND PATCH] kvm/selftests: Close opened file descriptor in stable_tsc_check_supported()

[Sean Christopherson]

On Wed, 05 Apr 2023 18:13:50 +0800, Hao Ge wrote:
> Close the "current_clocksource" file descriptor before
> returning or exiting from stable_tsc_check_supported()
> in vmx_nested_tsc_scaling_test
> 
> 

Applied to kvm-x86 selftests, thanks!

[1/1] kvm/selftests: Close opened file descriptor in stable_tsc_check_supported()
      https://github.com/kvm-x86/linux/commit/771214507387

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes

Re: [RESEND PATCH] kvm/selftests: Close opened file descriptor in stable_tsc_check_supported()

[gehao]

在 4/5/23 22:46, Sean Christopherson 写道:

> This is not a RESEND, it is a new version.  From Documentation/process/submitting-patches.rst:
>
>    Don't add "RESEND" when you are submitting a modified version of your
>    patch or patch series - "RESEND" only applies to resubmission of a
>    patch or patch series which have not been modified in any way from the
>    previous submission.
>
> And the "in any way" really does mean in _any_ way.  E.g. if a patch is rebased,
> the version needs to be bumped.  RESEND should only ever be used when sending
> literally the same patch/email file, e.g. if something went awry in the delivery
> of the email, or you forgot to Cc the right people, tec.
>
> On Wed, Apr 05, 2023, Hao Ge wrote:
>> Close the "current_clocksource" file descriptor before
> Wrap closer to ~75 chars, wrapping at ~55 is too aggressive.
>
>> returning or exiting from stable_tsc_check_supported()
>> in vmx_nested_tsc_scaling_test
>
> Vipin provided his Reviewed-by, that should have been captured here.  Please read
> through Documentation/process/submitting-patches.rst, guidance on "using" the
> various tags is also provided there.  And if you have time, pretty much all of
> Documentation/process/ is worth reading.
>
> No need to send a new version, all of this is easy to fixup when applying.

Thank you for your guidance,this has been very helpful to me for sunmitting

patches.

Re: [RESEND PATCH] kvm/selftests: Close opened file descriptor in stable_tsc_check_supported()

[Hao Ge]

Sorry for bother yours, the computer has updated some configurations,

causing an issue with the email name,so correct it.


On 4/6/23 10:56, gehao@kylinos.cn wrote:
> 在 4/5/23 22:46, Sean Christopherson 写道:
>
>> This is not a RESEND, it is a new version.  From 
>> Documentation/process/submitting-patches.rst:
>>
>>    Don't add "RESEND" when you are submitting a modified version of your
>>    patch or patch series - "RESEND" only applies to resubmission of a
>>    patch or patch series which have not been modified in any way from 
>> the
>>    previous submission.
>>
>> And the "in any way" really does mean in _any_ way.  E.g. if a patch 
>> is rebased,
>> the version needs to be bumped.  RESEND should only ever be used when 
>> sending
>> literally the same patch/email file, e.g. if something went awry in 
>> the delivery
>> of the email, or you forgot to Cc the right people, tec.
>>
>> On Wed, Apr 05, 2023, Hao Ge wrote:
>>> Close the "current_clocksource" file descriptor before
>> Wrap closer to ~75 chars, wrapping at ~55 is too aggressive.
>>
>>> returning or exiting from stable_tsc_check_supported()
>>> in vmx_nested_tsc_scaling_test
>>
>> Vipin provided his Reviewed-by, that should have been captured here.  
>> Please read
>> through Documentation/process/submitting-patches.rst, guidance on 
>> "using" the
>> various tags is also provided there.  And if you have time, pretty 
>> much all of
>> Documentation/process/ is worth reading.
>>
>> No need to send a new version, all of this is easy to fixup when 
>> applying.
>
> Thank you for your guidance,this has been very helpful to me for 
> submitting
>
> patches.
>
>
Thanks

Best Regards

Hao