Skip to content

hipFile: Limit IO size to MAX_RW_COUNT on fastpath#165

Merged
derobins merged 13 commits into
developfrom
kumcmill/aiscp-large-files
Jan 22, 2026
Merged

hipFile: Limit IO size to MAX_RW_COUNT on fastpath#165
derobins merged 13 commits into
developfrom
kumcmill/aiscp-large-files

Conversation

@kurtmcmillan

@kurtmcmillan kurtmcmillan commented Jan 20, 2026

Copy link
Copy Markdown
Collaborator

Motivation

When testing aiscp with files larger than MAX_RW_COUNT amdgpu/kfd would return an ILLEGAL SEEK error. To address this, the AIS path (fastpath) will the limit IO size to MAX_RW_COUNT. This matches the behavior of POSIX read/write (see man pages).

aiscp is also updated to handle files larger than MAX_RW_COUNT.

Finally, since I was in the area, aiscp now handles files with zero size.

AIHIPFILE-111

@kurtmcmillan kurtmcmillan self-assigned this Jan 20, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses an ILLEGAL SEEK error that occurs when processing files larger than MAX_RW_COUNT (0x7ffff000 bytes) in the AIS fastpath. The fix limits IO operations to MAX_RW_COUNT, matching POSIX read/write behavior, and updates the aiscp example to handle large files through chunked transfers.

Changes:

  • Limits IO size to MAX_RW_COUNT in the fastpath backend to prevent kernel driver errors
  • Updates aiscp to perform chunk-based copying for files larger than MAX_RW_COUNT
  • Adds handling for zero-size files in aiscp

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/amd_detail/backend/fastpath.cpp Adds MAX_RW_COUNT size limitation to prevent ILLEGAL SEEK errors
test/amd_detail/fastpath.cpp Adds test to verify IO size is properly truncated to MAX_RW_COUNT
examples/aiscp/aiscp.cpp Implements chunked file copying with alignment support and zero-size file handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/amd_detail/backend/fastpath.cpp Outdated
Comment thread examples/aiscp/aiscp.cpp Outdated
Comment thread examples/aiscp/aiscp.cpp Outdated
Comment thread test/amd_detail/fastpath.cpp Outdated
kurtmcmillan and others added 3 commits January 21, 2026 09:14
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…oMaxRWCount

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/amd_detail/backend/fastpath.cpp
Comment thread examples/aiscp/aiscp.cpp
Comment thread examples/aiscp/aiscp.cpp Outdated
}
// Copy the file chunk-by-chunk until we hit EOF
do {
nread = hipFileRead(src_handle, devbuf, buffer_size, ncopy, 0);

@derobins derobins Jan 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ncopy is of type ssize_t and the parameter is an offset of type hoff_t, which is confusing. I would probably rename ncopy and use the correct type.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

@derobins
derobins merged commit ff74796 into develop Jan 22, 2026
33 checks passed
@derobins
derobins deleted the kumcmill/aiscp-large-files branch January 22, 2026 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants