Hello!

I noticed that the patch has some issues with minmax_only, and these
differences seem to be unintended to me:

1. minmax only resets updates mean/sum_var_time (but then when it
calculates them again, it also uses the data before the minmax reset)

+               /*
+                * Reset only min/max timing values and update 
minmax_stats_since.
+                * Iterate matching entries in the dshash, reset the 
corresponding
+                * pgstat counters' min/max fields.
+                */
...
+                                               
shared->counters.mean_time[kind] = 0;
+                                               
shared->counters.sum_var_time[kind] = 0;

2. min_time doesn't recover after a minmax reset:

+               if (pending->min_time[kind] < shared->min_time[kind])
+                       shared->min_time[kind] = pending->min_time[kind];

As it is only updated if it's smaller, but it is 0 after it, and
because the call count remains > 0 it doesn't get initialized with the
current value.



+               if ((int) pg_atomic_read_u64(&pgss_shared->nentries) <= 
pgss_max *
(100 - USAGE_DEALLOC_PERCENT) / 100)
+                       break;

isn't there a possible overflow here? The similar deleted check had
explicit uint64 casts for pgss_max multiplication.

-       /*
-        * Don't proceed if file does not exceed 512 bytes per possible entry.
-        */
-       if ((uint64) extent < (uint64) 512 * pgss_max)
-               return false;


Reply via email to