On 16/04/2015 14:18, Peter Lieven wrote:
> SCSI allowes to tell the target to not return from a write command
> if the date is not written to the disk. Use this so called FUA
> bit if it is supported to optimize WRITE commands if writeback is
> not allowed.
>
> In this case qemu always issues a WRITE followed by a FLUSH. This
> is 2 round trip times. If we set the FUA bit we can ignore the
> following FLUSH.
>
> Signed-off-by: Peter Lieven <p...@kamp.de>
> ---
> block/iscsi.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 237faa1..7fb04d7 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -68,6 +68,7 @@ typedef struct IscsiLun {
> bool lbprz;
> bool dpofua;
> bool has_write_same;
> + bool force_next_flush;
> } IscsiLun;
>
> typedef struct IscsiTask {
> @@ -370,6 +371,7 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState
> *bs,
> struct IscsiTask iTask;
> uint64_t lba;
> uint32_t num_sectors;
> + int fua = iscsilun->dpofua && !bs->enable_write_cache;
>
> if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
> return -EINVAL;
> @@ -388,12 +390,12 @@ retry:
> if (iscsilun->use_16_for_rw) {
> iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba,
> NULL, num_sectors *
> iscsilun->block_size,
> - iscsilun->block_size, 0, 0, 0, 0, 0,
> + iscsilun->block_size, 0, 0, fua, 0,
> 0,
> iscsi_co_generic_cb, &iTask);
> } else {
> iTask.task = iscsi_write10_task(iscsilun->iscsi, iscsilun->lun, lba,
> NULL, num_sectors *
> iscsilun->block_size,
> - iscsilun->block_size, 0, 0, 0, 0, 0,
> + iscsilun->block_size, 0, 0, fua, 0,
> 0,
> iscsi_co_generic_cb, &iTask);
> }
> if (iTask.task == NULL) {
> @@ -621,6 +623,11 @@ static int coroutine_fn iscsi_co_flush(BlockDriverState
> *bs)
> return 0;
> }
>
> + if (iscsilun->dpofua && !bs->enable_write_cache &&
> + !iscsilun->force_next_flush) {
> + return 0;
> + }
> +
> iscsi_co_init_iscsitask(iscsilun, &iTask);
>
> retry:
> @@ -648,6 +655,8 @@ retry:
> return -EIO;
> }
>
> + iscsilun->force_next_flush = false;
You still need a flush if you do WRITE SAME, WRITE+FUA, WRITE+FUA.
Also, since bs->enable_write_cache can be toggled arbitrarily, I would
prefer to set force_next_flush on all non-FUA writes, including those
done with bs->enable_write_cache.
> return 0;
> }
>
> @@ -969,6 +978,8 @@ retry:
> iscsi_allocationmap_set(iscsilun, sector_num, nb_sectors);
> }
>
> + iscsilun->force_next_flush = true;
Also, I think it is iscsi_co_generic_cb that should set
force_next_flush, so that it is only set on failure. Not really for the
optimization value, but because it's clearer.
I think that if you do these changes, iscsi_co_flush can just check "if
(!iscsilun->force_next_flush)".
But still---nice approach. :)
> return 0;
> }
>
>
