Update and document max IO size handling#206
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a reusable Doxygen note describing the maximum single-call I/O size limit, and inserts that note into the public API documentation for read/write calls.
Changes:
- Introduce a new Doxygen alias (
max_io_size_note) describing the max I/O size limit. - Reference
\max_io_size_notein the documentation blocks forhipFileRead,hipFileWrite,hipFileReadAsync, andhipFileWriteAsync.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| include/hipfile.h | Adds \max_io_size_note to multiple API doc blocks (sync + async read/write). |
| docs/Doxyfile.in | Defines the max_io_size_note Doxygen alias used by the header docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5dbe70c to
ca7c174
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
include/hipfile.h:536
- The doc comment still hard-codes the max transfer size as
0x7ffff000 (2,147,479,552)for hipFileRead/hipFileWrite, but the new implementation/note makes this dependent on the system page size (e.g., 64KiB pages yield a different limit). To avoid incorrect docs on non-4KiB systems, please remove or update these hard-coded values and rely on the new \max_io_size_note (or make the text page-size-aware).
* hipFileRead() will transfer at most 0x7ffff000 (2,147,479,552) bytes,
* returning the number of bytes actually transferred.
*
* @param [in] fh \hipfile_handle_param
* @param [in] buffer_base \buffer_base_param
* @param [in] size Number of bytes that should be read
* @param [in] file_offset Offset into the file that should be read from
* @param [in] buffer_offset Offset of the GPU buffer that that the data should be written to
*
* \max_io_size_note
*
* @return if >= 0: Number of bytes read
* @return if -1: System error (check `errno` for the specific error)
* @return else: Negative value of the related hipFileOpError_t
*/
HIPFILE_API
ssize_t hipFileRead(hipFileHandle_t fh, void *buffer_base, size_t size, hoff_t file_offset,
hoff_t buffer_offset);
/*!
* @brief Synchronously write data from a GPU buffer to a file
* @ingroup file
*
* hipFileWrite() will transfer at most 0x7ffff000 (2,147,479,552) bytes,
* returning the number of bytes actually transferred.
*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
ca7c174 to
39a2918
Compare
Since PAGE_SIZE is not the same on all plaforms, mirror the kernel's computation of MAX_RW_COUNT so that hipFile doesn't use an incorrect value.
39a2918 to
015c09f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The max IO size constants are initialized using a lambda that: * Occurs in every TU where it's used * Can throw on errors during static initialization The solution is to replace the globals + lambda with inline functions that have static variables.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* Minor change to Doxygen macro * _SC_PAGE_SIZE --> _SC_PAGESIZE
No description provided.