| Message ID | 20230405101350.259000-1-gehao@kylinos.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7712145073876092e9aa81f0b836fef8b5694b14 |
| Headers | show |
| Series | [RESEND] kvm/selftests: Close opened file descriptor in stable_tsc_check_supported() | expand |
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.
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; }
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
在 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.
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
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);
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(-)