[Xen-devel] [PATCH] xen/x86: Compress lines for immediate return

Simran Singhal posted 1 patch 4 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20200329061608.GA29651@simran-Inspiron-5558
Maintainers: Wei Liu <wl@xen.org>, Jan Beulich <jbeulich@suse.com>, "Roger Pau Monné" <roger.pau@citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>
There is a newer version of this series
xen/arch/x86/acpi/cpufreq/cpufreq.c     | 9 ++-------
xen/arch/x86/hvm/mtrr.c                 | 6 ++----
xen/arch/x86/hvm/vpic.c                 | 5 ++---
xen/arch/x86/oprofile/op_model_athlon.c | 3 +--
xen/arch/x86/time.c                     | 5 +----
5 files changed, 8 insertions(+), 20 deletions(-)
[Xen-devel] [PATCH] xen/x86: Compress lines for immediate return
Posted by Simran Singhal 4 years ago
Compress two lines into a single line if immediate return statement is found.
It also remove variables retval, freq, effective, vector and now
as they are no longer needed.

Signed-off-by: Simran Singhal <singhalsimran0@gmail.com>
---
 xen/arch/x86/acpi/cpufreq/cpufreq.c     | 9 ++-------
 xen/arch/x86/hvm/mtrr.c                 | 6 ++----
 xen/arch/x86/hvm/vpic.c                 | 5 ++---
 xen/arch/x86/oprofile/op_model_athlon.c | 3 +--
 xen/arch/x86/time.c                     | 5 +----
 5 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/xen/arch/x86/acpi/cpufreq/cpufreq.c b/xen/arch/x86/acpi/cpufreq/cpufreq.c
index 281be131a3..f1f3c6923f 100644
--- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
+++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
@@ -270,7 +270,6 @@ unsigned int get_measured_perf(unsigned int cpu, unsigned int flag)
     struct cpufreq_policy *policy;    
     struct perf_pair readin, cur, *saved;
     unsigned int perf_percent;
-    unsigned int retval;
 
     if (!cpu_online(cpu))
         return 0;
@@ -318,16 +317,13 @@ unsigned int get_measured_perf(unsigned int cpu, unsigned int flag)
     else
         perf_percent = 0;
 
-    retval = policy->cpuinfo.max_freq * perf_percent / 100;
-
-    return retval;
+    return policy->cpuinfo.max_freq * perf_percent / 100;
 }
 
 static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
 {
     struct cpufreq_policy *policy;
     struct acpi_cpufreq_data *data;
-    unsigned int freq;
 
     if (!cpu_online(cpu))
         return 0;
@@ -341,8 +337,7 @@ static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
         data->acpi_data == NULL || data->freq_table == NULL))
         return 0;
 
-    freq = extract_freq(get_cur_val(cpumask_of(cpu)), data);
-    return freq;
+    return extract_freq(get_cur_val(cpumask_of(cpu)), data);
 }
 
 static void feature_detect(void *info)
diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
index 8356e8de3d..511c3be1c8 100644
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -317,7 +317,7 @@ static uint8_t effective_mm_type(struct mtrr_state *m,
                                  uint32_t pte_flags,
                                  uint8_t gmtrr_mtype)
 {
-    uint8_t mtrr_mtype, pat_value, effective;
+    uint8_t mtrr_mtype, pat_value;
    
     /* if get_pat_flags() gives a dedicated MTRR type,
      * just use it
@@ -329,9 +329,7 @@ static uint8_t effective_mm_type(struct mtrr_state *m,
 
     pat_value = page_pat_type(pat, pte_flags);
 
-    effective = mm_type_tbl[mtrr_mtype][pat_value];
-
-    return effective;
+    return mm_type_tbl[mtrr_mtype][pat_value];
 }
 
 uint32_t get_pat_flags(struct vcpu *v,
diff --git a/xen/arch/x86/hvm/vpic.c b/xen/arch/x86/hvm/vpic.c
index 4897a0e05b..61f4b6784c 100644
--- a/xen/arch/x86/hvm/vpic.c
+++ b/xen/arch/x86/hvm/vpic.c
@@ -484,7 +484,7 @@ void vpic_irq_negative_edge(struct domain *d, int irq)
 
 int vpic_ack_pending_irq(struct vcpu *v)
 {
-    int irq, vector;
+    int irq;
     struct hvm_hw_vpic *vpic = &v->domain->arch.hvm.vpic[0];
 
     ASSERT(has_vpic(v->domain));
@@ -498,8 +498,7 @@ int vpic_ack_pending_irq(struct vcpu *v)
     if ( irq == -1 )
         return -1;
 
-    vector = vpic[irq >> 3].irq_base + (irq & 7);
-    return vector;
+    return vpic[irq >> 3].irq_base + (irq & 7);
 }
 
 /*
diff --git a/xen/arch/x86/oprofile/op_model_athlon.c b/xen/arch/x86/oprofile/op_model_athlon.c
index 5c48f868ae..2bc0d04a1f 100644
--- a/xen/arch/x86/oprofile/op_model_athlon.c
+++ b/xen/arch/x86/oprofile/op_model_athlon.c
@@ -343,9 +343,8 @@ static int athlon_check_ctrs(unsigned int const cpu,
 		}
 	}
 
-	ovf = handle_ibs(mode, regs);
 	/* See op_model_ppro.c */
-	return ovf;
+	return handle_ibs(mode, regs);
 }
 
 static inline void start_ibs(void)
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 2d4430b283..bbaea3aa65 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1142,16 +1142,13 @@ s_time_t get_s_time_fixed(u64 at_tsc)
 {
     const struct cpu_time *t = &this_cpu(cpu_time);
     u64 tsc, delta;
-    s_time_t now;
 
     if ( at_tsc )
         tsc = at_tsc;
     else
         tsc = rdtsc_ordered();
     delta = tsc - t->stamp.local_tsc;
-    now = t->stamp.local_stime + scale_delta(delta, &t->tsc_scale);
-
-    return now;
+    return t->stamp.local_stime + scale_delta(delta, &t->tsc_scale);
 }
 
 s_time_t get_s_time()
