From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
To: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@ozlabs.org,
Takuya Yoshikawa <takuya.yoshikawa@gmail.com>,
Alexander Graf <agraf@suse.de>,
kvm-ppc@vger.kernel.org, KVM list <kvm@vger.kernel.org>
Subject: Re: [PATCH 4/5] KVM: PPC: Book3s HV: Implement get_dirty_log using hardware changed bit
Date: Mon, 26 Dec 2011 14:05:20 +0900 [thread overview]
Message-ID: <4EF80090.7050008@oss.ntt.co.jp> (raw)
In-Reply-To: <20111225233524.GA5348@sammy.paulus.ozlabs.org>
(2011/12/26 8:35), Paul Mackerras wrote:
> On Fri, Dec 23, 2011 at 02:23:30PM +0100, Alexander Graf wrote:
>
>> So if I read things correctly, this is the only case you're setting
>> pages as dirty. What if you have the following:
>>
>> guest adds HTAB entry x
>> guest writes to page mapped by x
>> guest removes HTAB entry x
>> host fetches dirty log
>
> In that case the dirtiness is preserved in the setting of the
> KVMPPC_RMAP_CHANGED bit in the rmap entry. kvm_test_clear_dirty()
> returns 1 if that bit is set (and clears it). Using the rmap entry
> for this is convenient because (a) we also use it for saving the
> referenced bit when a HTAB entry is removed, and we can transfer both
> R and C over in one operation; (b) we need to be able to save away the
> C bit in real mode, and we already need to get the real-mode address
> of the rmap entry -- if we wanted to save it in a dirty bitmap we'd
> have to do an extra translation to get the real-mode address of the
> dirty bitmap word; (c) to avoid SMP races, if we were asynchronously
> setting bits in the dirty bitmap we'd have to do the double-buffering
> thing that x86 does, which seems more complicated than using the rmap
> entry (which we already have a lock bit for).
From my x86 dirty logging experience I have some concern about your code:
your code looks slow even when there is no/few dirty pages in the slot.
+ for (i = 0; i < memslot->npages; ++i) {
+ if (kvm_test_clear_dirty(kvm, rmapp))
+ __set_bit_le(i, map);
+ ++rmapp;
+ }
The check is being done for each page and this can be very expensive because
the number of pages is not small.
When we scan the dirty_bitmap 64 pages are checked at once and
the problem is not so significant.
Though I do not know well what kvm-ppc's dirty logging is aiming at, I guess
reporting cleanliness without noticeable delay to the user-space is important.
E.g. for VGA most of the cases are clean. For live migration, the
chance of seeing complete clean slot is small but almost all cases
are sparse.
>
>> PS: Always CC kvm@vger for stuff that other might want to review
>> (basically all patches)
(Though I sometimes check kvm-ppc on the archives,)
GET_DIRTY_LOG thing will be welcome.
Takuya
>
> So why do we have a separate kvm-ppc list then? :)
next prev parent reply other threads:[~2011-12-26 5:28 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-15 12:00 [PATCH 0/5] Make use of hardware reference and change bits in HPT Paul Mackerras
2011-12-15 12:01 ` [PATCH 1/5] KVM: PPC: Book3S HV: Keep HPTE locked when invalidating Paul Mackerras
2011-12-15 12:02 ` [PATCH 2/5] KVM: PPC: Book3s HV: Maintain separate guest and host views of R and C bits Paul Mackerras
2011-12-15 12:02 ` [PATCH 3/5] KVM: PPC: Book3S HV: Use the hardware referenced bit for kvm_age_hva Paul Mackerras
2011-12-15 12:03 ` [PATCH 4/5] KVM: PPC: Book3s HV: Implement get_dirty_log using hardware changed bit Paul Mackerras
2011-12-23 13:23 ` Alexander Graf
2011-12-25 23:35 ` Paul Mackerras
2011-12-26 5:05 ` Takuya Yoshikawa [this message]
2011-12-31 0:44 ` Paul Mackerras
2011-12-15 12:04 ` [PATCH 5/5] KVM: PPC: Book3s HV: Implement H_CLEAR_REF and H_CLEAR_MOD hcalls Paul Mackerras
2011-12-23 13:26 ` Alexander Graf
2011-12-23 13:36 ` [PATCH 0/5] Make use of hardware reference and change bits in HPT Alexander Graf
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=4EF80090.7050008@oss.ntt.co.jp \
--to=yoshikawa.takuya@oss.ntt.co.jp \
--cc=agraf@suse.de \
--cc=kvm-ppc@vger.kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=paulus@samba.org \
--cc=takuya.yoshikawa@gmail.com \
/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).