Am 04.03.2025 um 16:52 hat Alberto Faria geschrieben:
> Avoid emulating FUA when the driver supports it natively. This should
> provide better performance than a full flush after the write.
>
> Signed-off-by: Alberto Faria <afa...@redhat.com>
Did you try out if you can see performance improvements in practice?
It's always nice to have numbers in the commit message for patches that
promise performance improvements.
> hw/scsi/scsi-disk.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 8cf50845ab..ce48e20ee6 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -43,6 +43,7 @@
> #include "qemu/cutils.h"
> #include "trace.h"
> #include "qom/object.h"
> +#include "block/block_int-common.h"
>
> #ifdef __linux
> #include <scsi/sg.h>
> @@ -75,7 +76,7 @@ struct SCSIDiskClass {
> */
> DMAIOFunc *dma_readv;
> DMAIOFunc *dma_writev;
> - bool (*need_fua_emulation)(SCSICommand *cmd);
> + bool (*need_fua)(SCSICommand *cmd);
> void (*update_sense)(SCSIRequest *r);
> };
>
> @@ -86,6 +87,7 @@ typedef struct SCSIDiskReq {
> uint32_t sector_count;
> uint32_t buflen;
> bool started;
> + bool need_fua;
> bool need_fua_emulation;
> struct iovec iov;
> QEMUIOVector qiov;
> @@ -553,7 +555,7 @@ static void scsi_read_data(SCSIRequest *req)
>
> first = !r->started;
> r->started = true;
> - if (first && r->need_fua_emulation) {
> + if (first && r->need_fua) {
> block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct, 0,
> BLOCK_ACCT_FLUSH);
> r->req.aiocb = blk_aio_flush(s->qdev.conf.blk, scsi_do_read_cb, r);
> @@ -2384,7 +2386,9 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req,
> uint8_t *buf)
> scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
> return 0;
> }
> - r->need_fua_emulation = sdc->need_fua_emulation(&r->req.cmd);
> + r->need_fua = sdc->need_fua(&r->req.cmd);
> + r->need_fua_emulation = r->need_fua &&
> + (blk_bs(s->qdev.conf.blk)->supported_write_flags & BDRV_REQ_FUA) ==
> 0;
You can just use BDRV_REQ_FUA unconditionally. If the driver doesn't
support it directly, the block layer already emulates it internally. We
don't have to duplicate this here. If scsi_write_data() does a flush
directly for VERIFY (like scsi_read_data() already does),
scsi_write_do_fua() can go away completely.
However, we can only apply this to write requests. We still need to know
that FUA needs to be emulated for reads. scsi_read_data() issues a flush
for FUA requests and your patch would break it if writes support
BDRV_REQ_FUA.
Kevin
