Skip to content

Topic/ompio pipeline iread iwrite3 #11383

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

Conversation

edgargabriel
Copy link
Member

@edgargabriel edgargabriel commented Feb 6, 2023

Second attempt at the pipelined iread/iwrite operations optimized for GPU buffers and external32 representation.

The current pr has three separate commits:

  1. identical commit to the first attempt ( I probably overreacted by reverting that one, but I didn't immediately see how to fix the problem).
  2. extract the most relevant parameters of the fileview and the file pointer position into a separate data structure
  3. Upon posting an File_iread/iwrite operation that uses the pipeline protocol:
    a. replicate the fileview structure from file handle onto the request. This way, the position calculated when posting the next segment from the progress engine is completely independent of what other operations on the file handle are currently ongoing. Hence, other iread/iwrite operations or e.g. setting the file view will not interfere with an already ongoing operation.
    b. Move the file pointer on the file handle to the expected end of the operation after initiating the iread/iwrite. This way, subsequent read/write operations will continue where the iread/iwrite is expected to end, and not interfere (overwrite) ongoing operations - unless the user explicitly moves the file pointer back.

Copy link
Contributor

@devreal devreal left a comment

Choose a reason for hiding this comment

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

I think there are still some thread-safety issues, hope my comments help.

@edgargabriel edgargabriel force-pushed the topic/ompio-pipeline-iread-iwrite3 branch from 2d4b280 to 72b8758 Compare February 18, 2023 16:36
@edgargabriel
Copy link
Member Author

@devreal thank you for your review and feedback (and sorry it took me so long to get to it), I think I addressed all items, except for the one question regarding the mutex before the loop.

@edgargabriel edgargabriel force-pushed the topic/ompio-pipeline-iread-iwrite3 branch 2 times, most recently from efa13b3 to 784499f Compare February 20, 2023 18:33
Pipelined iread/iwrite operations require the notion of subrequests, i.e. a user level request
can contain multiple internal subrequests that all have to complete before the user level operation
is considered finished. This requires adjustments
to the internal ompio progress engine and data structures.

Note: this is purely just a pipelined algorithm, no overlap between different iterations.

Extract the file view into a separate datastructure. This is required in
the next step since we need to cache file view and position of the file pointer on the request.

Replicate the file view and the file pointer position on the request.
This is required to correctly increment where to read/write data for every
subrequest, and handle the potential situation that the code changes the file
after posting an iread/iwrite but before the operation finishes.

Furthermore, when initiating an iread/iwrite we need to immidiatly move the position
of the handle to the end of the operation, such that subsequent read/write operations
use the correct position to start out with, and don't accidentaly interfere
with the already ongoing non-blocking operation.
Signed-off-by: Edgar Gabriel <[email protected]>

Signed-off-by: Edgar Gabriel <[email protected]>
@edgargabriel edgargabriel force-pushed the topic/ompio-pipeline-iread-iwrite3 branch from 784499f to 25b8dd2 Compare February 20, 2023 19:56
@edgargabriel edgargabriel merged commit a73399e into open-mpi:main Feb 20, 2023
@edgargabriel edgargabriel deleted the topic/ompio-pipeline-iread-iwrite3 branch July 12, 2024 12:30
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.

2 participants