[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] regression from c/s 22071:c5aed2e049bc (ept: Put locks around ept_get_entry) ?


  • To: Jan Beulich <JBeulich@xxxxxxxxxx>
  • From: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
  • Date: Tue, 14 Dec 2010 14:34:04 +0000
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 14 Dec 2010 06:35:00 -0800
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=S++qnWNiAQ6bzNY5mXHdn+UzGMLHAvqYXBvBT+lDd4swAhScgbAa9cle4evrcggzqC xJoM7WMbT3EdZus/W0NSOOlWWaMFKulq4J23a9Ee16oFi5YELgWhYaenxRz8yG4ZcXTI mhIfFRpPmfMlS3+BRZPgz900TD2kh+4JngQ6A=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>

BTW, let me know if you want the other PoD / p2m / ept patches I
haven't upstreamed yet.  Even if you don't end up backporting them,
they may be handy to have if there are problems later.

 -George

On Tue, Dec 14, 2010 at 2:32 PM, George Dunlap
<George.Dunlap@xxxxxxxxxxxxx> wrote:
> Try this one.
>
> FYI, the logic is pretty simple: Without this patch, ept_next_level()
> gets a pointer and the logic goes through and reads the actual entry
> piecemeal.  ept_set_entry() gets a pointer and goes through setting
> bits in the actual entry piecemeal as well.  The idea is, have
> ept_next_level() read the entire entry into a local variable, and then
> act on that; and have ept_set_entry() write the new entry into a local
> variable and then write the whole entry once.  So it's mostly changing
> "ept_entry->" to "new_entry."
>
>  -George
>
> On Tue, Dec 14, 2010 at 12:37 PM, Jan Beulich <JBeulich@xxxxxxxxxx> wrote:
>>>>> On 14.12.10 at 11:47, George Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote:
>>> Yes, I have to apologize: I have a queue of PoD, p2m, and ept-related
>>> fixes that I haven't pushed to the list because:
>>> * they require non-negligible reworking
>>> * it's been really difficult for me to set up an OSS-based system to test
>>> them
>>>
>>> It actually turns out that doing locking in ept_get_entry() is the
>>> wrong thing to do anyway; it can cause the following deadlock:
>>>
>>> p2m_change_type [grabs p2m lock] -> set_p2m_entry -> ept_set_entry ->
>>> ept_set_middle_level -> p2m_alloc [grabs hap lock]
>>>
>>> write cr4 -> hap_update_paging_modes [grabes hap lock] ->
>>> hap_update_cr3 -> gfn_to_mfn -> ept_get_entry -> [grabs p2m lock]
>>>
>>> Attached is a ported patch that removes locking in ept_get_entry(),
>>> and implements access-once semantics for reading and writing.  This
>>> solves the original problem (a race between reading and writing the
>>> table) without causing deadlocks.  I haven't had a chance to test it
>>> -- can you give it a spin?
>>
>> For really giving this a try I'd have to use it on 4.0, where it
>> doesn't apply at all. Resolving the rejects is non-obvious for me
>> in some cases, as I don't know this code well enough. Hence
>> for the moment we'll just drop the bad backport of your first
>> attempt.
>>
>> Jan
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
>>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.