On Thu, Nov 27, 2025 at 01:51:39PM +0530, Rahila Syed wrote:
> Thank you for this improvement. I think it will be very helpful to have
> the ability to report dsa and dshash memory sizes.
Thanks for reviewing!
> 1. As a result of this change, the following comment in
> pg_get_dsm_registry_allocations is incorrect.
Fixed.
> + LWLockAcquire(&control->lock, LW_SHARED);
>
> The dsa_get_total_size function takes LW_EXCLUSIVE lock for the same
> purpose. I wonder why that is the case and whether both should be
> consistent.
That function was added by commit ee1b30f, which AFAICT used an exclusive
lock just to stay consistent with the rest of dsa.c [0]. I don't see any
discussion about this in the original DSA thread [1]. Perhaps we could go
through dsa.c and switch to LW_SHARED where appropriate, although I doubt
it makes much difference.
> +size_t
> +dsa_get_total_size_from_handle(dsa_handle handle)
>
> I believe this function will report the size as long as the dsa control
> structure is created within a dsm segment, since all dsm segments are
> tracked by the global list - dsm_segment_list, regardless of whether the
> dsa is created with dsa_create or dsa_create_in_place. In that case,
> perhaps we should update the comment above to reflect this.
Sorry, I'm not following what you think we should update the comment to
say.
> 4. Since, with this change, the size column will show memory allocation
> regardless of whether it is currently mapped in the local process, I
> think it would be helpful to add a boolean column to display the mapped
> status as a future enhancement.
Maybe, although I'm struggling to think of a scenario where that
information would be useful.
[0] https://postgr.es/m/CAD21AoDoWrbNf%2BK2Fwg2n%3DCZDHigjkndwqy_86BGgXBp9Kbq4Q%40mail.gmail.com
[1] https://postgr.es/m/CAEepm%3D1z5WLuNoJ80PaCvz6EtG9dN0j-KuHcHtU6QEfcPP5-qA%40mail.gmail.com
--
nathan
