[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 3/3] x86/mm-locks: apply a bias to lock levels for current domain
- To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- From: George Dunlap <george.dunlap@xxxxxxxxxx>
- Date: Wed, 19 Dec 2018 18:10:04 +0000
- Autocrypt: addr=george.dunlap@xxxxxxxxxx; prefer-encrypt=mutual; keydata= mQINBFPqG+MBEACwPYTQpHepyshcufo0dVmqxDo917iWPslB8lauFxVf4WZtGvQSsKStHJSj 92Qkxp4CH2DwudI8qpVbnWCXsZxodDWac9c3PordLwz5/XL41LevEoM3NWRm5TNgJ3ckPA+J K5OfSK04QtmwSHFP3G/SXDJpGs+oDJgASta2AOl9vPV+t3xG6xyfa2NMGn9wmEvvVMD44Z7R W3RhZPn/NEZ5gaJhIUMgTChGwwWDOX0YPY19vcy5fT4bTIxvoZsLOkLSGoZb/jHIzkAAznug Q7PPeZJ1kXpbW9EHHaUHiCD9C87dMyty0N3TmWfp0VvBCaw32yFtM9jUgB7UVneoZUMUKeHA fgIXhJ7I7JFmw3J0PjGLxCLHf2Q5JOD8jeEXpdxugqF7B/fWYYmyIgwKutiGZeoPhl9c/7RE Bf6f9Qv4AtQoJwtLw6+5pDXsTD5q/GwhPjt7ohF7aQZTMMHhZuS52/izKhDzIufl6uiqUBge 0lqG+/ViLKwCkxHDREuSUTtfjRc9/AoAt2V2HOfgKORSCjFC1eI0+8UMxlfdq2z1AAchinU0 eSkRpX2An3CPEjgGFmu2Je4a/R/Kd6nGU8AFaE8ta0oq5BSFDRYdcKchw4TSxetkG6iUtqOO ZFS7VAdF00eqFJNQpi6IUQryhnrOByw+zSobqlOPUO7XC5fjnwARAQABtCRHZW9yZ2UgVy4g RHVubGFwIDxkdW5sYXBnQHVtaWNoLmVkdT6JAkAEEwEKACoCGwMFCwkIBwMFFQoJCAsFFgID AQACHgECF4ACGQEFAlpk2IEFCQo9I54ACgkQpjY8MQWQtG1A1BAAnc0oX3+M/jyv4j/ESJTO U2JhuWUWV6NFuzU10pUmMqpgQtiVEVU2QbCvTcZS1U/S6bqAUoiWQreDMSSgGH3a3BmRNi8n HKtarJqyK81aERM2HrjYkC1ZlRYG+jS8oWzzQrCQiTwn3eFLJrHjqowTbwahoiMw/nJ+OrZO /VXLfNeaxA5GF6emwgbpshwaUtESQ/MC5hFAFmUBZKAxp9CXG2ZhTP6ROV4fwhpnHaz8z+BT NQz8YwA4gkmFJbDUA9I0Cm9D/EZscrCGMeaVvcyldbMhWS+aH8nbqv6brhgbJEQS22eKCZDD J/ng5ea25QnS0fqu3bMrH39tDqeh7rVnt8Yu/YgOwc3XmgzmAhIDyzSinYEWJ1FkOVpIbGl9 uR6seRsfJmUK84KCScjkBhMKTOixWgNEQ/zTcLUsfTh6KQdLTn083Q5aFxWOIal2hiy9UyqR VQydowXy4Xx58rqvZjuYzdGDdAUlZ+D2O3Jp28ez5SikA/ZaaoGI9S1VWvQsQdzNfD2D+xfL qfd9yv7gko9eTJzv5zFr2MedtRb/nCrMTnvLkwNX4abB5+19JGneeRU4jy7yDYAhUXcI/waS /hHioT9MOjMh+DoLCgeZJYaOcgQdORY/IclLiLq4yFnG+4Ocft8igp79dbYYHkAkmC9te/2x Kq9nEd0Hg288EO+5AQ0EVFpnOgEIAM6XPDYOTqW64Yma5+vV6947NvKfm+GvtATrwuPDX6za L2cOHhXiiM5iP7ehJCZEqgSMaG1kaQZMBsHhDbKp3dKooJrA8ODeyfV8dIfQEQ6olsV+I6+7 vcWriPgkSdawTTt1Vd9EHQAsEOC6oUf1gPiI3YcjB8I9xCRhOtTXT/4dM32i2AG7xIOO/0z0 4RbJuJvEXem1+0ZK6zoAWy/wDp2DjBIr8n2WSl9b74hHpgLy33ZNpWbe1Zul/32ym1fLT1Lm RC8zXnSb00wUt/5dRVc/TlHCw3loRhHZcalx9LGFoRPfj10wH8+ScSh/izHrcBDPA27jqAyK ZiBmSq2ftn0AEQEAAYkDRAQYAQoADwIbAgUCWmTW+QUJB+ujPwEpwF0gBBkBCgAGBQJUWmc6 AAoJELIVx6fHhBvtxesIALSpB4RaYtr2gQA9r7lTrC8bW3+aLbaBk3q7NBcfV9og6gN6Gvs8 8RITq25H+8gJNOdpKt3hQM816o6pUXTth7FYPUsNxAbo+dGoLkMhfVEYTcFpJoyXakUk/zL5 yF7CzXXI/wYMFvFoixNwdkjWJUgL1cuGh56BaLzi9hzwXjOIANV+jBuZu9xXDXWATy2YAsLB N4F5lW15eOHQ4QsfCtzX/iPjK8Q2MhdE75AsiCTjeQHntSmvi0/YwRyzSh2A8z5D6gRM4nTT HMuCROcs+KYLUUhbZs5l1OP5Srp7NFLYsqw2Zb49FG83IDmiMRsD99rGYCMxm0t1JJJ4UrzL hKgJEKY2PDEFkLRtji8P/RTPQdWZmdN29QhJ92ws/IuYmEOrwlAmvQGZWxADe+9VIoQeQaSA e/i8yuC9nbPJhl5DyrbmOv9A3EnAXvxyt1c1jpznWg3m0xuB214G7iN5l5g71tOajy9ZhId8 HKRwnmefRcT153tE0Kfw1ILgpslhUasrGuuICsMUAeNPCgdT3siIXDTD5kY/M0m7sHYdM+Ik DzK4vYhB89lZY4k87SrNEAs2YRu8nub27iRB+mb+qjSRWCVlQ1OWQ8gq2BmSoNch1zF3ukB0 KHIclPZ9EI8JpQ6qVbP6RkNPf7AdtIZrI+5eIjsVNvqhCXfaXxfB4fwHmMcbMT5f3s6CFH3M TVm/j7CpXCt8PQOZIWlDrdRhW9ywFPcKWwfUI37WAbHxJI4tzZAUytHi0TlpcQpPHXbbw10s ME4mbMuOlW/Rt01sc2d5SuZkG2/rw7E4TBq6VA3ZbSztvA6ZW6IZX/oX9dFyhw28gHG7+yRw WSNLkCgnO2rXhPJTNfOAn4bdBcQ8Adb9QbWdtqt0xpe6/NjAWGJMBmvXMiiDAKcyS3o8EXK2 CKtRdNjWisu3q/6KPQup7UxP1fMQ0dN9qGz6Cuw1tBKaTDRLS80c8i0WEHcHDSkEIx63sny1 GhyT0XIEmJfhdw99RvEh5S3CkxYnUpHay6KaHJgNKL5L2+oxzpIWA1S6uQENBFRaur0BCADt onSLWlBKZRHpldkPZgQPGJrYHJHS5mhNLs3Q1i/U6NTy/qnTXu7QVyjn5CiO799n3tJweGnn EZUCTmTFkEUNPii8l3Sch5KvdttbB83MbHXBrO193Ne3qfcwEqvsCGKgHWb6+6TfWt51R2eF u283s7jQwL5+BKTn/6NEbFjcg5U+ihArNQ7sznUag6DjCX2JrcfYTM6gaE3a+lNtPyoJwv3Z llnCQFGV2gBaftzWEQpJO5Pd/VWlKaGOdfQni68pnVXZHuuigolgUFzJILTBrxpOYC0C8uB9 yl76V6A62CoMrMu43jnHMSPKMKIjnbW3zPE0w8lj0WII82/SwKQPABEBAAGJAiUEGAEKAA8C GwwFAlpk1zMFCQfrT/YACgkQpjY8MQWQtG2/tg//YY59ZOVnER5btfVhrh+qtCoJtS0U+z55 0s/dOIoBzRJTAeWu8EY8OZHTcFN7EZtp55h3jiR/JGI9h59UIF+UqkLMrFkx1jhLHhnqF8nc fc2WZLd6ECTPvTVdVYytGzl8KoYkMhFFs+f/ZeOuxUv5OBSeQhzUbpr4S2tJdhxBLuacauOt x0GRw7eGBP/WO+Hlzp2AgeJ62MUA/xklxGb1q8hFq3g6Ghas6tUyrcx4RYEBu8hVBHqcS0VF LWLBKU+kZLNpeCwqht4VQ9FERSIk8rsScd1Qtk2uCx94cULYmiKbl6qtg+M+t4erwsdsMX2X P1kRxm6+DQJQfNZd+UP1B8jKHFbmC49JZRdK8FOAI4imealjUhHbxKS+N3072WMUIQwo0Eym 29/KJruT+JDn9R0+7PpJkCkbYiwZah8ytew+Cv9fNAA8O2t4J5q+UbpnGT9zRkkmQOoz+bza kKTbuIKqzxVjUCkHFvBwYmBYKukqC0EFm0cSQx700WCdprO6AnvO9IIeA9cBRaky3sl4lao3 XRDRjWj/GZQg8OhFPNjfAZ+S1yo0dRlqNlCtwo65B6U7d2GGb64UtjDthGBHFo8ruiwCxf5U us+iynkGfrfQHUFHCC5a8fSMal7+hrwKASyWNY4xgavv5ET61l6aGkJ+xV1hnzKlPjZGPXp8 q5c=
- Cc: Wei Liu <wei.liu2@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Wed, 19 Dec 2018 18:10:22 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Openpgp: preference=signencrypt
On 12/19/18 2:59 PM, Roger Pau Monné wrote:
>> Using 'current' means that potential deadlocks which would now cause a
>> BUG() won't anymore. I'm fine with not adding extra protections that
>> aren't there now; but I don't want to remove protections that are.
>
> The lock ordering enforcement is still kept as-is, but Xen is allowed
> to lock the caller mm locks in the right order (be it privileged or
> not) after having locked a subject domain ones also in the correct
> order.
Suppose we have mm_lock_x, with value 8, and mm_lock_y with value 16.
Suppose we have H1, which grabs current->mm_lock_x and tgt->mm_lock_y.
And suppose we have H2, which grabs current->mm_lock_y, and tgt->mm_lock_x.
And suppose domA calls H1 on domB at the same time that domB calls H2 on
domA. We could have the following sequence:
* H1: grab A->mm_lock_x
* H2: grab B->mm_lock_y
* H1: wait on B->mm_lock_y
* H2: wait on A->mm_lock_x #DEADLOCK
With the current mm lock checking, H2 would cause a BUG() the first time
it was called.
With my "special privilege only" boosting you'd also get a BUG() in at
least one of the invocations.
With your "current bias" patch, no BUG() would be encountered; we'd only
discover the deadlock once a live server had actually deadlocked.
This is what I'm talking about -- with "boost dom0", you have a global
order to the locks. It goes:
1-8: All domU locks (in the order listed in mm-locks.h)
9-16: All dom0 locks
Thus we know for certain that there can't be a caller / callee deadlock
that's not detected. With your patch, there isn't a global order: the
order changes based on who made the hypercall, so it's more difficult to
reason about whether there's a deadlock or not.
So, do H1 and H2 exist right now? I think probably not, but I can't
immediately say. Will such a pair *never* exist? I don't think I can
guarantee that either. That's why I want something to check.
> Using is_control_domain will limit the usage of paging_log_dirty_op to
> Dom0 (because that's the only domain that has is_privileged set,
> there's no way for the toolstack to create a domain with is_privileged
> set), no other paging domain will be able to use such hypercall
> without triggering the lock level check. Is that really what we
> want?
>
> I assume that under a distributed Xen system it would be possible to
> issue this hypercall from a domain different that Dom0 if it's given
> the right privileges?
I take it you mean a "disaggregated" system? This is what Andy was
talking about with XSM. As I said to him, at the moment, this will
_already_ fail, so it's not a regression.
I'm not willing to open up new potential holes for deadlocks,
particularly for a hypothetical use case that nobody has asked for and
doesn't work upstream at the moment.
If you want to make the calls succeed under a more "peer-to-peer"
arrangement, you're going to have to come up with something else:
Change the order of the mm locks, or grab the caller's p2m lock before
grabbing the paging lock, or refactoring what's covered by what, or
something.
Or, we can fix what's actually broken at the moment, and fix other
things when we encounter them.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|