On Wed, Aug 28, 2024 at 12:36:08PM GMT, Dave Chinner wrote:
> Doing this inside the tight loop (adding 3 atomics and two calls to
> ktime_get_ns()) is total overkill. Such fine grained accounting
> doesn't given any extra insight into behaviour compared to
> accounting the entire loop once.
>
> e.g. the actual time the shrinker takes to run is the time the whole
> loop takes to run, not only the individual shrinker->scan_objects
> call.
>
> The shrinker code already calculates the total objects scanned and
> the total objects freed by the entire loop, so there is no reason to
> be calculating this again using much more expensive atomic
> operations.
>
> And these are diagnostic stats - the do not need to be perfectly
> correct and so atomics are unnecessary overhead the vast majority of
> the time. This code will have much lower impact on runtime overhead
> written like this:
>
> start_time = ktime_get_ns();
>
> while (total_scan >= batch_size ||
> total_scan >= freeable) {
> .....
> }
>
> shrinker->objects_requested_to_free += scanned;
> shrinker->objects_freed += freed;
>
> end_time = ktime_get_ns();
> shrinker->ns_run += end_time - start_time;
> shrinker->last_scanned = end_time;
> if (freed)
> shrinker->last_freed = end_time;
>
> And still give pretty much the exact same debug information without
> any additional runtime overhead....
*nod*
This is all going to be noise compared to the expense of ->scan()
itself, but there's no reason not to, I'll make the change...
