[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature-persistent
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature-persistent if grant copy is not available |
Date: |
Tue, 20 Jun 2017 15:19:33 -0700 (PDT) |
User-agent: |
Alpine 2.10 (DEB 1266 2009-07-14) |
On Tue, 20 Jun 2017, Paul Durrant wrote:
> If grant copy is available then it will always be used in preference to
> persistent maps. In this case feature-persistent should not be advertized
> to the frontend, otherwise it may needlessly copy data into persistently
> granted buffers.
>
> Signed-off-by: Paul Durrant <address@hidden>
CC'ing Roger.
It is true that using feature-persistent together with grant copies is a
a very bad idea.
But this change enstablishes an explicit preference of
feature_grant_copy over feature-persistent in the xen_disk backend. It
is not obvious to me that it should be the case.
Why is feature_grant_copy (without feature-persistent) better than
feature-persistent (without feature_grant_copy)? Shouldn't we simply
avoid grant copies to copy data to persistent grants?
> ---
> Cc: Stefano Stabellini <address@hidden>
> Cc: Anthony Perard <address@hidden>
> Cc: Kevin Wolf <address@hidden>
> Cc: Max Reitz <address@hidden>
> ---
> hw/block/xen_disk.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 3a22805fbc..9b06e3aa81 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -1023,11 +1023,18 @@ static int blk_init(struct XenDevice *xendev)
>
> blkdev->file_blk = BLOCK_SIZE;
>
> + blkdev->feature_grant_copy =
> + (xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) ==
> 0);
> +
> + xen_pv_printf(&blkdev->xendev, 3, "grant copy operation %s\n",
> + blkdev->feature_grant_copy ? "enabled" : "disabled");
> +
> /* fill info
> * blk_connect supplies sector-size and sectors
> */
> xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
> - xenstore_write_be_int(&blkdev->xendev, "feature-persistent", 1);
> + xenstore_write_be_int(&blkdev->xendev, "feature-persistent",
> + !blkdev->feature_grant_copy);
> xenstore_write_be_int(&blkdev->xendev, "info", info);
>
> blk_parse_discard(blkdev);
> @@ -1202,12 +1209,6 @@ static int blk_connect(struct XenDevice *xendev)
>
> xen_be_bind_evtchn(&blkdev->xendev);
>
> - blkdev->feature_grant_copy =
> - (xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) ==
> 0);
> -
> - xen_pv_printf(&blkdev->xendev, 3, "grant copy operation %s\n",
> - blkdev->feature_grant_copy ? "enabled" : "disabled");
> -
> xen_pv_printf(&blkdev->xendev, 1, "ok: proto %s, ring-ref %d, "
> "remote port %d, local port %d\n",
> blkdev->xendev.protocol, blkdev->ring_ref,
> --
> 2.11.0
>
- [Qemu-devel] [PATCH 0/3] xen-disk: performance improvements, Paul Durrant, 2017/06/20
- [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature-persistent if grant copy is not available, Paul Durrant, 2017/06/20
- Re: [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature-persistent if grant copy is not available,
Stefano Stabellini <=
- Re: [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature-persistent if grant copy is not available, Roger Pau Monné, 2017/06/21
- Re: [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature-persistent if grant copy is not available, Paul Durrant, 2017/06/21
- Re: [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature-persistent if grant copy is not available, Paul Durrant, 2017/06/21
- Re: [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature-persistent if grant copy is not available, Roger Pau Monne, 2017/06/21
- Re: [Qemu-devel] [PATCH 1/3] xen-disk: only advertize feature-persistent if grant copy is not available, Paul Durrant, 2017/06/21
[Qemu-devel] [PATCH 3/3] xen-disk: use an IOThread per instance, Paul Durrant, 2017/06/20
[Qemu-devel] [PATCH 2/3] xen-disk: add support for multi-page shared rings, Paul Durrant, 2017/06/20