fio
fio copied to clipboard
Add posixaio_waitcomplete engine.
Provide a variant of the posixaio engine that uses FreeBSD's aio_waitcomplete() function to consume completions, instead of running around polling all IOs with aio_error().
To preserve the traditional posixaio behaviour and for fair comparison with it, it drains as many extra completions as it can without waiting. To disable that and drain just the minimum number as some other engines do, there's a new option --posixaio-drain-min; this might keep the IO queue depth more stable/full.
Overall looks fine, just a few notes:
- Please update HOWTO/fio.1 with the new engine option
- Commit needs a Signed-off-by at the end with your name/email
- Use container_of(), that's much more readable. Just drop the io_u_from_aiocb() helper.
- Minor style issues noted
Actually, the style issue I was going to mention goes away when you kill io_u_from_aiocb().
(Sorry for the force pushes, not too used to github workflow and didn't know that'd show up like that. :-))
Thanks for the feedback! I was writing up the docs and realised there is a better way to spin this: it's kinda sorta arguably a bug that posixaio doesn't respect iodepth_batch_complete_max. Let's just make posixaio_waitcomplete behave "normally", and add an option for posixaio that opts into a behaviour change to make it more like other engines. Hence, split into two commits. What do you think?
I’m used to it as the person pulling, I don’t use gh much as a contributor. I just wait until the person tells me it’s now good to go :-)
Splitting into two commits seems sane, I generally always prefer splitting if it makes any sense to do so.
On Aug 21, 2021, at 10:41 PM, Thomas Munro @.***> wrote:
(Sorry for the force pushes, not too used to github workflow and didn't know that'd show up like that. :-))
Thanks for the feedback! I was writing up the docs and realised there is a better way to spin this: it's kinda sorta arguably a bug that posixaio doesn't respect iodepth_batch_complete_max. Let's just make posixaio_waitcomplete behave "normally", and add an option for posixaio that opts into a behaviour change to make it more like other engines. Hence, split into two commits. What do you think?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.
Now on laptop and looking at it, I think the best approach is to just have this be an option for the posixaio engine, if the platform supports the waitcomplete mode. Don't think a an extra engine makes a lot of sense, even if they end up having two separate getevents implementations. Just have a parent one that picks the right one.
Provide an option for the posixaio engine that selects which mode to use, and just error if waitcomplete is specified and it isn't available.
It'd be nice to include some performance numbers from the commit enabling waitcomplete in the commit message.
@macdice any follow up on this one?
Sorry for the delay: I will post a new patch this weekend coming, with an option as requested.
Done. Seems quite nice this way. BTW I plan to propose a couple more values for posixaio_wait, in later PRs (sigwaitinfo and kevent).