-- 
2.17.1


Re: [Xen-devel] [PATCH] xen/x86: Compress lines for immediate return
Posted by Wei Liu 4 years ago
On Sun, Mar 29, 2020 at 11:46:08AM +0530, Simran Singhal wrote:
> diff --git a/xen/arch/x86/oprofile/op_model_athlon.c b/xen/arch/x86/oprofile/op_model_athlon.c
> index 5c48f868ae..2bc0d04a1f 100644
> --- a/xen/arch/x86/oprofile/op_model_athlon.c
> +++ b/xen/arch/x86/oprofile/op_model_athlon.c
> @@ -343,9 +343,8 @@ static int athlon_check_ctrs(unsigned int const cpu,
>  		}
>  	}
>  
> -	ovf = handle_ibs(mode, regs);
>  	/* See op_model_ppro.c */
> -	return ovf;
> +	return handle_ibs(mode, regs);

Hmm... ovf can potentially be set in the for loop before this hunk, but
then immediately get overwritten by the function call. I bet the ovf = 1
line is the reason why you didn't remove ovf out right.

I think you can perhaps just remove ovf here. It would make any
difference logically.

Other changes look correct to me.

Wei.

Re: [Xen-devel] [PATCH] xen/x86: Compress lines for immediate return
Posted by Simran Singhal 4 years ago
On Sun, Mar 29, 2020 at 7:17 PM Wei Liu <wl@xen.org> wrote:

> On Sun, Mar 29, 2020 at 11:46:08AM +0530, Simran Singhal wrote:
> > diff --git a/xen/arch/x86/oprofile/op_model_athlon.c
> b/xen/arch/x86/oprofile/op_model_athlon.c
> > index 5c48f868ae..2bc0d04a1f 100644
> > --- a/xen/arch/x86/oprofile/op_model_athlon.c
> > +++ b/xen/arch/x86/oprofile/op_model_athlon.c
> > @@ -343,9 +343,8 @@ static int athlon_check_ctrs(unsigned int const cpu,
> >               }
> >       }
> >
> > -     ovf = handle_ibs(mode, regs);
> >       /* See op_model_ppro.c */
> > -     return ovf;
> > +     return handle_ibs(mode, regs);
>
> Hmm... ovf can potentially be set in the for loop before this hunk, but
> then immediately get overwritten by the function call. I bet the ovf = 1
> line is the reason why you didn't remove ovf out right.
>
> I think you can perhaps just remove ovf here. It would make any
> difference logically.
>
> Other changes look correct to me.
>

Thanks for your feedback.
Ah! I agree on "ovf = 1" is the reason I decided to leave ovf as it is.
I'll remove ovf now and send the new version of this patch.

Thanks
Simran


>
> Wei.
>
Re: [Xen-devel] [PATCH] xen/x86: Compress lines for immediate return
Posted by Wei Liu 4 years ago
On Sun, Mar 29, 2020 at 02:46:57PM +0100, Wei Liu wrote:
> On Sun, Mar 29, 2020 at 11:46:08AM +0530, Simran Singhal wrote:
> > diff --git a/xen/arch/x86/oprofile/op_model_athlon.c b/xen/arch/x86/oprofile/op_model_athlon.c
> > index 5c48f868ae..2bc0d04a1f 100644
> > --- a/xen/arch/x86/oprofile/op_model_athlon.c
> > +++ b/xen/arch/x86/oprofile/op_model_athlon.c
> > @@ -343,9 +343,8 @@ static int athlon_check_ctrs(unsigned int const cpu,
> >  		}
> >  	}
> >  
> > -	ovf = handle_ibs(mode, regs);
> >  	/* See op_model_ppro.c */
> > -	return ovf;
> > +	return handle_ibs(mode, regs);
> 
> Hmm... ovf can potentially be set in the for loop before this hunk, but
> then immediately get overwritten by the function call. I bet the ovf = 1
> line is the reason why you didn't remove ovf out right.
> 
> I think you can perhaps just remove ovf here. It would make any
> difference logically.

I meant "It would _not_ make any difference logically" here.

Wei.