The value of cycles and flags can be assigned directly without
intermediate variables.
Remove the useless variables.
Signed-off-by: Minfei Huang <[email protected]>
---
arch/x86/include/asm/pvclock.h | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
index fdcc040..fb95dac 100644
--- a/arch/x86/include/asm/pvclock.h
+++ b/arch/x86/include/asm/pvclock.h
@@ -80,19 +80,12 @@ static __always_inline
unsigned __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
cycle_t *cycles, u8 *flags)
{
- unsigned version;
- cycle_t ret, offset;
- u8 ret_flags;
-
- version = src->version;
+ cycle_t offset;
offset = pvclock_get_nsec_offset(src);
- ret = src->system_time + offset;
- ret_flags = src->flags;
-
- *cycles = ret;
- *flags = ret_flags;
- return version;
+ *cycles = src->system_time + offset;
+ *flags = src->flags;
+ return src->version;
}
struct pvclock_vsyscall_time_info {
--
2.6.3
ping.
Any comment is appreciate.
Thanks
Minfei
On 04/25/16 at 02:53P, Minfei Huang wrote:
> The value of cycles and flags can be assigned directly without
> intermediate variables.
>
> Remove the useless variables.
>
> Signed-off-by: Minfei Huang <[email protected]>
> ---
> arch/x86/include/asm/pvclock.h | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
> index fdcc040..fb95dac 100644
> --- a/arch/x86/include/asm/pvclock.h
> +++ b/arch/x86/include/asm/pvclock.h
> @@ -80,19 +80,12 @@ static __always_inline
> unsigned __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
> cycle_t *cycles, u8 *flags)
> {
> - unsigned version;
> - cycle_t ret, offset;
> - u8 ret_flags;
> -
> - version = src->version;
> + cycle_t offset;
>
> offset = pvclock_get_nsec_offset(src);
> - ret = src->system_time + offset;
> - ret_flags = src->flags;
> -
> - *cycles = ret;
> - *flags = ret_flags;
> - return version;
> + *cycles = src->system_time + offset;
> + *flags = src->flags;
> + return src->version;
> }
>
> struct pvclock_vsyscall_time_info {
> --
> 2.6.3
>
On Mon, Apr 25, 2016 at 02:53:14PM +0800, Minfei Huang wrote:
> The value of cycles and flags can be assigned directly without
> intermediate variables.
>
> Remove the useless variables.
>
> Signed-off-by: Minfei Huang <[email protected]>
> ---
> arch/x86/include/asm/pvclock.h | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
> index fdcc040..fb95dac 100644
> --- a/arch/x86/include/asm/pvclock.h
> +++ b/arch/x86/include/asm/pvclock.h
> @@ -80,19 +80,12 @@ static __always_inline
> unsigned __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
> cycle_t *cycles, u8 *flags)
> {
> - unsigned version;
> - cycle_t ret, offset;
> - u8 ret_flags;
> -
> - version = src->version;
> + cycle_t offset;
>
> offset = pvclock_get_nsec_offset(src);
You could go a step further and get rid of that
pvclock_get_nsec_offset() simple wrapper too and move its meat into
__pvclock_read_cycles()...
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
On Sat, Apr 30, 2016 at 6:24 AM, Borislav Petkov <[email protected]> wrote:
> On Mon, Apr 25, 2016 at 02:53:14PM +0800, Minfei Huang wrote:
>> The value of cycles and flags can be assigned directly without
>> intermediate variables.
>>
>> Remove the useless variables.
>>
>> Signed-off-by: Minfei Huang <[email protected]>
>> ---
>> arch/x86/include/asm/pvclock.h | 15 ++++-----------
>> 1 file changed, 4 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
>> index fdcc040..fb95dac 100644
>> --- a/arch/x86/include/asm/pvclock.h
>> +++ b/arch/x86/include/asm/pvclock.h
>> @@ -80,19 +80,12 @@ static __always_inline
>> unsigned __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
>> cycle_t *cycles, u8 *flags)
>> {
>> - unsigned version;
>> - cycle_t ret, offset;
>> - u8 ret_flags;
>> -
>> - version = src->version;
>> + cycle_t offset;
>>
>> offset = pvclock_get_nsec_offset(src);
>
> You could go a step further and get rid of that
> pvclock_get_nsec_offset() simple wrapper too and move its meat into
> __pvclock_read_cycles()...
>
Take a look at vread_pvclock. I decided that __pvclock_read_cycles
was too ugly to use and was very slow and I just gave up and rewrote
it.
> --
> Regards/Gruss,
> Boris.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
> --
--
Andy Lutomirski
AMA Capital Management, LLC
On Sat, Apr 30, 2016 at 10:47:49AM -0700, Andy Lutomirski wrote:
> Take a look at vread_pvclock. I decided that __pvclock_read_cycles
> was too ugly to use and was very slow and I just gave up and rewrote
> it.
Should we kill __pvclock_read_cycles in favor of vread_pvclock? It looks
doable at a quick scan...
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
On Apr 30, 2016 12:17 PM, "Borislav Petkov" <[email protected]> wrote:
>
> On Sat, Apr 30, 2016 at 10:47:49AM -0700, Andy Lutomirski wrote:
> > Take a look at vread_pvclock. I decided that __pvclock_read_cycles
> > was too ugly to use and was very slow and I just gave up and rewrote
> > it.
>
> Should we kill __pvclock_read_cycles in favor of vread_pvclock? It looks
> doable at a quick scan...
>
The in-kernel version might have to be a bit different because it
needs to handle the !stable case. If !stable, it should just use the
current CPU's copy which means that, realistically, it should just
get_cpu and use the local copy unconditionally. Other than that, it
could look a lot like the vread_pvclock variant.
But I agree, the current thing is incomprehensible.
--Andy
> --
> Regards/Gruss,
> Boris.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
> --