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

Attachment: signature.asc
Description: PGP signature

Reply via email to