On Wed, May 27, 2020 at 05:08:55PM +0300, Alexander Monakov wrote: > On Wed, 27 May 2020, Guenter Roeck wrote: > > > >> I understand that. If you anticipate that the module may be inserted after a > > >> wraparound, the driver should populate 'prev_value' with actual counter > > >> values instead of zeros. That way the driver will properly accumulate > > >> energy over time it's been inserted. As implemented, the driver counts > > >> energy since boot time, minus unknown amount lost to wraparounds if the > > >> driver was loaded too late. > > > No problem if this module is built into the kernel. > > > If this module is inserted at later point, unless the user keeps the > > > counters since > > > the boot and provide it as an input during the module insert (we can > > > implement this). > > > > I won't accept such a parameter. If you may recall, I did bring this up as > > reason why I abandoned the matching change for the coretemp driver, predicting > > that people would complain about it. Now they do. Not surprising. We (and you) > > will have to live with it. > > I'm not exactly complaining, I'm proposing a solution: at probe time, populate > prev_value members with MSR values instead of zeros. That way, the module will > correctly count energy over the time it's been loaded. It can be unloaded and > reloaded freely, and doing so would allow to easily measure energy across large > spans of time, which sounds like an improvement. > That would ignore energy accumulated from before the driver was loaded, and would thus trigger another set of complaints. A slight improvement might be to add up core energy counters when loading the driver, compare it against the package counter, and pick the larger value for the initial package counter(s). This would at least ensure that the package counter is never less than the sum of the core counters. Guenter > I can try to cook a corresponding patch if that sounds alright. > > Cheers. > Alexander
