Hi,
Sorry for the delay and for the noise on this older version. I first want to understand the code better.
On Thursday 22 Oct 2020 at 11:55:28 (+0100), Lukasz Luba wrote: [..]
+{
- /* Make some space if needed */
- if (status->busy_time > 0xffff) {
status->busy_time >>= 10;status->total_time >>= 10;- }
How about removing the above code and adding here:
status->busy_time = status->busy_time ? : 1;
It's not equivalent. The code operates on raw device values, which might be big (e.g. read from counters). If it's lager than the 0xffff, it is going to be shifted to get smaller.
Yes, the big values are handled below through the division and by making total_time = 1024. These two initial checks are only to cover the possibility for busy_time and total_time being 0, or busy_time > total_time.
- if (status->busy_time > status->total_time)
This check would then cover the possibility that total_time is 0.
status->busy_time = status->total_time;But a reversal is needed here: status->total_time = status->busy_time;
No, I want to clamp the busy_time, which should not be bigger that total time. It could happen when we deal with 'raw' values from device counters.
Yes, I understand. But isn't making total_time = busy_time accomplishing the same thing?
- status->busy_time *= 100;
- status->busy_time /= status->total_time ? : 1;
- /* Avoid division by 0 */
- status->busy_time = status->busy_time ? : 1;
- status->total_time = 100;
Then all of this code can be replaced by:
status->busy_time = (unsigned long)div64_u64((u64)status->busy_time << 10, status->total_time); status->total_time = 1 << 10;
No, the total_time closed to 'unsigned long' would overflow.
I'm not sure I understand. total_time gets a value of 1024, it's not itself shifted by 10.
This way you gain some resolution to busy_time and the divisions in the callers would just become shifts by 10.
I don't want to gain more resolution here. I want to be prepare for raw (not processed yet) big values coming from driver.
Agreed! The higher resolution is an extra benefit. The more important benefit is that, through my suggestion, you'd be replacing all future divisions by shifts.
Thanks, Ionela.
Regards, Lukasz
Hope it helps, Ionela.