On Tue, Nov 05, 2024 at 08:37:08PM +0300, Karina Litskevich wrote: > I suggest eliminating holding the excessive lock. See the attached patch. > This would also restore the consistency between the code and the comments > about entry's mutex spinlock usage.
You are right. minmax_stats_since and stats_since are only set when an entry is allocated or reset, so this is not going to matter. > + /* > + * There is no need to hold entry->mutex when reading > stats_since and > + * minmax_stats_since for (unlike counters) they are always > written > + * while holding pgss->lock exclusively. We are holding > pgss->lock > + * shared so there should be no race here. > + */ > stats_since = entry->stats_since; > minmax_stats_since = entry->minmax_stats_since; > - SpinLockRelease(&entry->mutex); The comment could be simpler, say a "The spinlock is not required when reading these two as they are always updated when holding pgss->lock exclusively.". Or something like that. -- Michael
signature.asc
Description: PGP signature