2022-08-31 15:08:42

by CGEL

[permalink] [raw]
Subject: [PATCH linux-next] KVM: selftests: remove redundant variable tsc_val

From: Jinpeng Cui <[email protected]>

Return value directly from expression instead of
getting value from redundant variable tsc_val.

Reported-by: Zeal Robot <[email protected]>
Signed-off-by: Jinpeng Cui <[email protected]>
---
tools/testing/selftests/kvm/include/x86_64/processor.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 0cbc71b7af50..75920678f34d 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -237,7 +237,6 @@ static inline uint64_t get_desc64_base(const struct desc64 *desc)
static inline uint64_t rdtsc(void)
{
uint32_t eax, edx;
- uint64_t tsc_val;
/*
* The lfence is to wait (on Intel CPUs) until all previous
* instructions have been executed. If software requires RDTSC to be
@@ -245,8 +244,8 @@ static inline uint64_t rdtsc(void)
* execute LFENCE immediately after RDTSC
*/
__asm__ __volatile__("lfence; rdtsc; lfence" : "=a"(eax), "=d"(edx));
- tsc_val = ((uint64_t)edx) << 32 | eax;
- return tsc_val;
+
+ return ((uint64_t)edx) << 32 | eax;
}

static inline uint64_t rdtscp(uint32_t *aux)
--
2.25.1


2022-08-31 16:54:18

by Jim Mattson

[permalink] [raw]
Subject: Re: [PATCH linux-next] KVM: selftests: remove redundant variable tsc_val

On Wed, Aug 31, 2022 at 7:31 AM <[email protected]> wrote:
>
> From: Jinpeng Cui <[email protected]>
>
> Return value directly from expression instead of
> getting value from redundant variable tsc_val.

Nit: I think you mean 'superfluous' rather than 'redundant'?

> Reported-by: Zeal Robot <[email protected]>
> Signed-off-by: Jinpeng Cui <[email protected]>
> ---
> tools/testing/selftests/kvm/include/x86_64/processor.h | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index 0cbc71b7af50..75920678f34d 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -237,7 +237,6 @@ static inline uint64_t get_desc64_base(const struct desc64 *desc)
> static inline uint64_t rdtsc(void)
> {
> uint32_t eax, edx;
> - uint64_t tsc_val;
> /*
> * The lfence is to wait (on Intel CPUs) until all previous
> * instructions have been executed. If software requires RDTSC to be
> @@ -245,8 +244,8 @@ static inline uint64_t rdtsc(void)
> * execute LFENCE immediately after RDTSC
> */
> __asm__ __volatile__("lfence; rdtsc; lfence" : "=a"(eax), "=d"(edx));
> - tsc_val = ((uint64_t)edx) << 32 | eax;
> - return tsc_val;
> +
> + return ((uint64_t)edx) << 32 | eax;
> }

This does beg the question: "Why?"

2022-09-20 20:13:39

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH linux-next] KVM: selftests: remove redundant variable tsc_val

On Wed, Aug 31, 2022, Jim Mattson wrote:
> On Wed, Aug 31, 2022 at 7:31 AM <[email protected]> wrote:
> > diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> > index 0cbc71b7af50..75920678f34d 100644
> > --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> > +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> > @@ -237,7 +237,6 @@ static inline uint64_t get_desc64_base(const struct desc64 *desc)
> > static inline uint64_t rdtsc(void)
> > {
> > uint32_t eax, edx;
> > - uint64_t tsc_val;
> > /*
> > * The lfence is to wait (on Intel CPUs) until all previous
> > * instructions have been executed. If software requires RDTSC to be
> > @@ -245,8 +244,8 @@ static inline uint64_t rdtsc(void)
> > * execute LFENCE immediately after RDTSC
> > */
> > __asm__ __volatile__("lfence; rdtsc; lfence" : "=a"(eax), "=d"(edx));
> > - tsc_val = ((uint64_t)edx) << 32 | eax;
> > - return tsc_val;
> > +
> > + return ((uint64_t)edx) << 32 | eax;
> > }
>
> This does beg the question: "Why?"

Yeah, for this one I think having a local variable adds value.

2022-09-22 19:02:51

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH linux-next] KVM: selftests: remove redundant variable tsc_val

