From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
xen-devel@lists.xenproject.org,
George Dunlap <george.dunlap@eu.citrix.com>,
qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] xen_disk: fix unmapping of persistent grants
Date: Thu, 13 Nov 2014 09:33:58 +0100 [thread overview]
Message-ID: <54646CF6.7090302@citrix.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1411121629320.26318@kaball.uk.xensource.com>
El 12/11/14 a les 18.41, Stefano Stabellini ha escrit:
> On Wed, 12 Nov 2014, Roger Pau Monne wrote:
>> This patch fixes two issues with persistent grants and the disk PV backend
>> (Qdisk):
>>
>> - Don't use batch mappings when using persistent grants, doing so prevents
>> unmapping single grants (the whole area has to be unmapped at once).
>
> The real issue is that destroy_grant cannot work with batch_maps.
> One could reimplement destroy_grant to build a single array with all the
> grants to unmap and make a single xc_gnttab_munmap call.
>
> Do you think that would be feasible?
Making destroy_grant work with batch maps using the current tree
structure is going to be quite complicated, because destroy_grant
iterates on every entry on the tree, and doesn't know which grants
belong to which regions.
IMHO a simpler solution would be to introduce another tree (or list)
that keeps track of grant-mapped regions, and on tear down use the data
in that list to unmap the regions. This way the current tree will still
be used to perform the grant_ref->vaddr translation, but on teardown the
newly introduced list would be used instead.
In general I was reluctant to do this because not using batch maps with
persistent grants should not introduce a noticeable performance
regression due to the fact that grants are only mapped once for the
whole life-cycle of the virtual disk. Also, if we plan to implement
indirect descriptors for Qdisk we really need to be able to unmap single
grants in order to purge the list, since in that case it's not possible
to keep all possible grants persistently mapped.
Since this alternate solution is easy to implement I will send a new
patch using this approach, then we can decide what to do.
Roger.
next prev parent reply other threads:[~2014-11-13 8:34 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-12 15:45 [Qemu-devel] [PATCH] xen_disk: fix unmapping of persistent grants Roger Pau Monne
2014-11-12 15:55 ` George Dunlap
2014-11-12 17:20 ` [Qemu-devel] [PATCH for-xen-4.5] " Konrad Rzeszutek Wilk
2014-11-12 17:41 ` [Qemu-devel] [PATCH] " Stefano Stabellini
2014-11-13 8:33 ` Roger Pau Monné [this message]
2014-11-13 11:42 ` Kevin Wolf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54646CF6.7090302@citrix.com \
--to=roger.pau@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=stefano.stabellini@eu.citrix.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).