We definitely do need separate control between read and write fua, and only the read and write scsi commands actually support a fua bit. Also, I don't think the sg engine can actually be considered general purpose, as it really only supports the read/write/sync operations currently. I also think overloading the meaning of the "direct" option would set a bad precedent - even though it's not used by the sg engine. The "direct" bit changes behavior of the kernel or driver, but fua bit is a device operation. Thanks Kris Davis Western Digital Coporation Email: kris.davis@xxxxxxx Office:: +1-507-322-2376 -----Original Message----- From: Sitsofe Wheeler [mailto:sitsofe@xxxxxxxxx] Sent: Thursday, March 15, 2018 1:36 AM To: Kris Davis <Kris.Davis@xxxxxxx> Cc: fio@xxxxxxxxxxxxxxx; Jens Axboe <axboe@xxxxxxxxx> Subject: Re: Patch to add read and write fua bit options to the SG engine Hi, On 15 March 2018 at 03:59, Kris Davis <Kris.Davis@xxxxxxx> wrote: > I like to submit the patch below to add read and write fua bit options to the SG engine. It adds a readfua=bool, and writefua=bool options in the SG engine, which causes the "force unit access" bits to be set in the read and write operations respectively. I patterned the code changes from what I found in the libaio engine option code. So, I also added a "__FIO_OPT_G_SG" enum to optgroup.h, which seemed to follow the pattern - but I really don't understand where or how the enum settings values in the options are used, so I'm unsure. If you don't need different I/Os to have different FUA states (if they were ever supported in the sg ioengine could trims via unmaps/write_sames be FUA'd?) could you overload direct=1 for this purpose? -- Sitsofe | http://sucs.org/~sits/ On 15 March 2018 at 03:59, Kris Davis <Kris.Davis@xxxxxxx> wrote: > I like to submit the patch below to add read and write fua bit options to the SG engine. It adds a readfua=bool, and writefua=bool options in the SG engine, which causes the "force unit access" bits to be set in the read and write operations respectively. I patterned the code changes from what I found in the libaio engine option code. So, I also added a "__FIO_OPT_G_SG" enum to optgroup.h, which seemed to follow the pattern - but I really don't understand where or how the enum settings values in the options are used, so I'm unsure. > Thanks > Kris Davis > > > > diff --git a/HOWTO b/HOWTO > index acb9e97..4f9d26a 100644 > --- a/HOWTO > +++ b/HOWTO > @@ -1747,6 +1747,7 @@ > :manpage:`read(2)` and :manpage:`write(2)` for asynchronous > I/O. Requires :option:`filename` option to specify either block or > character devices. > + The sg engine includes engine specific options. > > **null** > Doesn't transfer any data, just pretends to. > This is mainly used to @@ -2068,6 +2069,17 @@ > multiple paths exist between the client and the server or in certain loopback > configurations. > > +.. option:: readfua=bool : [sg] > + > + With readfua option set to 1, read operations include the > + the force unit access (fua) flag. > + > +.. option:: writefua=bool : [sg] > + > + With writefua option set to 1, write operations include the > + the force unit access (fua) flag. > + > + > I/O depth > ~~~~~~~~~ > > diff --git a/engines/sg.c b/engines/sg.c index 4540b57..f240755 100644 > --- a/engines/sg.c > +++ b/engines/sg.c > @@ -12,8 +12,42 @@ > #include <sys/poll.h> > > #include "../fio.h" > +#include "../optgroup.h" > > #ifdef FIO_HAVE_SGIO > + > + > +struct sg_options { > + void *pad; > + unsigned int readfua; > + unsigned int writefua; > +}; > + > +static struct fio_option options[] = { > + { > + .name = "readfua", > + .lname = "sg engine read fua flag support", > + .type = FIO_OPT_BOOL, > + .off1 = offsetof(struct sg_options, readfua), > + .help = "Set FUA flag (force unit access) for all Read operations", > + .def = "0", > + .category = FIO_OPT_C_ENGINE, > + .group = FIO_OPT_G_SG, > + }, > + { > + .name = "writefua", > + .lname = "sg engine write fua flag support", > + .type = FIO_OPT_BOOL, > + .off1 = offsetof(struct sg_options, writefua), > + .help = "Set FUA flag (force unit access) for all Write operations", > + .def = "0", > + .category = FIO_OPT_C_ENGINE, > + .group = FIO_OPT_G_SG, > + }, > + { > + .name = NULL, > + }, > +}; > > #define MAX_10B_LBA 0xFFFFFFFFULL > #define SCSI_TIMEOUT_MS 30000 // 30 second timeout; currently no method to override > @@ -267,6 +301,7 @@ > static int fio_sgio_prep(struct thread_data *td, struct io_u *io_u) > { > struct sg_io_hdr *hdr = &io_u->hdr; > + struct sg_options *o = td->eo; > struct sgio_data *sd = td->io_ops_data; > long long nr_blocks, lba; > > @@ -286,6 +321,10 @@ > hdr->cmdp[0] = 0x28; // read(10) > else > hdr->cmdp[0] = 0x88; // read(16) > + > + if (o->readfua) > + hdr->cmdp[1] |= 0x08; > + > } else if (io_u->ddir == DDIR_WRITE) { > sgio_hdr_init(sd, hdr, io_u, 1); > > @@ -294,6 +333,10 @@ > hdr->cmdp[0] = 0x2a; // write(10) > else > hdr->cmdp[0] = 0x8a; // write(16) > + > + if (o->writefua) > + hdr->cmdp[1] |= 0x08; > + > } else { > sgio_hdr_init(sd, hdr, io_u, 0); > hdr->dxfer_direction = SG_DXFER_NONE; @@ -822,6 +865,8 > @@ > .close_file = generic_close_file, > .get_file_size = fio_sgio_get_file_size, > .flags = FIO_SYNCIO | FIO_RAWIO, > + .options = options, > + .option_struct_size = sizeof(struct sg_options) > }; > > #else /* FIO_HAVE_SGIO */ > diff --git a/optgroup.h b/optgroup.h > index 815ac16..d5e968d 100644 > --- a/optgroup.h > +++ b/optgroup.h > @@ -55,10 +55,11 @@ > __FIO_OPT_G_LIBAIO, > __FIO_OPT_G_ACT, > __FIO_OPT_G_LATPROF, > - __FIO_OPT_G_RBD, > - __FIO_OPT_G_GFAPI, > - __FIO_OPT_G_MTD, > + __FIO_OPT_G_RBD, > + __FIO_OPT_G_GFAPI, > + __FIO_OPT_G_MTD, > __FIO_OPT_G_HDFS, > + __FIO_OPT_G_SG, > __FIO_OPT_G_NR, > > FIO_OPT_G_RATE = (1ULL << __FIO_OPT_G_RATE), > @@ -93,6 +94,7 @@ > FIO_OPT_G_GFAPI = (1ULL << __FIO_OPT_G_GFAPI), > FIO_OPT_G_MTD = (1ULL << __FIO_OPT_G_MTD), > FIO_OPT_G_HDFS = (1ULL << __FIO_OPT_G_HDFS), > + FIO_OPT_G_SG = (1ULL << __FIO_OPT_G_SG), > FIO_OPT_G_INVALID = (1ULL << __FIO_OPT_G_NR), > }; > > > -- > To unsubscribe from this list: send the line "unsubscribe fio" in the > body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at > http://vger.kernel.org/majordomo-info.html -- Sitsofe | http://sucs.org/~sits/ ��.n��������+%������w��{.n�������^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
