From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: regression from c/s 22071:c5aed2e049bc (ept: Put locks around ept_get_entry) ? Date: Tue, 14 Dec 2010 10:47:26 +0000 Message-ID: References: <4D073B3A0200007800027BDF@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=0015175ccf32e3d5a104975c8bce Return-path: In-Reply-To: <4D073B3A0200007800027BDF@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich , Christoph Egger Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org --0015175ccf32e3d5a104975c8bce Content-Type: text/plain; charset=ISO-8859-1 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? Thanks, -George On Tue, Dec 14, 2010 at 8:39 AM, Jan Beulich wrote: > George, > > with ept_get_entry() calling ept_pod_check_and_populate(), which in > turn unconditionally calls p2m_lock(), we got a report of the BUG() in > p2m_lock() triggering (in a backport of the patch on top of 4.0.1 - the > logic seems unchanged in -unstable, and hence the issue ought to > similarly exist there). Wouldn't therefore ept_pod_check_and_populate(), > only ever called from ept_get_entry(), not need to do any locking on > its own at all? > > Thanks, Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > --0015175ccf32e3d5a104975c8bce Content-Type: text/x-diff; charset=US-ASCII; name="ept-access-once.diff" Content-Disposition: attachment; filename="ept-access-once.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_ghonzxas0 ZXB0OiBSZW1vdmUgbG9jayBpbiBlcHRfZ2V0X2VudHJ5LCByZXBsYWNlIHdpdGggYWNjZXNzLW9u Y2Ugc2VtYW50aWNzLgoKVGhpcyBtaXJyb3JzIHRoZSBSVkkvc2hhZG93IHNpdHVhdGlvbiwgd2hl cmUgcDJtIHJlYWQgYWNjZXNzIGlzIGxvY2tsZXNzCmJlY2F1c2UgaXQncyBkb25lIGluIHRoZSBo YXJkd2FyZSAobGluZWFyIG1hcCBvZiB0aGUgcDJtIHRhYmxlKS4KClRoaXMgZml4ZXMgdGhlIG9y aWdpbmFsIGJ1ZyAoY2FsbCBpdCBidWcgQSkgd2l0aG91dCBpbnRyb2R1Y2luZyBidWcgQiAoYSBk ZWFkbG9jaykuCgpCdWcgQSB3YXMgY2F1c2VkIGJ5IGEgcmFjZSB3aGVuIHVwZGF0aW5nIHAybSBl bnRyaWVzOiBiZXR3ZWVuIHRlc3RpbmcgaWYgaXQncyB2YWxpZCwKYW5kIHRlc3RpbmcgaWYgaXQn cyBwb3B1bGF0ZS1vbi1kZW1hbmQsIGl0IG1heSBoYXZlIGJlZW4gY2hhbmdlZCBmcm9tIHBvcHVs YXRlLW9uLWRlbWFuZCB0byB2YWxpZC4KCk15IG9yaWdpbmFsIHBhdGNoIHNpbXBseSBpbnRyb2R1 Y2VkIGEgbG9jayBpbnRvIGVwdF9nZXRfZW50cnksIGJ1dCB0aGF0CmNhdXNlZCBidWcgQiwgY2F1 c2VkIGJ5IGNpcmN1bGFyIGxvY2tpbmcgb3JkZXI6CnAybV9jaGFuZ2VfdHlwZSBbZ3JhYnMgcDJt IGxvY2tdIC0+IHNldF9wMm1fZW50cnkgLT4gZXB0X3NldF9lbnRyeSAtPiBlcHRfc2V0X21pZGRs ZV9sZXZlbCAtPiBwMm1fYWxsb2MgW2dyYWJzIGhhcCBsb2NrXQp3cml0ZSBjcjQgLT4gaGFwX3Vw ZGF0ZV9wYWdpbmdfbW9kZXMgW2dyYWJlcyBoYXAgbG9ja10gLT4gaGFwX3VwZGF0ZV9jcjMgLT4g Z2ZuX3RvX21mbiAtPiBlcHRfZ2V0X2VudHJ5IC0+IFtncmFicyBwMm0gbG9ja10KClNpZ25lZC1v ZmYtYnk6IEdlb3JnZSBEdW5sYXAgPGdlb3JnZS5kdW5sYXBAZXUuY2l0cml4LmNvbT4KCmRpZmYg LXIgNDlkMmFhNWNlZTRlIHhlbi9hcmNoL3g4Ni9tbS9oYXAvcDJtLWVwdC5jCi0tLSBhL3hlbi9h cmNoL3g4Ni9tbS9oYXAvcDJtLWVwdC5jCVRodSBEZWMgMDkgMTY6MTc6MzMgMjAxMCArMDAwMAor KysgYi94ZW4vYXJjaC94ODYvbW0vaGFwL3AybS1lcHQuYwlUdWUgRGVjIDE0IDEwOjM2OjQzIDIw MTAgKzAwMDAKQEAgLTIxMSw3ICsyMTEsNyBAQAogICAgICAgICAgICAgICAgICAgICAgICAgICBp bnQgbmV4dF9sZXZlbCkKIHsKICAgICB1bnNpZ25lZCBsb25nIG1mbjsKLSAgICBlcHRfZW50cnlf dCAqZXB0X2VudHJ5OworICAgIGVwdF9lbnRyeV90ICplcHRfZW50cnksIGU7CiAgICAgdTMyIHNo aWZ0LCBpbmRleDsKIAogICAgIHNoaWZ0ID0gbmV4dF9sZXZlbCAqIEVQVF9UQUJMRV9PUkRFUjsK QEAgLTIyMyw5ICsyMjMsMTQgQEAKIAogICAgIGVwdF9lbnRyeSA9ICgqdGFibGUpICsgaW5kZXg7 CiAKLSAgICBpZiAoICFpc19lcHRlX3ByZXNlbnQoZXB0X2VudHJ5KSApCisgICAgLyogZXB0X25l eHRfbGV2ZWwoKSBpcyBjYWxsZWQgKHNvbWV0aW1lcykgd2l0aG91dCBhIGxvY2suICBSZWFkCisg ICAgICogdGhlIGVudHJ5IG9uY2UsIGFuZCBhY3Qgb24gdGhlICJjYWNoZWQiIGVudHJ5IGFmdGVy IHRoYXQgdG8KKyAgICAgKiBhdm9pZCByYWNlcy4gKi8KKyAgICBlPSplcHRfZW50cnk7CisKKyAg ICBpZiAoICFpc19lcHRlX3ByZXNlbnQoJmUpICkKICAgICB7Ci0gICAgICAgIGlmICggZXB0X2Vu dHJ5LT5hdmFpbDEgPT0gcDJtX3BvcHVsYXRlX29uX2RlbWFuZCApCisgICAgICAgIGlmICggZS5h dmFpbDEgPT0gcDJtX3BvcHVsYXRlX29uX2RlbWFuZCApCiAgICAgICAgICAgICByZXR1cm4gR1VF U1RfVEFCTEVfUE9EX1BBR0U7CiAKICAgICAgICAgaWYgKCByZWFkX29ubHkgKQpAQCAtMjMzLDEz ICsyMzgsMTUgQEAKIAogICAgICAgICBpZiAoICFlcHRfc2V0X21pZGRsZV9lbnRyeShwMm0sIGVw dF9lbnRyeSkgKQogICAgICAgICAgICAgcmV0dXJuIEdVRVNUX1RBQkxFX01BUF9GQUlMRUQ7Cisg ICAgICAgIGVsc2UKKyAgICAgICAgICAgIGU9KmVwdF9lbnRyeTsgLyogUmVmcmVzaCAqLwogICAg IH0KIAogICAgIC8qIFRoZSBvbmx5IHRpbWUgc3Agd291bGQgYmUgc2V0IGhlcmUgaXMgaWYgd2Ug aGFkIGhpdCBhIHN1cGVycGFnZSAqLwotICAgIGlmICggaXNfZXB0ZV9zdXBlcnBhZ2UoZXB0X2Vu dHJ5KSApCisgICAgaWYgKCBpc19lcHRlX3N1cGVycGFnZSgmZSkgKQogICAgICAgICByZXR1cm4g R1VFU1RfVEFCTEVfU1VQRVJfUEFHRTsKIAotICAgIG1mbiA9IGVwdF9lbnRyeS0+bWZuOworICAg IG1mbiA9IGUubWZuOwogICAgIHVubWFwX2RvbWFpbl9wYWdlKCp0YWJsZSk7CiAgICAgKnRhYmxl ID0gbWFwX2RvbWFpbl9wYWdlKG1mbik7CiAgICAgKmdmbl9yZW1haW5kZXIgJj0gKDFVTCA8PCBz aGlmdCkgLSAxOwpAQCAtMzE4LDE5ICszMjUsMjQgQEAKICAgICAgICAgaWYgKCBtZm5fdmFsaWQo bWZuX3gobWZuKSkgfHwgZGlyZWN0X21taW8gfHwgcDJtX2lzX3BhZ2VkKHAybXQpIHx8CiAgICAg ICAgICAgICAgKHAybXQgPT0gcDJtX3JhbV9wYWdpbmdfaW5fc3RhcnQpICkKICAgICAgICAgewot ICAgICAgICAgICAgZXB0X2VudHJ5LT5lbXQgPSBlcHRlX2dldF9lbnRyeV9lbXQocDJtLT5kb21h aW4sIGdmbiwgbWZuLCAmaXBhdCwKKyAgICAgICAgICAgIGVwdF9lbnRyeV90IG5ld19lbnRyeTsK KworICAgICAgICAgICAgLyogQ29uc3RydWN0IHRoZSBuZXcgZW50cnksIGFuZCB0aGVuIHdyaXRl IGl0IG9uY2UgKi8KKyAgICAgICAgICAgIG5ld19lbnRyeS5lbXQgPSBlcHRlX2dldF9lbnRyeV9l bXQocDJtLT5kb21haW4sIGdmbiwgbWZuLCAmaXBhdCwKICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgIGRpcmVjdF9tbWlvKTsKLSAgICAgICAgICAgIGVwdF9l bnRyeS0+aXBhdCA9IGlwYXQ7Ci0gICAgICAgICAgICBlcHRfZW50cnktPnNwID0gb3JkZXIgPyAx IDogMDsKLSAgICAgICAgICAgIGVwdF9lbnRyeS0+YXZhaWwxID0gcDJtdDsKLSAgICAgICAgICAg IGVwdF9lbnRyeS0+YXZhaWwyID0gMDsKKyAgICAgICAgICAgIG5ld19lbnRyeS5pcGF0ID0gaXBh dDsKKyAgICAgICAgICAgIG5ld19lbnRyeS5zcCA9IG9yZGVyID8gMSA6IDA7CisgICAgICAgICAg ICBuZXdfZW50cnkuYXZhaWwxID0gcDJtdDsKKyAgICAgICAgICAgIG5ld19lbnRyeS5hdmFpbDIg PSAwOwogCi0gICAgICAgICAgICBpZiAoIGVwdF9lbnRyeS0+bWZuID09IG1mbl94KG1mbikgKQor ICAgICAgICAgICAgaWYgKCBuZXdfZW50cnkubWZuID09IG1mbl94KG1mbikgKQogICAgICAgICAg ICAgICAgIG5lZWRfbW9kaWZ5X3Z0ZF90YWJsZSA9IDA7CiAgICAgICAgICAgICBlbHNlCi0gICAg ICAgICAgICAgICAgZXB0X2VudHJ5LT5tZm4gPSBtZm5feChtZm4pOworICAgICAgICAgICAgICAg IG5ld19lbnRyeS5tZm4gPSBtZm5feChtZm4pOwogCi0gICAgICAgICAgICBlcHRfcDJtX3R5cGVf dG9fZmxhZ3MoZXB0X2VudHJ5LCBwMm10KTsKKyAgICAgICAgICAgIGVwdF9wMm1fdHlwZV90b19m bGFncygmbmV3X2VudHJ5LCBwMm10KTsKKworICAgICAgICAgICAgZXB0X2VudHJ5LT5lcHRlID0g bmV3X2VudHJ5LmVwdGU7CiAgICAgICAgIH0KICAgICAgICAgZWxzZQogICAgICAgICAgICAgZXB0 X2VudHJ5LT5lcHRlID0gMDsKQEAgLTMzOSw2ICszNTEsNyBAQAogICAgIHsKICAgICAgICAgLyog V2UgbmVlZCB0byBzcGxpdCB0aGUgb3JpZ2luYWwgcGFnZS4gKi8KICAgICAgICAgZXB0X2VudHJ5 X3Qgc3BsaXRfZXB0X2VudHJ5OworICAgICAgICBlcHRfZW50cnlfdCBuZXdfZW50cnk7CiAKICAg ICAgICAgQVNTRVJUKGlzX2VwdGVfc3VwZXJwYWdlKGVwdF9lbnRyeSkpOwogCkBAIC0zNjUsMTgg KzM3OCwyMCBAQAogCiAgICAgICAgIGVwdF9lbnRyeSA9IHRhYmxlICsgaW5kZXg7CiAKLSAgICAg ICAgZXB0X2VudHJ5LT5lbXQgPSBlcHRlX2dldF9lbnRyeV9lbXQoZCwgZ2ZuLCBtZm4sICZpcGF0 LCBkaXJlY3RfbW1pbyk7Ci0gICAgICAgIGVwdF9lbnRyeS0+aXBhdCA9IGlwYXQ7Ci0gICAgICAg IGVwdF9lbnRyeS0+c3AgPSBpID8gMSA6IDA7Ci0gICAgICAgIGVwdF9lbnRyeS0+YXZhaWwxID0g cDJtdDsKLSAgICAgICAgZXB0X2VudHJ5LT5hdmFpbDIgPSAwOworICAgICAgICBuZXdfZW50cnku ZW10ID0gZXB0ZV9nZXRfZW50cnlfZW10KGQsIGdmbiwgbWZuLCAmaXBhdCwgZGlyZWN0X21taW8p OworICAgICAgICBuZXdfZW50cnkuaXBhdCA9IGlwYXQ7CisgICAgICAgIG5ld19lbnRyeS5zcCA9 IGkgPyAxIDogMDsKKyAgICAgICAgbmV3X2VudHJ5LmF2YWlsMSA9IHAybXQ7CisgICAgICAgIG5l d19lbnRyeS5hdmFpbDIgPSAwOwogCi0gICAgICAgIGlmICggZXB0X2VudHJ5LT5tZm4gPT0gbWZu X3gobWZuKSApCisgICAgICAgIGlmICggbmV3X2VudHJ5Lm1mbiA9PSBtZm5feChtZm4pICkKICAg ICAgICAgICAgICBuZWVkX21vZGlmeV92dGRfdGFibGUgPSAwOwogICAgICAgICBlbHNlIC8qIHRo ZSBjYWxsZXIgc2hvdWxkIHRha2UgY2FyZSBvZiB0aGUgcHJldmlvdXMgcGFnZSAqLwotICAgICAg ICAgICAgZXB0X2VudHJ5LT5tZm4gPSBtZm5feChtZm4pOworICAgICAgICAgICAgbmV3X2VudHJ5 Lm1mbiA9IG1mbl94KG1mbik7CiAKLSAgICAgICAgZXB0X3AybV90eXBlX3RvX2ZsYWdzKGVwdF9l bnRyeSwgcDJtdCk7CisgICAgICAgIGVwdF9wMm1fdHlwZV90b19mbGFncygmbmV3X2VudHJ5LCBw Mm10KTsKKworICAgICAgICBlcHRfZW50cnktPmVwdGUgPSBuZXdfZW50cnkuZXB0ZTsKICAgICB9 CiAKICAgICAvKiBUcmFjayB0aGUgaGlnaGVzdCBnZm4gZm9yIHdoaWNoIHdlIGhhdmUgZXZlciBo YWQgYSB2YWxpZCBtYXBwaW5nICovCkBAIC00MzcsMTAgKzQ1Miw2IEBACiAgICAgaW50IGk7CiAg ICAgaW50IHJldCA9IDA7CiAgICAgbWZuX3QgbWZuID0gX21mbihJTlZBTElEX01GTik7Ci0gICAg aW50IGRvX2xvY2tpbmcgPSAhcDJtX2xvY2tlZF9ieV9tZShwMm0pOwotCi0gICAgaWYgKCBkb19s b2NraW5nICkKLSAgICAgICAgcDJtX2xvY2socDJtKTsKIAogICAgICp0ID0gcDJtX21taW9fZG07 CiAKQEAgLTUxNyw4ICs1MjgsNiBAQAogICAgIH0KIAogb3V0OgotICAgIGlmICggZG9fbG9ja2lu ZyApCi0gICAgICAgIHAybV91bmxvY2socDJtKTsKICAgICB1bm1hcF9kb21haW5fcGFnZSh0YWJs ZSk7CiAgICAgcmV0dXJuIG1mbjsKIH0K --0015175ccf32e3d5a104975c8bce Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --0015175ccf32e3d5a104975c8bce--