The patch LGTM overall. I had tested the v1 and it worked fine.
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.
Thank you for highlighting the discussions. I'm unsure about the best approach here, but I think it would be safe to stay consistent with the rest of the code in dsa.c, especially since it's unclear that the use of LW_EXCLUSIVE for reading values in dsa is a mistake.
> +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.
Sorry for the confusion, I am trying to say that we can change the following comment
+ *The area must have + * been created with dsa_create (not dsa_create_in_place).
to say this:
"The area must have been created using dsm_segments"
Since, this function can report the size of an area created with dsa_create_in_place too, as long as the area is created using dsm_segments.
> 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.
Fair enough. I was thinking of a scenario where a user might want to see how much dsa memory is allocated in the client backend process. However, I understand now that this view is designed for the entire cluster, and adding a column which is process-specific could lead to confusion.
