Skip to content
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

Jump to bottom

Flush enhancements and fixes #1777

Merged
merged 6 commits into from
Jul 11, 2024
Merged

Flush enhancements and fixes #1777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7116c1f
io_uring: Add support FLUSH command
minwooim Jun 22, 2024
87b1002
td: Rename last_ddir to last_ddir_completed
minwooim Jul 9, 2024
e11046b
td: Replace last_was_sync with last_ddir_issued
minwooim Jul 9, 2024
704a8cf
io_u: Ensure fsync only after write(s)
minwooim Jul 9, 2024
cf168c5
io_u: Support fsync for --rw=trimwrite
minwooim Jun 22, 2024
e84adcf
t/nvmept.py: Add test cases for FLUSH
minwooim Jul 4, 2024
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
5 changes: 5 additions & 0 deletions engines/nvme.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,11 @@ int fio_nvme_uring_cmd_prep(struct nvme_uring_cmd *cmd, struct io_u *io_u,
case DDIR_TRIM:
fio_nvme_uring_cmd_trim_prep(cmd, io_u, dsm);
return 0;
case DDIR_SYNC:
case DDIR_DATASYNC:
cmd->opcode = nvme_cmd_flush;
cmd->nsid = data->nsid;
return 0;
default:
return -ENOTSUP;
}
Expand Down
1 change: 1 addition & 0 deletions engines/nvme.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ enum nvme_admin_opcode {
};

