Skip to content

nvme: add dry-run NVME_ARGS default option #2751

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

Closed
ikegami-t wants to merge 2 commits into from
Closed

nvme: add dry-run NVME_ARGS default option #2751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1882cd
nvme: add dry-run NVME_ARGS default option
ikegami-t Mar 27, 2025
e1667bd
nvme: move submit_io show-command and dry-run outputs into logging
ikegami-t Apr 13, 2025
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
2 changes: 1 addition & 1 deletion nvme-print.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#define nvme_print(name, flags, ...) \
do { \
struct print_ops *ops = nvme_print_ops(flags); \
if (ops && ops->name) \
if (ops && ops->name && !nvme_cfg.dry_run) \
ops->name(__VA_ARGS__); \
} while (false)

Expand Down
50 changes: 10 additions & 40 deletions nvme.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ struct passthru_config {
char *metadata;
bool raw_binary;
bool show_command;
bool dry_run;
bool read;
bool write;
__u8 prefill;
Expand Down Expand Up @@ -192,6 +191,7 @@ const char *output_format = "Output format: normal|binary";
#endif /* CONFIG_JSONC */
const char *timeout = "timeout value, in milliseconds";
const char *verbose = "Increase output verbosity";
const char *dry_run = "show command instead of sending";

static const char *app_tag = "app tag for end-to-end PI";
static const char *app_tag_mask = "app tag mask for end-to-end PI";
Expand All @@ -201,7 +201,6 @@ static const char *csi = "command set identifier";
static const char *buf_len = "buffer len (if) data is sent or received";
static const char *domainid = "Domain Identifier";
static const char *doper = "directive operation";
static const char *dry = "show command instead of sending";
static const char *dspec_w_dtype = "directive specification associated with directive type";
static const char *dtype = "directive type";
static const char *endgid = "Endurance Group Identifier (ENDGID)";
Expand Down Expand Up @@ -445,6 +444,8 @@ static int parse_args(int argc, char *argv[], const char *desc,
log_level = map_log_level(nvme_cfg.verbose, false);
nvme_init_default_logging(stderr, log_level, false, false);

set_dry_run(nvme_cfg.dry_run);

return 0;
}

Expand Down Expand Up @@ -8008,15 +8009,13 @@ unsigned long long elapsed_utime(struct timeval start_time,

static int submit_io(int opcode, char *command, const char *desc, int argc, char **argv)
{
struct timeval start_time, end_time;
void *buffer;
_cleanup_free_ void *mbuffer = NULL;
int err = 0;
_cleanup_fd_ int dfd = -1, mfd = -1;
int flags, pi_size;
int mode = 0644;
__u16 control = 0, nblocks = 0;
__u32 dsmgmt = 0;
unsigned int logical_block_size = 0;
unsigned long long buffer_size = 0, mbuffer_size = 0;
_cleanup_huge_ struct nvme_mem_huge mh = { 0, };
Expand Down Expand Up @@ -8060,7 +8059,6 @@ static int submit_io(int opcode, char *command, const char *desc, int argc, char
__u16 dspec;
__u8 dsmgmt;
bool show;
bool dry_run;
bool latency;
bool force;
};
Expand All @@ -8085,7 +8083,6 @@ static int submit_io(int opcode, char *command, const char *desc, int argc, char
.dspec = 0,
.dsmgmt = 0,
.show = false,
.dry_run = false,
.latency = false,
.force = false,
};
Expand All @@ -8110,7 +8107,7 @@ static int submit_io(int opcode, char *command, const char *desc, int argc, char
OPT_SHRT("dir-spec", 'S', &cfg.dspec, dspec),
OPT_BYTE("dsm", 'D', &cfg.dsmgmt, dsm),
OPT_FLAG("show-command", 'V', &cfg.show, show),
OPT_FLAG("dry-run", 'w', &cfg.dry_run, dry),
Copy link
Copy Markdown
Collaborator

@igaw igaw Apr 2, 2025

Choose a reason for hiding this comment

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

I think we should keep the -w short hand here, to stay compatible.

Copy link
Copy Markdown
Contributor Author

@ikegami-t ikegami-t Apr 2, 2025

Choose a reason for hiding this comment

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

Fixed as mentioned.

OPT_FLAG("dry-run", 'w', &nvme_cfg.dry_run, dry_run),
OPT_FLAG("latency", 't', &cfg.latency, latency),
OPT_FLAG("force", 0, &cfg.force, force));

Expand Down Expand Up @@ -8148,7 +8145,6 @@ static int submit_io(int opcode, char *command, const char *desc, int argc, char
if (cfg.prinfo > 0xf)
return err;

dsmgmt = cfg.dsmgmt;
control |= (cfg.prinfo << 10);
if (cfg.limited_retry)
control |= NVME_IO_LR;
Expand All @@ -8162,7 +8158,6 @@ static int submit_io(int opcode, char *command, const char *desc, int argc, char
return -EINVAL;
}
control |= cfg.dtype << 4;
dsmgmt |= ((__u32)cfg.dspec) << 16;
}

if (opcode & 1) {
Expand Down Expand Up @@ -8286,27 +8281,6 @@ static int submit_io(int opcode, char *command, const char *desc, int argc, char
}
}

if (cfg.show || cfg.dry_run) {
printf("opcode : %02x\n", opcode);
printf("nsid : %02x\n", cfg.namespace_id);
printf("flags : %02x\n", 0);
printf("control : %04x\n", control);
printf("nblocks : %04x\n", nblocks);
printf("metadata : %"PRIx64"\n", (uint64_t)(uintptr_t)mbuffer);
printf("addr : %"PRIx64"\n", (uint64_t)(uintptr_t)buffer);
printf("slba : %"PRIx64"\n", (uint64_t)cfg.start_block);
printf("dsmgmt : %08x\n", dsmgmt);
printf("reftag : %"PRIx64"\n", (uint64_t)cfg.ref_tag);
printf("apptag : %04x\n", cfg.app_tag);
printf("appmask : %04x\n", cfg.app_tag_mask);
printf("storagetagcheck : %04x\n", cfg.storage_tag_check);
printf("storagetag : %"PRIx64"\n", (uint64_t)cfg.storage_tag);
printf("pif : %02x\n", pif);
printf("sts : %02x\n", sts);
}
if (cfg.dry_run)
return 0;

struct nvme_io_args args = {
.args_size = sizeof(args),
.fd = dev_fd(dev),
Expand All @@ -8330,16 +8304,13 @@ static int submit_io(int opcode, char *command, const char *desc, int argc, char
.timeout = nvme_cfg.timeout,
.result = NULL,
};
gettimeofday(&start_time, NULL);
err = nvme_io(&args, opcode);
gettimeofday(&end_time, NULL);
if (cfg.latency)
printf(" latency: %s: %llu us\n", command, elapsed_utime(start_time, end_time));

err = nvme_submit_io(&args, opcode, cfg.show, cfg.latency);
if (err < 0) {
nvme_show_error("submit-io: %s", nvme_strerror(errno));
} else if (err) {
nvme_show_status(err);
} else {
} else if (!nvme_cfg.dry_run) {
if (!(opcode & 1) && write(dfd, (void *)buffer, buffer_size) < 0) {
nvme_show_error("write: %s: failed to write buffer to output file",
strerror(errno));
Expand Down Expand Up @@ -9069,7 +9040,6 @@ static int passthru(int argc, char **argv, bool admin,
.metadata = "",
.raw_binary = false,
.show_command = false,
.dry_run = false,
.read = false,
.write = false,
.latency = false,
Expand All @@ -9095,7 +9065,7 @@ static int passthru(int argc, char **argv, bool admin,
OPT_FILE("metadata", 'M', &cfg.metadata, metadata),
OPT_FLAG("raw-binary", 'b', &cfg.raw_binary, raw_dump),
OPT_FLAG("show-command", 's', &cfg.show_command, show),
OPT_FLAG("dry-run", 'd', &cfg.dry_run, dry),
OPT_FLAG("dry-run", 'd', &nvme_cfg.dry_run, dry_run),
OPT_FLAG("read", 'r', &cfg.read, re),
OPT_FLAG("write", 'w', &cfg.write, wr),
OPT_FLAG("latency", 'T', &cfg.latency, latency));
Expand Down Expand Up @@ -9172,7 +9142,7 @@ static int passthru(int argc, char **argv, bool admin,
}
}

if (cfg.show_command || cfg.dry_run) {
if (cfg.show_command || nvme_cfg.dry_run) {
printf("opcode : %02x\n", cfg.opcode);
printf("flags : %02x\n", cfg.flags);
printf("rsvd1 : %04x\n", cfg.rsvd);
Expand All @@ -9191,7 +9161,7 @@ static int passthru(int argc, char **argv, bool admin,
printf("cdw15 : %08x\n", cfg.cdw15);
printf("timeout_ms : %08x\n", nvme_cfg.timeout);
}
if (cfg.dry_run)
if (nvme_cfg.dry_run)
return 0;

gettimeofday(&start_time, NULL);
Expand Down
3 changes: 3 additions & 0 deletions nvme.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ struct nvme_config {
char *output_format;
int verbose;
__u32 timeout;
bool dry_run;
};

/*
Expand All @@ -90,6 +91,7 @@ struct nvme_config {
OPT_FMT("output-format", 'o', &nvme_cfg.output_format, output_format), \
##__VA_ARGS__, \
OPT_UINT("timeout", 't', &nvme_cfg.timeout, timeout), \
OPT_FLAG("dry-run", 0, &nvme_cfg.dry_run, dry_run), \
OPT_END() \
}

Expand Down Expand Up @@ -131,6 +133,7 @@ static inline DEFINE_CLEANUP_FUNC(
extern const char *output_format;
extern const char *timeout;
extern const char *verbose;
extern const char *dry_run;
extern struct nvme_config nvme_cfg;

int validate_output_format(const char *format, nvme_print_flags_t *flags);
Expand Down
67 changes: 65 additions & 2 deletions util/logging.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "logging.h"

int log_level;
static bool dry_run;

int map_log_level(int verbose, bool quiet)
{
Expand Down Expand Up @@ -44,6 +45,11 @@ int map_log_level(int verbose, bool quiet)
return log_level;
}

void set_dry_run(bool enable)
{
dry_run = enable;
}

static void nvme_show_common(struct nvme_passthru_cmd *cmd)
{
printf("opcode : %02x\n", cmd->opcode);
Expand Down Expand Up @@ -93,6 +99,11 @@ int nvme_submit_passthru(int fd, unsigned long ioctl_cmd,
struct timeval end;
int err;

if (dry_run) {
nvme_show_common(cmd);
return 0;
}

if (log_level >= LOG_DEBUG)
gettimeofday(&start, NULL);

Expand All @@ -116,12 +127,16 @@ int nvme_submit_passthru64(int fd, unsigned long ioctl_cmd,
{
struct timeval start;
struct timeval end;
int err;
int err = 0;

if (dry_run) {
nvme_show_common((struct nvme_passthru_cmd *)cmd);
return 0;
}

if (log_level >= LOG_DEBUG)
gettimeofday(&start, NULL);


err = ioctl(fd, ioctl_cmd, cmd);

if (log_level >= LOG_DEBUG) {
Expand All @@ -135,3 +150,51 @@ int nvme_submit_passthru64(int fd, unsigned long ioctl_cmd,

return err;
}

static void nvme_show_io_command(struct nvme_io_args *args, __u8 opcode)
{
__u32 dsmgmt = args->dsm;
__u8 dtype = (args->control >> 4) & 0xf;

if (dtype)
dsmgmt |= ((__u32)args->dspec) << 16;

printf("opcode : %02x\n", opcode);
printf("nsid : %02x\n", args->nsid);
printf("flags : %02x\n", 0);
printf("control : %04x\n", args->control);
printf("nblocks : %04x\n", args->nlb);
printf("metadata : %"PRIx64"\n", (uint64_t)(uintptr_t)args->metadata);
printf("addr : %"PRIx64"\n", (uint64_t)(uintptr_t)args->data);
printf("slba : %"PRIx64"\n", (uint64_t)(uintptr_t)args->slba);
printf("dsmgmt : %08x\n", dsmgmt);
printf("reftag : %"PRIx64"\n", (uint64_t)args->reftag_u64);
printf("apptag : %04x\n", args->apptag);
printf("appmask : %04x\n", args->appmask);
printf("storagetagcheck : %04x\n", args->control & NVME_IO_STC ? true : false);
printf("storagetag : %"PRIx64"\n", (uint64_t)args->storage_tag);
printf("pif : %02x\n", args->pif);
printf("sts : %02x\n", args->sts);
}

int nvme_submit_io(struct nvme_io_args *args, __u8 opcode, bool show, bool latency)
Copy link
Copy Markdown
Collaborator

@igaw igaw Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I suggest instead adding more indirect code for the logging, we could just look at the opcode in nvme_show_command, e.g.

static void nvme_show_command(struct nvme_passthru_cmd *cmd, int err)
{
	switch (cmd->opcode) {
		case nvme_cmd_read:
		case nvme_cmd_write:
		case nvme_cmd_compare:
			nvme_show_io_command(cmd);
			break;
		default:
			nvme_show_common(cmd);
	}
	printf("result       : %08x\n", cmd->result);
	printf("err          : %d\n", err);
}

{
struct timeval start_time;
struct timeval end_time;
int err;

if (show || dry_run)
nvme_show_io_command(args, opcode);

if (dry_run)
Copy link
Copy Markdown
Collaborator

@igaw igaw Apr 14, 2025

Choose a reason for hiding this comment

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

and then the dry_run maybe should be

int nvme_submit_passthru(int fd, unsigned long ioctl_cmd,
			 struct nvme_passthru_cmd *cmd, __u32 *result)
{
	struct timeval start;
	struct timeval end;
	int err = 0;

	if (log_level >= LOG_DEBUG)
		gettimeofday(&start, NULL);

	if (!dry_run)
		err = ioctl(fd, ioctl_cmd, cmd);

	if (log_level >= LOG_DEBUG) {
		gettimeofday(&end, NULL);
		nvme_show_command(cmd, err);
		nvme_show_latency(start, end);
	}

	if (err >= 0 && result)
		*result = cmd->result;

	return err;
}

return 0;

gettimeofday(&start_time, NULL);
err = nvme_io(args, opcode);
gettimeofday(&end_time, NULL);

if (latency)
nvme_show_latency(start_time, end_time);

return err;
}
3 changes: 2 additions & 1 deletion util/logging.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@
extern int log_level;

int map_log_level(int verbose, bool quiet);

void set_dry_run(bool enable);
int nvme_submit_io(struct nvme_io_args *args, __u8 opcode, bool show, bool latency);
#endif // DEBUG_H_