From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takuya Yoshikawa Date: Mon, 26 Dec 2011 05:05:20 +0000 Subject: Re: [PATCH 4/5] KVM: PPC: Book3s HV: Implement get_dirty_log using Message-Id: <4EF80090.7050008@oss.ntt.co.jp> List-Id: References: <20111215120018.GA20629@bloggs.ozlabs.ibm.com> <20111215120322.GE20629@bloggs.ozlabs.ibm.com> <20111225233524.GA5348@sammy.paulus.ozlabs.org> In-Reply-To: <20111225233524.GA5348@sammy.paulus.ozlabs.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Paul Mackerras Cc: Alexander Graf , kvm-ppc@vger.kernel.org, linuxppc-dev@ozlabs.org, KVM list , Takuya Yoshikawa (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? :) From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from serv2.oss.ntt.co.jp (serv2.oss.ntt.co.jp [222.151.198.100]) by ozlabs.org (Postfix) with ESMTP id 5EF92B6FBC for ; Mon, 26 Dec 2011 16:28:23 +1100 (EST) Message-ID: <4EF80090.7050008@oss.ntt.co.jp> Date: Mon, 26 Dec 2011 14:05:20 +0900 From: Takuya Yoshikawa MIME-Version: 1.0 To: Paul Mackerras Subject: Re: [PATCH 4/5] KVM: PPC: Book3s HV: Implement get_dirty_log using hardware changed bit References: <20111215120018.GA20629@bloggs.ozlabs.ibm.com> <20111215120322.GE20629@bloggs.ozlabs.ibm.com> <20111225233524.GA5348@sammy.paulus.ozlabs.org> In-Reply-To: <20111225233524.GA5348@sammy.paulus.ozlabs.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Cc: linuxppc-dev@ozlabs.org, Takuya Yoshikawa , Alexander Graf , kvm-ppc@vger.kernel.org, KVM list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , (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? :) From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takuya Yoshikawa 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 Message-ID: <4EF80090.7050008@oss.ntt.co.jp> References: <20111215120018.GA20629@bloggs.ozlabs.ibm.com> <20111215120322.GE20629@bloggs.ozlabs.ibm.com> <20111225233524.GA5348@sammy.paulus.ozlabs.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Alexander Graf , kvm-ppc@vger.kernel.org, linuxppc-dev@ozlabs.org, KVM list , Takuya Yoshikawa To: Paul Mackerras Return-path: Received: from serv2.oss.ntt.co.jp ([222.151.198.100]:41953 "EHLO serv2.oss.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750699Ab1LZFEN (ORCPT ); Mon, 26 Dec 2011 00:04:13 -0500 In-Reply-To: <20111225233524.GA5348@sammy.paulus.ozlabs.org> Sender: kvm-owner@vger.kernel.org List-ID: (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? :)