enum nvme_io_opcode {
nvme_cmd_flush = 0x00,
nvme_cmd_write = 0x01,
nvme_cmd_read = 0x02,
nvme_cmd_write_uncor = 0x04,
Expand Down
7 changes: 4 additions & 3 deletions fio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,9 @@ struct thread_data {
size_t orig_buffer_size;
volatile int runstate;
volatile bool terminate;
bool last_was_sync;
enum fio_ddir last_ddir;

enum fio_ddir last_ddir_completed;
enum fio_ddir last_ddir_issued;

int mmapfd;

Expand Down Expand Up @@ -629,7 +630,7 @@ static inline bool multi_range_trim(struct thread_data *td, struct io_u *io_u)

static inline bool should_fsync(struct thread_data *td)
{
if (td->last_was_sync)
if (ddir_sync(td->last_ddir_issued))
return false;
if (td_write(td) || td->o.override_sync)
return true;
Expand Down
10 changes: 5 additions & 5 deletions io_u.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ static enum fio_ddir get_rw_ddir(struct thread_data *td)
* See if it's time to fsync/fdatasync/sync_file_range first,
* and if not then move on to check regular I/Os.
*/
if (should_fsync(td)) {
if (should_fsync(td) && td->last_ddir_issued == DDIR_WRITE) {
if (td->o.fsync_blocks && td->io_issues[DDIR_WRITE] &&
!(td->io_issues[DDIR_WRITE] % td->o.fsync_blocks))
return DDIR_SYNC;
Expand Down Expand Up @@ -815,7 +815,7 @@ static void set_rw_ddir(struct thread_data *td, struct io_u *io_u)
if (td->o.zone_mode == ZONE_MODE_ZBD)
ddir = zbd_adjust_ddir(td, io_u, ddir);

if (td_trimwrite(td)) {
if (td_trimwrite(td) && !ddir_sync(ddir)) {
struct fio_file *f = io_u->file;
if (f->last_start[DDIR_WRITE] == f->last_start[DDIR_TRIM])
ddir = DDIR_TRIM;
Expand Down Expand Up @@ -1757,7 +1757,7 @@ static bool check_get_trim(struct thread_data *td, struct io_u *io_u)
if (get_next_trim(td, io_u))
return true;
} else if (!(td->io_hist_len % td->o.trim_backlog) &&
td->last_ddir != DDIR_READ) {
td->last_ddir_completed != DDIR_READ) {
td->trim_batch = td->o.trim_batch;
if (!td->trim_batch)
td->trim_batch = td->o.trim_backlog;
Expand All @@ -1779,7 +1779,7 @@ static bool check_get_verify(struct thread_data *td, struct io_u *io_u)
if (td->verify_batch)
get_verify = 1;
else if (!(td->io_hist_len % td->o.verify_backlog) &&
td->last_ddir != DDIR_READ) {
td->last_ddir_completed != DDIR_READ) {
td->verify_batch = td->o.verify_batch;
if (!td->verify_batch)
td->verify_batch = td->o.verify_backlog;
Expand Down Expand Up @@ -2122,7 +2122,7 @@ static void io_completed(struct thread_data *td, struct io_u **io_u_ptr,
return;
}

td->last_ddir = ddir;
td->last_ddir_completed = ddir;

if (!io_u->error && ddir_rw(ddir)) {
unsigned long long bytes = io_u->xfer_buflen - io_u->resid;
Expand Down
4 changes: 2 additions & 2 deletions ioengines.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ enum fio_q_status td_io_queue(struct thread_data *td, struct io_u *io_u)
td->ts.total_io_u[io_u->ddir]++;
}

td->last_was_sync = ddir_sync(io_u->ddir);
td->last_ddir_issued = ddir;
} else if (ret == FIO_Q_QUEUED) {
td->io_u_queued++;

Expand All @@ -448,7 +448,7 @@ enum fio_q_status td_io_queue(struct thread_data *td, struct io_u *io_u)
if (td->io_u_queued >= td->o.iodepth_batch)
td_io_commit(td);

td->last_was_sync = ddir_sync(io_u->ddir);
td->last_ddir_issued = ddir;
}

if (!td_ioengine_flagged(td, FIO_SYNCIO) &&
Expand Down
1 change: 0 additions & 1 deletion libfio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ static void reset_io_counters(struct thread_data *td, int all)

td->zone_bytes = 0;

td->last_was_sync = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we never should have set last_was_sync to false here because when ramp time is over we want to continue to sync at the appropriate point, so there is no need to modify the value for last_ddir_issued. Agree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you meant we don't need to modify the value of last_ddir_issued in this place, yes, I agree.

td->rwmix_issues = 0;

/*
Expand Down
91 changes: 91 additions & 0 deletions t/nvmept.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,53 @@ def check_result(self):
self.passed = False


class FlushTest(FioJobCmdTest):
def setup(self, parameters):
fio_args = [
"--name=nvmept-flush",
"--ioengine=io_uring_cmd",
"--cmd_type=nvme",
"--randrepeat=0",
f"--filename={self.fio_opts['filename']}",
f"--rw={self.fio_opts['rw']}",
f"--output={self.filenames['output']}",
f"--output-format={self.fio_opts['output-format']}",
]

for opt in ['fixedbufs', 'nonvectored', 'force_async', 'registerfiles',
'sqthread_poll', 'sqthread_poll_cpu', 'hipri', 'nowait',
'time_based', 'runtime', 'verify', 'io_size', 'num_range',
'iodepth', 'iodepth_batch', 'iodepth_batch_complete',
'size', 'rate', 'bs', 'bssplit', 'bsrange', 'randrepeat',
'buffer_pattern', 'verify_pattern', 'offset', 'fdp',
'fdp_pli', 'fdp_pli_select', 'dataplacement', 'plid_select',
'plids', 'dp_scheme', 'number_ios', 'read_iolog', 'fsync']:
if opt in self.fio_opts:
option = f"--{opt}={self.fio_opts[opt]}"
fio_args.append(option)

super().setup(fio_args)

def check_result(self):
super().check_result()

job = self.json_data['jobs'][0]

rw = self.fio_opts['rw']
fsync = self.fio_opts['fsync']

nr_write = job['write']['total_ios']
nr_sync = job['sync']['total_ios']

nr_sync_exp = nr_write // fsync

# The actual number of DDIR_SYNC issued might miss one DDIR_SYNC command
# when the last command issued was DDIR_WRITE command.
if not ((nr_sync == nr_sync_exp) or (nr_sync + 1 == nr_sync_exp)):
logging.error(f"nr_write={nr_write}, nr_sync={nr_sync}, fsync={fsync}")
self.passed = False


TEST_LIST = [
{
"test_id": 1,
Expand Down Expand Up @@ -255,6 +302,50 @@ def check_result(self):
},
"test_class": PassThruTest,
},
{
"test_id": 16,
"fio_opts": {
"rw": 'read',
"bs": 4096,
"number_ios": 10,
"fsync": 1,
"output-format": "json",
},
"test_class": FlushTest,
},
{
"test_id": 17,
"fio_opts": {
"rw": 'write',
"bs": 4096,
"number_ios": 10,
"fsync": 1,
"output-format": "json",
},
"test_class": FlushTest,
},
{
"test_id": 18,
"fio_opts": {
"rw": 'readwrite',
"bs": 4096,
"number_ios": 10,
"fsync": 1,
"output-format": "json",
},
"test_class": FlushTest,
},
{
"test_id": 19,
"fio_opts": {
"rw": 'trimwrite',
"bs": 4096,
"number_ios": 10,
"fsync": 1,
"output-format": "json",
},
"test_class": FlushTest,
},
]

def parse_args():
Expand Down