Skip to content

Add a Doxygen note about max IO size to API calls #206

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.

Sign up for GitHub

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

Open
derobins wants to merge 1 commit into
base: develop
Choose a base branch
from
+9 −0
Open

Add a Doxygen note about max IO size to API calls #206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/Doxyfile.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ TAB_SIZE = 4
# @} or use a double escape (\\{ and \\})

ALIASES = hipFileError_t="A hipFileError_t struct that holds both hipFile and HIP error values"
ALIASES += max_io_size_note="@note The maximum IO size is determined by the Linux kernel and is currently 2^31 - PAGE_SIZE"
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This alias claims the max IO size is “determined by the Linux kernel” and “currently 2^31 - PAGE_SIZE”, but hipFile appears to hard-code MAX_RW_COUNT to 0x7ffff000 (see src/amd_detail/backend.h). On systems with non-4K PAGE_SIZE, the kernel’s MAX_RW_COUNT differs, so this note could be misleading. Suggest rewording to match what hipFile actually enforces (e.g., explicitly say hipFile caps requests to 0x7ffff000 / Linux MAX_RW_COUNT on supported platforms), or update the implementation to derive the limit from PAGE_SIZE if that’s the intended behavior.

Suggested change
ALIASES += max_io_size_note="@note The maximum IO size is determined by the Linux kernel and is currently 2^31 - PAGE_SIZE"
ALIASES += max_io_size_note="@note hipFile currently caps individual IO requests to 0x7ffff000 bytes (matching Linux MAX_RW_COUNT on systems with 4 KiB pages)."

Copilot uses AI. Check for mistakes.

# Set the OPTIMIZE_OUTPUT_FOR_C tag to YES if your project consists of C sources
# only. Doxygen will then generate output that is more tailored for C. For
Expand Down
8 changes: 8 additions & 0 deletions include/hipfile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,8 @@ hipFileError_t hipFileBufDeregister(const void *buffer_base);
* hipFileRead() will transfer at most 0x7ffff000 (2,147,479,552) bytes,
* returning the number of bytes actually transferred.
Comment on lines 532 to 533
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The doc now states both a fixed max transfer size (0x7ffff000) and then expands \max_io_size_note (“2^31 - PAGE_SIZE”). These can diverge (e.g., non-4K PAGE_SIZE) and read as contradictory. Consider replacing the hard-coded 0x7ffff000 sentence with the alias (or adjust the alias to match the constant used by hipFile) so there is a single authoritative statement of the limit.

Suggested change
* hipFileRead() will transfer at most 0x7ffff000 (2,147,479,552) bytes,
* returning the number of bytes actually transferred.
* hipFileRead() will transfer at most the maximum I/O size described in
* \max_io_size_note, returning the number of bytes actually transferred.

Copilot uses AI. Check for mistakes.
*
* \max_io_size_note
*
* @param [in] fh Opaque file handle for GPU IO operations
* @param [in] buffer_base Base address of the GPU buffer
* @param [in] size Number of bytes that should be read
Expand All @@ -553,6 +555,8 @@ ssize_t hipFileRead(hipFileHandle_t fh, void *buffer_base, size_t size, hoff_t f
* hipFileWrite() will transfer at most 0x7ffff000 (2,147,479,552) bytes,
* returning the number of bytes actually transferred.
*
* \max_io_size_note
*
Comment on lines 555 to +559
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

Same issue as hipFileRead(): the comment includes a fixed max (0x7ffff000) and also \max_io_size_note (“2^31 - PAGE_SIZE”). To avoid potentially conflicting documentation, prefer one source of truth (either use the alias in place of the hard-coded value, or make the alias explicitly reflect the constant hipFile enforces).

Copilot uses AI. Check for mistakes.
* @param [in] fh Opaque file handle for GPU IO operations
* @param [in] buffer_base Base address of the GPU buffer
* @param [in] size Number of bytes that should be written
Expand Down Expand Up @@ -898,6 +902,8 @@ void hipFileBatchIODestroy(hipFileBatchHandle_t batch_idp);
* @param [in] stream The hipStream to enqueue this async IO request.
* If NULL, this request will be synchronous.
*
* \max_io_size_note
*
* @return \hipFileError_t
* @return hipFileSuccess
* @return hipFileDriverError
Expand All @@ -912,6 +918,8 @@ hipFileError_t hipFileReadAsync(hipFileHandle_t fh, void *buffer_base, size_t *s
* @brief Perform an asynchronous write to a stream
* @ingroup async
*
* \max_io_size_note
*
* @param [in] fh Opaque file handle for GPU IO operations
* @param [in] buffer_base Base address of the GPU buffer
* @param [in] size_p Number of bytes that should be written
Expand Down