All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
To: Takuya Yoshikawa <takuya.yoshikawa@gmail.com>
Cc: Avi Kivity <avi@redhat.com>,
	mtosatti@redhat.com, kvm@vger.kernel.org,
	kvm-ppc@vger.kernel.org
Subject: Re: [PATCH 0/4] KVM: Decouple rmap_pde from lpage_info write_count
Date: Wed, 25 Jan 2012 06:20:15 +0000	[thread overview]
Message-ID: <4F1F9F1F.1030601@oss.ntt.co.jp> (raw)
In-Reply-To: <20120124233549.17cf98a619b05724cfab78ad@gmail.com>

(2012/01/24 23:35), Takuya Yoshikawa wrote:
> On Tue, 24 Jan 2012 13:24:56 +0200
> Avi Kivity<avi@redhat.com>  wrote:
>
>> On 01/23/2012 12:42 PM, Takuya Yoshikawa wrote:
>>> The last one is an RFC patch:
>>>
>>> I think it is better to refactor the rmap things, if needed, before
>>> other architectures than x86 starts large pages support.
>>>
>>
>> Not commenting about the meat of the patches (not sufficiently recovered
>> yet), but other architectures may not want the write_count stuff at all,
>> or even kvm_memory_slot::rmap.  It may make sense to move those members
>> into a new kvm_memory_slot::arch arch-specific substructure.
>
> It seems nice.
>
> We can also put double buffering related things into that arch structure!
> I will look again and take the new approach.
>

... and I found __kvm_set_memory_region() was a bit complicated to make
my work harder than necessary, especially:

	1. around "#ifndef CONFIG_S390" which allocates rmap, lpage_info,
	   dirty_bitmap

	2. kvm_free_physmem_slot(free, dont) stuff

1 will naturally disappear, eliminating the need of ugly #ifdef and
"(void)level;" warning killer, by introducing kvm_memory_slot::arch and
related allocation functions:

	kvm_arch_alloc/init_*()

But one thing I hesitate to do is introducing architecture specific version
of 2:

	kvm_arch_free_physmem_slot(free, dont)

@free, @dont technique is already a bit tricky and distributing this kind
of style to other files than kvm_main.c will make the logic harder to follow.

Though it may be a pain, doesn't it make sense to clean up this stuff?


	Takuya

WARNING: multiple messages have this Message-ID (diff)
From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
To: Takuya Yoshikawa <takuya.yoshikawa@gmail.com>
Cc: Avi Kivity <avi@redhat.com>,
	mtosatti@redhat.com, kvm@vger.kernel.org,
	kvm-ppc@vger.kernel.org
Subject: Re: [PATCH 0/4] KVM: Decouple rmap_pde from lpage_info write_count
Date: Wed, 25 Jan 2012 15:20:15 +0900	[thread overview]
Message-ID: <4F1F9F1F.1030601@oss.ntt.co.jp> (raw)
In-Reply-To: <20120124233549.17cf98a619b05724cfab78ad@gmail.com>

(2012/01/24 23:35), Takuya Yoshikawa wrote:
> On Tue, 24 Jan 2012 13:24:56 +0200
> Avi Kivity<avi@redhat.com>  wrote:
>
>> On 01/23/2012 12:42 PM, Takuya Yoshikawa wrote:
>>> The last one is an RFC patch:
>>>
>>> I think it is better to refactor the rmap things, if needed, before
>>> other architectures than x86 starts large pages support.
>>>
>>
>> Not commenting about the meat of the patches (not sufficiently recovered
>> yet), but other architectures may not want the write_count stuff at all,
>> or even kvm_memory_slot::rmap.  It may make sense to move those members
>> into a new kvm_memory_slot::arch arch-specific substructure.
>
> It seems nice.
>
> We can also put double buffering related things into that arch structure!
> I will look again and take the new approach.
>

... and I found __kvm_set_memory_region() was a bit complicated to make
my work harder than necessary, especially:

	1. around "#ifndef CONFIG_S390" which allocates rmap, lpage_info,
	   dirty_bitmap

	2. kvm_free_physmem_slot(free, dont) stuff

1 will naturally disappear, eliminating the need of ugly #ifdef and
"(void)level;" warning killer, by introducing kvm_memory_slot::arch and
related allocation functions:

	kvm_arch_alloc/init_*()

But one thing I hesitate to do is introducing architecture specific version
of 2:

	kvm_arch_free_physmem_slot(free, dont)

@free, @dont technique is already a bit tricky and distributing this kind
of style to other files than kvm_main.c will make the logic harder to follow.

Though it may be a pain, doesn't it make sense to clean up this stuff?


	Takuya

  reply	other threads:[~2012-01-25  6:20 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-23 10:42 [PATCH 0/4] KVM: Decouple rmap_pde from lpage_info write_count Takuya Yoshikawa
2012-01-23 10:42 ` Takuya Yoshikawa
2012-01-23 10:43 ` [PATCH 1/4] KVM: MMU: Use gfn_to_rmap() in audit_write_protection() Takuya Yoshikawa
2012-01-23 10:43   ` Takuya Yoshikawa
2012-01-23 10:43 ` [PATCH 2/4] KVM: MMU: Use __gfn_to_rmap() in kvm_handle_hva() Takuya Yoshikawa
2012-01-23 10:43   ` Takuya Yoshikawa
2012-01-23 10:44 ` [PATCH 3/4] KVM: Introduce gfn_to_index() which returns the index for a given level Takuya Yoshikawa
2012-01-23 10:44   ` Takuya Yoshikawa
2012-01-23 10:45 ` [RFC PATCH 4/4] KVM: Decouple rmap_pde from lpage_info write_count Takuya Yoshikawa
2012-01-23 10:45   ` Takuya Yoshikawa
2012-01-23 18:40 ` [PATCH 0/4] " Marcelo Tosatti
2012-01-23 18:40   ` Marcelo Tosatti
2012-01-24 11:24 ` Avi Kivity
2012-01-24 11:24   ` Avi Kivity
2012-01-24 14:35   ` Takuya Yoshikawa
2012-01-24 14:35     ` Takuya Yoshikawa
2012-01-25  6:20     ` Takuya Yoshikawa [this message]
2012-01-25  6:20       ` Takuya Yoshikawa

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=4F1F9F1F.1030601@oss.ntt.co.jp \
    --to=yoshikawa.takuya@oss.ntt.co.jp \
    --cc=avi@redhat.com \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.