Messages in this thread | | | Date | Mon, 4 Apr 2022 17:36:02 +0100 | Subject | Re: [PATCH v3 21/21] x86/resctrl: Make resctrl_arch_rmid_read() return values in bytes | From | James Morse <> |
| |
Hi Rob,
On 3/23/22 21:17, Rob Herring wrote: > On Thu, Feb 17, 2022 at 06:21:10PM +0000, James Morse wrote: >> resctrl_arch_rmid_read() returns a value in chunks, as read from the >> hardware. This needs scaling to bytes by mon_scale, as provided by >> the architecture code. >> >> Now that resctrl_arch_rmid_read() performs the overflow and corrections >> itself, it may as well return a value in bytes directly. This allows >> the accesses to the architecture specific 'hw' structure to be removed. >> >> Move the mon_scale conversion into resctrl_arch_rmid_read(). >> mbm_bw_count() is updated to calculate bandwidth from bytes.
>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c >> index 3a6555f49720..437e7db77f93 100644 >> --- a/arch/x86/kernel/cpu/resctrl/monitor.c >> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c >> @@ -211,10 +211,11 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain *d, >> if (am) { >> am->chunks += mbm_overflow_count(am->prev_msr, msr_val, >> hw_res->mbm_width); >> - *val = get_corrected_mbm_count(rmid, am->chunks); >> + chunks = get_corrected_mbm_count(rmid, am->chunks); >> + *val = chunks * hw_res->mon_scale; > > This can be moved out of the if/else if you make the following change: > >> am->prev_msr = msr_val; >> } else { >> - *val = msr_val; >> + *val = msr_val * hw_res->mon_scale; > > chunks = msr_val; > >> }
Sure, I guess it makes sense to only set *val in one place.
>> >> return 0; >> @@ -400,15 +397,14 @@ static u64 __mon_event_count(u32 rmid, struct rmid_read *rr) >> */ >> static void mbm_bw_count(u32 rmid, struct rmid_read *rr) >> { >> - struct rdt_hw_resource *hw_res = resctrl_to_arch_res(rr->r); >> struct mbm_state *m = &rr->d->mbm_local[rmid]; >> - u64 cur_bw, chunks, cur_chunks; >> + u64 cur_bw, bytes, cur_bytes; >> >> - cur_chunks = rr->val; >> - chunks = cur_chunks - m->prev_bw_chunks; >> - m->prev_bw_chunks = cur_chunks; >> + cur_bytes = rr->val; >> + bytes = cur_bytes - m->prev_bw_bytes; >> + m->prev_bw_bytes = cur_bytes; >> >> - cur_bw = (chunks * hw_res->mon_scale) >> 20; >> + cur_bw = bytes >> 20; > > 'bytes / SZ_1M' would be more readable and any decent compiler should > optimize a power of 2 divide. But maybe that's a separate change as > there are other cases of bytes to MB.
I agree its more readable. As I've touched this one, I'll change it.
Thanks,
James
| |