On Thu, Mar 05, 2026 at 04:25:57PM -0600, Sami Imseih wrote: > With the above assert in place, the usable_pages of 879 ars you have > in test_dsa.c crashes > due to the assertion failure.
Yep, the assertion looks useful to have in place. > As far as the fix, I do agree with what you have in 0001-, except I am > not so sure about the "+1 for rounding". > Can we just do ceiling division? Ceiling division is more precise at the end, and can be checked, I'd tend to stick with your approach. > + /* > + * We must also account for pagemap entries needed to cover > the > + * metadata pages themselves. The pagemap must track > all pages in the > + * segment, including the pages occupied by metadata. > + */ > + metadata_bytes += > + ((metadata_bytes + (FPM_PAGE_SIZE - > sizeof(dsa_pointer)) - 1) / > + (FPM_PAGE_SIZE - sizeof(dsa_pointer))) * > + sizeof(dsa_pointer); > > I don't think we should add a test_dsa, but I do think it was useful > to verify the issue. I think that a test would be actually nice to have in test_dsa, but the proposed one lacks flexibility. How about changing it to a function that takes in input the values you'd want to test for pagemap_start, usable_pages and the offset? The point is to check a dsa_allocate() with the sanity of an offset, hence a simple test_dsa_allocate() as function name? The test module exists down to v17, which should be enough to check things across platforms moving ahead. > This sounds like it should be backpatched, but we'll see what a > committer thinks. This should be backpatched. If you'd have told me that the allocation is oversized, then an optimization may have been possible on HEAD, especially if overestimation was large. A small reduction in size may not even be worth worrying in some cases, as we tend to oversize some shmem areas as well. An undersized calculation is a very different story: memory corruptions on the stack are not cool at all, especially on stable branches, and they lead to unpredictible behaviors. -- Michael
signature.asc
Description: PGP signature
