[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86: reduce redundancy in tsc_[gs]et_info()
>>> On 30.04.14 at 16:48, <andrew.cooper3@xxxxxxxxxx> wrote: > On 30/04/14 14:53, Jan Beulich wrote: >> --- a/xen/arch/x86/time.c >> +++ b/xen/arch/x86/time.c >> @@ -1786,26 +1786,22 @@ void tsc_get_info(struct domain *d, uint >> >> switch ( *tsc_mode ) >> { >> + uint64_t tsc; >> + >> case TSC_MODE_NEVER_EMULATE: >> - *elapsed_nsec = *gtsc_khz = 0; >> + *elapsed_nsec = *gtsc_khz = 0; >> break; >> - case TSC_MODE_ALWAYS_EMULATE: >> - *elapsed_nsec = get_s_time() - d->arch.vtsc_offset; >> - *gtsc_khz = d->arch.tsc_khz; >> - break; >> case TSC_MODE_DEFAULT: >> if ( d->arch.vtsc ) >> { >> + case TSC_MODE_ALWAYS_EMULATE: >> *elapsed_nsec = get_s_time() - d->arch.vtsc_offset; >> *gtsc_khz = d->arch.tsc_khz; > > double space after = I left them untouched on lines I didn't need to touch anyway, but of course I can as well clean them up uniformly (there were two or three more of them). >> @@ -1815,10 +1811,9 @@ void tsc_get_info(struct domain *d, uint >> } >> else >> { >> - uint64_t tsc = 0; >> rdtscll(tsc); >> - *elapsed_nsec = (scale_delta(tsc,&d->arch.vtsc_to_ns) - >> - d->arch.vtsc_offset); >> + *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) - >> + d->arch.vtsc_offset; >> *gtsc_khz = 0; /* ignored by tsc_set_info */ >> } >> break; > > There is a coverity complaint that following this switch statement, > there is a read of *elapsed_nsec which might be uninitialised if > tsc_mode missed on of the case statements. While I believe this is > currently impossible, it might we wise to have a "default: BUG();" in > case half a new tsc_mode appears. I'm not in favor of such constructs when it's sufficiently obvious that all cases are being handled. >> @@ -1875,28 +1870,24 @@ void tsc_set_info(struct domain *d, >> >> switch ( d->arch.tsc_mode = tsc_mode ) >> { >> - case TSC_MODE_NEVER_EMULATE: >> - d->arch.vtsc = 0; >> - break; >> - case TSC_MODE_ALWAYS_EMULATE: >> - d->arch.vtsc = 1; >> - d->arch.vtsc_offset = get_s_time() - elapsed_nsec; >> - d->arch.tsc_khz = gtsc_khz ? gtsc_khz : cpu_khz; >> - set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 ); >> - d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns); >> - break; >> case TSC_MODE_DEFAULT: >> - d->arch.vtsc = 1; >> + case TSC_MODE_ALWAYS_EMULATE: >> d->arch.vtsc_offset = get_s_time() - elapsed_nsec; >> - d->arch.tsc_khz = gtsc_khz ? gtsc_khz : cpu_khz; >> - set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 ); >> - /* use native TSC if initial host has safe TSC, has not migrated >> - * yet and tsc_khz == cpu_khz */ >> - if ( host_tsc_is_safe() && incarnation == 0 && >> - d->arch.tsc_khz == cpu_khz ) > > This wont apply cleanly on top of staging. Boris made some changed in > this area with his HVM guest TSC series. Right - I forgot I need to re-sync before submitting. Will be a v2 with the above formatting changes also taken care of. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |