From: Andrea Arcangeli <aarcange@redhat.com>
To: Avi Kivity <avi@redhat.com>
Cc: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>,
Marcelo Tosatti <mtosatti@redhat.com>,
LKML <linux-kernel@vger.kernel.org>, KVM <kvm@vger.kernel.org>
Subject: Re: [PATCH] KVM: MMU: fix huge page adapted on non-PAE host
Date: Mon, 28 May 2012 16:42:05 +0200 [thread overview]
Message-ID: <20120528144205.GG4016@redhat.com> (raw)
In-Reply-To: <4FC38992.8070408@redhat.com>
On Mon, May 28, 2012 at 05:20:02PM +0300, Avi Kivity wrote:
> The "right thing" we should be doing is running get_page() on every
> small page within the frame (we asked for a small page but are
> opportunistrically using the pages around it, without a proper ref).
> That's a bit slow though, so we cheat.
The problem is that we're aligning the pfn. The refcount move just
follows the pfn alignment. The spte setting I think need the correct
aligned pfn that matches the hugepmd NPT/EPT alignment.
So then we're moving the pfn refcount too, otherwise
kvm_release_pfn_clean then would run on a different pfn than the one
that was returned by gup-fast.
If we would drop the refcount before calling __direct_map or use a
gup-fast that doesn't even take a pin, we wouldn't need to move the
refcount and we could only free the page (without having to do a
get_page).
> But I guess we can start with your fix. But what about shifting mask by
> one bit? Isn't it sufficient?
>
> - mask = KVM_PAGES_PER_HPAGE(level) - 1;
> + mask = KVM_PAGES_PER_HPAGE(level);
> + mask *= KVM_HOST_HPAGES_PER_HPAGE;
> + mask -= 1;
>
> This should move the reference to the right place.
The pfn passed to mmu_set_spte then would then be aligned at 4M
despite the NTP/EPT size is 2M, so I doubt it would be ok. The real
thing to check here is that the pfn passed is correct. The refcount
just follows.
Just doing s/get_page_unless_zero()/get_page()/ should work I
think. And good thing there's no chance to get this wrong by testing,
it either boots or doesn't boot.
next prev parent reply other threads:[~2012-05-28 14:42 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-28 6:10 [PATCH] KVM: MMU: fix huge page adapted on non-PAE host Xiao Guangrong
2012-05-28 10:57 ` Avi Kivity
2012-05-28 11:39 ` Xiao Guangrong
2012-05-28 12:24 ` Avi Kivity
2012-05-28 12:56 ` Xiao Guangrong
2012-05-28 13:14 ` Avi Kivity
2012-05-28 13:41 ` Xiao Guangrong
2012-05-28 13:53 ` Avi Kivity
2012-05-28 14:05 ` Xiao Guangrong
2012-05-28 14:20 ` Avi Kivity
2012-05-28 14:42 ` Andrea Arcangeli [this message]
2012-05-28 14:32 ` Andrea Arcangeli
2012-05-28 14:40 ` Avi Kivity
2012-05-28 14:44 ` Andrea Arcangeli
2012-05-29 14:23 ` Avi Kivity
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=20120528144205.GG4016@redhat.com \
--to=aarcange@redhat.com \
--cc=avi@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=xiaoguangrong@linux.vnet.ibm.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.