| Message ID | 20200329061608.GA29651@simran-Inspiron-5558 (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/x86: Compress lines for immediate return | expand |
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.
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.
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. >
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()
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(-)