axboe / fio

Flexible I/O Tester
GNU General Public License v2.0
5.01k stars 1.23k forks source link

Flush enhancements and fixes #1777

Open minwooim opened 2 weeks ago

minwooim commented 2 weeks ago

This patchset introduces enhancements of fsync/fdatasync operations and fixes. The first patch added support for NVMe FLUSH commands in io_uring_cmd ioengine. The second one fixes issuing fsync/fdatasync even if the previous command is READ. As documentation says, fsync should sync the file after every N WRITE commands issued. The third one fixes not issuing fsync/fdatasync in trimwrite worklload. report.

Minwoo Im (4): io_uring: Add support FLUSH command io_u: ensure fsync only after write(s) io_u: Support fsync for --rw=trimwrite

vincentkfu commented 1 week ago

For the second patch, it looks like what's happening is that fio issues a write, followed by a sync, then a read, then (since last_was_sync is false) another sync.

For the second patch, why don't you also remove last_was_sync when adding last_ddir_issued?

In should_fsync replace the check of last_was_sync by checking if last_ddir_issued is false.

Also please rename last_ddir to last_ddir_completed.

Can you create a test case for this?

minwooim commented 1 week ago

For the second patch, it looks like what's happening is that fio issues a write, followed by a sync, then a read, then (since last_was_sync is false) another sync.

If last_ddir_issued are going to like:

last_ddir_issued = DDIR_WRITE
last_ddir_issued = DDIR_SYNC
last_ddir_issued = DDIR_READ

then if (should_fsync(td) && td->last_ddir_issued != DDIR_READ) should be false and another sync will not happen.

For the second patch, why don't you also remove last_was_sync when adding last_ddir_issued?

In should_fsync replace the check of last_was_sync by checking if last_ddir_issued is false.

If you meant last_ddir_issued is not DDIR_SYNC, got your point.

Also please rename last_ddir to last_ddir_completed.

Okay.

Can you create a test case for this?

Alright, I'll work on the mixed workload and create test cases using the issued rwts section of the report.

minwooim commented 2 days ago

@vincentkfu ,

I've added a test case to test whether FLUSH commands are issued properly or not. Please review.