Re: [PATCH] fio: add device dax engine

[Ross Zwisler <ross.zwisler@linux.intel.com>]

On Tue, Dec 06, 2016 at 05:05:43PM -0700, Dave Jiang wrote:
> Adding an ioengine that access DAX device directly. Very similar to the
> mmap engine. Uses the libpmem's pmem_memcpy_persist() call for writes.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

With just 2 tiny nits:

Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>

> +static int fio_devdax_file(struct thread_data *td, struct fio_file *f,
> +			   size_t length, off_t off)
> +{
> +	struct fio_devdax_data *fdd = FILE_ENG_DATA(f);
> +	int flags = 0;
> +
> +	if (td_rw(td))
> +		flags = PROT_READ | PROT_WRITE;
> +	else if (td_write(td)) {
> +		flags = PROT_WRITE;
> +
> +		if (td->o.verify != VERIFY_NONE)
> +			flags |= PROT_READ;
> +	} else
> +		flags = PROT_READ;
> +
> +	fdd->devdax_ptr = mmap(NULL, length, flags, MAP_SHARED, f->fd, off);
> +	if (fdd->devdax_ptr == MAP_FAILED) {
> +		fdd->devdax_ptr = NULL;
> +		td_verror(td, errno, "mmap");
> +		goto err;
> +	}
> +
> +err:

We don't need the 'err' label or the associated goto.  These are needed in the
mmap.c engine, but we can just fall through since we don't have the
posix_madvise() work.

> +	if (td->error && fdd->devdax_ptr)
> +		munmap(fdd->devdax_ptr, length);
> +
> +	return td->error;
> +}
> +
> +/*
> + * Just mmap an appropriate portion, we cannot mmap the full extent
> + */
> +static int fio_devdax_prep_limited(struct thread_data *td, struct io_u *io_u)
> +{
> +	struct fio_file *f = io_u->file;
> +	struct fio_devdax_data *fdd = FILE_ENG_DATA(f);
> +
> +	if (io_u->buflen > f->real_file_size) {
> +		log_err("fio: bs too big for mmap engine\n");
					     dev-dax
> +		return EIO;
> +	}