Skip to content

fbtl/posix: fix data-sieving calculations #11932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

edgargabriel
Copy link
Member

@edgargabriel edgargabriel commented Sep 15, 2023

as part of introducing atomicity support for ompi v5.0, we also tried to improve the robustness in some file I/O routines. Unfortunately, this also introduced a bug since ret_code returned by a function does not necessarily contain the number of bytes read or written,
but could contain the last value (e.g. 0). The value was however used in a subsequent calculation and we ended not copying data out of the temporary buffer used in the data sieving at all.

This commit also simplifies some of the logic in the while loop, no need to retry to read past the end of the file multiple times.

Fixes issue #11917

Code was tested with the reproducer provided as part of the issue, our internal testsuite, and the hdf5-1.14.2 testsuite, all tests pass.

as part of introducing atomicity support for ompi v5.0, we also tried to improve the robustness in some file I/O routines.
Unfortunately, this also introduced a bug since ret_code returned by a function does not necessarily contain the number of
bytes read or written,
but could contain the last value (e.g. 0). The value was however used in a subsequent calculation and we ended not copying
data out of the temporary buffer used in the data sieving at all.

This commit also simplifies some of the logic in the while loop, no need to retry to read past the end of the file
multiple times.

Fixes issue open-mpi#11917

Code was tested with the reproducer provided as part of the issue, our internal testsuite, and the hdf5-1.4.2 testsuite, all tests pass.

Signed-off-by: Edgar Gabriel <edgar.gabriel@amd.com>
@edgargabriel
Copy link
Member Author

I am looking for a volunteer to review this patch, since it also needs to be ported afterwards to 5.0

Copy link
Contributor

@qkoziol qkoziol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@edgargabriel
Copy link
Member Author

LGTM

thank you!

@janjust janjust merged commit a86c131 into open-mpi:main Sep 19, 2023
@janjust
Copy link
Contributor

janjust commented Sep 19, 2023

@edgargabriel please open up v5.0 cherry pick

@edgargabriel
Copy link
Member Author

@janjust done, #11933

@edgargabriel edgargabriel deleted the topic/data-sieving-fix branch July 12, 2024 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants