Skip to content

Add AIO and Direct IO support for IOR phases#104

Draft
jxiong wants to merge 1 commit intoIO500:mainfrom
jxiong:aio_dio_support
Draft

Add AIO and Direct IO support for IOR phases#104
jxiong wants to merge 1 commit intoIO500:mainfrom
jxiong:aio_dio_support

Conversation

@jxiong
Copy link
Copy Markdown

@jxiong jxiong commented Mar 1, 2026

This commit introduces a set of features and fixes to improve IOR execution flexibility and overall test stability:

IOR Execution Enhancements:

  • Added '--with-aio' to IOR configuration in prepare.sh to enable AIO.
  • Introduced 'direct' option for IOR phases to support '--posix.odirect'.
  • Added 'iteration' option to the IOR easy read phase.

Phase Execution & Validation:

  • Added should_run_phase() check in main.c to properly skip disabled phases.
  • Improved the stonewall condition validation logic in phase_dbg.c.

Bug Fixes:

  • Added CFLAGS="-fPIC" for pfind compilation in prepare.sh.
  • Fixed missing return value validation for fread() in phase_mdworkbench.c.

This commit introduces a set of features and fixes to improve IOR execution
flexibility and overall test stability:

IOR Execution Enhancements:
- Added '--with-aio' to IOR configuration in prepare.sh to enable AIO.
- Introduced 'direct' option for IOR phases to support '--posix.odirect'.
- Added 'iteration' option to the IOR easy read phase.

Phase Execution & Validation:
- Added should_run_phase() check in main.c to properly skip disabled phases.
- Improved the stonewall condition validation logic in phase_dbg.c.

Bug Fixes:
- Added CFLAGS="-fPIC" for pfind compilation in prepare.sh.
- Fixed missing return value validation for fread() in phase_mdworkbench.c.

Signed-off-by: Jinshan Xiong <jinshanx@google.com>
@jxiong jxiong requested a review from a team as a code owner March 1, 2026 00:47
@jxiong jxiong marked this pull request as draft March 5, 2026 04:51
@seattleplus
Copy link
Copy Markdown
Contributor

Huge thanks for the patch.

I think the "IOR execution" portions of direct, iteration, aio need further discussion and should be separated into a separate PR. The bug fixes and phase execution would also be great if they could be separated into 2 separate PRs.

On the ior execution pieces, the iteration part is not in the spirit of io500 and could bias the results, and so that will not be accepted. We need to discuss the aio portion, as we might prefer for users to just do this another way...but thank you for the example of how to get it done. On the direct, we don't want to add a new var to the function for every new option, so we might think about ways to generalize this, but thanks for the patch as an example of how to get started.

I'm a bit confused as to why should_run_phase is needed?

I'll let the others comment on the other fixes...

@jxiong
Copy link
Copy Markdown
Author

jxiong commented Mar 5, 2026

For the direct part, I can make it to 'aio --posix.direct' just like the current 'POSIX --posix.direct' does.

iteration is to stabilize the reading result. The publishers will run the whole tests multiple times and pick the best ones anyways, why not make their life easier by embedding this option? I will remove it in the next refresh.

The should_run_phase() is to honor the config options in the ini file. Otherwise it would have this results:

ERROR INVALID (src/main.c:437) Runtime of phase (0.000012) is below stonewall time. This shouldn't happen!
ERROR INVALID (src/main.c:443) Runtime is smaller than expected minimum runtime
[RESULT]    mdtest-hard-write        0.000000 kIOPS : time 0.000 seconds [INVALID]

I agree the fix is not necessary for a valid run but just trying to make it better. In my recent io500 run, I didn't include anything from this patch.

@gflofst
Copy link
Copy Markdown
Contributor

gflofst commented Mar 11, 2026

@JulianKunkel, can you comment on some of the deeper code changes? We aren't sure about the implications of this.

@JulianKunkel
Copy link
Copy Markdown
Contributor

For me, this appears to works for individual phases:
API = AIO --aio.max-pending=128 --aio.granularity=8 --posix.odirect

(using the latest IOR main)

Copy link
Copy Markdown
Contributor

@JulianKunkel JulianKunkel left a comment

Choose a reason for hiding this comment

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

See my comments please.

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.

I agree to this change.

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.

Wouldn't accept these changes, anyone can modify it. We will integrate some change to clarify that you are allowed to change it.

{"uniqueDir", "Use unique directory per file per process", 0, INI_BOOL, "FALSE", & ior_easy_o.uniqueDir},
{"run", "Run this phase", 0, INI_BOOL, "TRUE", & ior_easy_o.run},
{"verbosity", "The verbosity level", 0, INI_INT, 0, & ior_easy_o.verbosity},
{"direct", "Use direct IO (posix.odirect)", 0, INI_BOOL, "TRUE", & ior_easy_o.direct},
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.

Can make an exception for supporting posix.odirect for AIO module.
I suppose you tried using --posix.odirect even though AIO is used?
Let's recap what users expect.

I think what should have worked was sth like:
API = POSIX --aio.max-pending=128 --aio.granularity=8 --posix.odirect
Or did it not?

Copy link
Copy Markdown
Author

@jxiong jxiong Mar 17, 2026

Choose a reason for hiding this comment

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

This is using POSIX direct IO. What I am trying to do is to enable AIO + direct IO. Something like AIO --aio.max-pending=128 --aio.granularity=8 --posix.odirect would be equivalent but I didn't test it out.

EDITTED: AIO --aio.max-pending=128 --aio.granularity=8 --posix.odirect works with a warning message to complain prefix with --with-aio in the IOR build config.

IO500 version 66fac1a7cca8 (standard)
; WARNING Provided API option --posix.odirect starts with a different prefix and might be not supported - don't worry if you checked!
WARNING Provided API option --posix.odirect starts with a different prefix and might be not supported - don't worry if you checked!
WARNING Provided API option --posix.odirect starts with a different prefix and might be not supported - don't worry if you checked!

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.

Remind me, why is the current approach not sufficient? It does check right?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yeah I think I can deprecate this patch.

@adilger
Copy link
Copy Markdown
Contributor

adilger commented Mar 16, 2026

Hi @jxiong, can you please split your PR into separate patches for each of the parts, so that they can be reviewed and landed separately.

@jxiong
Copy link
Copy Markdown
Author

jxiong commented Mar 17, 2026

Hi @jxiong, can you please split your PR into separate patches for each of the parts, so that they can be reviewed and landed separately.

Will do

JulianKunkel pushed a commit that referenced this pull request Mar 18, 2026
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.

5 participants