On Tue, 2021-04-20 at 22:37 +0800, Chen Yu wrote: > On Tue, Apr 20, 2021 at 09:28:06AM -0400, Calvin Walton wrote: > > This patch has the same issue I noticed with the initial revision > > of > > Terry's patch - the idx_to_offset function returns type int (32-bit > > signed), but MSR_PKG_ENERGY_STAT is greater than INT_MAX (or > > rather, > > would be interpreted as a negative number) > > > > The end result is, as far as I can tell, that it hits the if > > (offset < > > 0) check in update_msr_sum() resulting in the timer callback for > > updating the stat in the background when long durations are used to > > not > > happen. > > > > For short durations it still works fine since the background update > > isn't used. > > > Ah, got it, nice catch. How about an incremental patch based on Bas' > one > to fix this 'overflow' issue? Would converting offset_to_idx(), > idx_to_offset() and > update_msr_sum() to use off_t instead of int be enough? Do you or > Terry have interest > to cook that patch? For Terry's version, I'm not sure if spliting > the code into different CPU vendor would benefit in the future, > except > that we would have plenty of new MSRs to be introduced in the future.
Yes, I believe updating the offset_to_idx(), idx_to_offset(), and update_msr_sum() functions is sufficient. I can do the incremental patch for that this evening if nobody beats me to it :) -- Calvin Walton <calvin.wal...@kepstin.ca>