On 8/31/22 08:31, [email protected] wrote:
> From: Jinpeng Cui <[email protected]>
>
> Return value directly from expression instead of
> getting value from redundant variable tsc_val.
>
> Reported-by: Zeal Robot <[email protected]>
> Signed-off-by: Jinpeng Cui <[email protected]>
> ---
> tools/testing/selftests/kvm/include/x86_64/processor.h | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index 0cbc71b7af50..75920678f34d 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -237,7 +237,6 @@ static inline uint64_t get_desc64_base(const struct desc64 *desc)
> static inline uint64_t rdtsc(void)
> {
> uint32_t eax, edx;
> - uint64_t tsc_val;
> /*
> * The lfence is to wait (on Intel CPUs) until all previous
> * instructions have been executed. If software requires RDTSC to be
> @@ -245,8 +244,8 @@ static inline uint64_t rdtsc(void)
> * execute LFENCE immediately after RDTSC
> */
> __asm__ __volatile__("lfence; rdtsc; lfence" : "=a"(eax), "=d"(edx));
> - tsc_val = ((uint64_t)edx) << 32 | eax;
> - return tsc_val;
> +
> + return ((uint64_t)edx) << 32 | eax;
> }
>
> static inline uint64_t rdtscp(uint32_t *aux)

My understanding is that this patch isn't coming from individuals that work
for ZTE. We won't be able to accept these patches. Refer to the following
for reasons why we can't accept these patches.

https://patchwork.kernel.org/project/linux-kselftest/patch/[email protected]/

thanks,
-- Shuah

2022-09-22 20:41:56

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH linux-next] KVM: selftests: remove redundant variable tsc_val

On Thu, Sep 22, 2022, Shuah Khan wrote:
> On 8/31/22 08:31, [email protected] wrote:
> > From: Jinpeng Cui <[email protected]>
> >
> > Return value directly from expression instead of
> > getting value from redundant variable tsc_val.
> >
> > Reported-by: Zeal Robot <[email protected]>
> > Signed-off-by: Jinpeng Cui <[email protected]>
> > ---

...

> My understanding is that this patch isn't coming from individuals that work
> for ZTE. We won't be able to accept these patches. Refer to the following
> for reasons why we can't accept these patches.

Ouch. Thanks much for the heads up!

2022-09-23 06:52:40

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH linux-next] KVM: selftests: remove redundant variable tsc_val

On Thu, Sep 22, 2022 at 12:45:22PM -0600, Shuah Khan wrote:
> On 8/31/22 08:31, [email protected] wrote:
> > From: Jinpeng Cui <[email protected]>
> >
> > Return value directly from expression instead of
> > getting value from redundant variable tsc_val.
> >
> > Reported-by: Zeal Robot <[email protected]>
> > Signed-off-by: Jinpeng Cui <[email protected]>
> > ---
> > tools/testing/selftests/kvm/include/x86_64/processor.h | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> > index 0cbc71b7af50..75920678f34d 100644
> > --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> > +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> > @@ -237,7 +237,6 @@ static inline uint64_t get_desc64_base(const struct desc64 *desc)
> > static inline uint64_t rdtsc(void)
> > {
> > uint32_t eax, edx;
> > - uint64_t tsc_val;
> > /*
> > * The lfence is to wait (on Intel CPUs) until all previous
> > * instructions have been executed. If software requires RDTSC to be
> > @@ -245,8 +244,8 @@ static inline uint64_t rdtsc(void)
> > * execute LFENCE immediately after RDTSC
> > */
> > __asm__ __volatile__("lfence; rdtsc; lfence" : "=a"(eax), "=d"(edx));
> > - tsc_val = ((uint64_t)edx) << 32 | eax;
> > - return tsc_val;
> > +
> > + return ((uint64_t)edx) << 32 | eax;
> > }
> > static inline uint64_t rdtscp(uint32_t *aux)
>
> My understanding is that this patch isn't coming from individuals that work
> for ZTE. We won't be able to accept these patches. Refer to the following
> for reasons why we can't accept these patches.
>
> https://patchwork.kernel.org/project/linux-kselftest/patch/[email protected]/

Thanks for catching this.

Also this address has now been banned from the kernel mailing lists, so
watch out for patches sent to maintainers that do not show up on
lore.kernel.org.

thanks,

greg k-h