* [PATCH for-4.8] altp2m: don't attempt to unshare pages during change_altp2m_gfn op
@ 2016-10-14 0:00 Tamas K Lengyel
2016-10-20 16:18 ` George Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Tamas K Lengyel @ 2016-10-14 0:00 UTC (permalink / raw)
To: xen-devel; +Cc: George Dunlap, Tamas K Lengyel, Jan Beulich, Andrew Cooper
Attempting to change gfn mappings with altp2m on a memory shared page results
in a lock-order violation (mm locking order violation: 282 > 254), which
crashes the hypervisor. Don't attempt to automatically unshare such pages and
just fall back to failing the op if the page type is not correct.
Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
xen/arch/x86/mm/p2m.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 9526fff..6a45185 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2628,7 +2628,7 @@ int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
if ( !mfn_valid(mfn) )
{
mfn = __get_gfn_type_access(hp2m, gfn_x(old_gfn), &t, &a,
- P2M_ALLOC | P2M_UNSHARE, &page_order, 0);
+ P2M_ALLOC, &page_order, 0);
if ( !mfn_valid(mfn) || t != p2m_ram_rw )
goto out;
--
2.9.3
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH for-4.8] altp2m: don't attempt to unshare pages during change_altp2m_gfn op
2016-10-14 0:00 [PATCH for-4.8] altp2m: don't attempt to unshare pages during change_altp2m_gfn op Tamas K Lengyel
@ 2016-10-20 16:18 ` George Dunlap
2016-10-20 16:19 ` Wei Liu
2016-10-20 16:29 ` Tamas K Lengyel
0 siblings, 2 replies; 6+ messages in thread
From: George Dunlap @ 2016-10-20 16:18 UTC (permalink / raw)
To: Tamas K Lengyel, xen-devel
Cc: George Dunlap, Andrew Cooper, Wei Liu, Jan Beulich
On 14/10/16 01:00, Tamas K Lengyel wrote:
> Attempting to change gfn mappings with altp2m on a memory shared page results
> in a lock-order violation (mm locking order violation: 282 > 254), which
> crashes the hypervisor. Don't attempt to automatically unshare such pages and
> just fall back to failing the op if the page type is not correct.
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
It would be nice to try to untangle thus such that you can reasonably
unshare a page in this circumstance; but given the point in the release
cycle, making it return an error instead of crashing is probably the
right thing to do.
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
This needs a release ack now I think as well.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.8] altp2m: don't attempt to unshare pages during change_altp2m_gfn op
2016-10-20 16:18 ` George Dunlap
@ 2016-10-20 16:19 ` Wei Liu
2016-10-20 16:29 ` Tamas K Lengyel
1 sibling, 0 replies; 6+ messages in thread
From: Wei Liu @ 2016-10-20 16:19 UTC (permalink / raw)
To: George Dunlap
Cc: Wei Liu, George Dunlap, Tamas K Lengyel, Jan Beulich,
Andrew Cooper, xen-devel
On Thu, Oct 20, 2016 at 05:18:36PM +0100, George Dunlap wrote:
> On 14/10/16 01:00, Tamas K Lengyel wrote:
> > Attempting to change gfn mappings with altp2m on a memory shared page results
> > in a lock-order violation (mm locking order violation: 282 > 254), which
> > crashes the hypervisor. Don't attempt to automatically unshare such pages and
> > just fall back to failing the op if the page type is not correct.
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
>
> It would be nice to try to untangle thus such that you can reasonably
> unshare a page in this circumstance; but given the point in the release
> cycle, making it return an error instead of crashing is probably the
> right thing to do.
>
> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
>
> This needs a release ack now I think as well.
>
>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.8] altp2m: don't attempt to unshare pages during change_altp2m_gfn op
2016-10-20 16:18 ` George Dunlap
2016-10-20 16:19 ` Wei Liu
@ 2016-10-20 16:29 ` Tamas K Lengyel
2016-10-20 16:40 ` George Dunlap
1 sibling, 1 reply; 6+ messages in thread
From: Tamas K Lengyel @ 2016-10-20 16:29 UTC (permalink / raw)
To: George Dunlap
Cc: George Dunlap, Andrew Cooper, Wei Liu, Jan Beulich, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 1125 bytes --]
On Oct 20, 2016 18:18, "George Dunlap" <george.dunlap@citrix.com> wrote:
>
> On 14/10/16 01:00, Tamas K Lengyel wrote:
> > Attempting to change gfn mappings with altp2m on a memory shared page
results
> > in a lock-order violation (mm locking order violation: 282 > 254), which
> > crashes the hypervisor. Don't attempt to automatically unshare such
pages and
> > just fall back to failing the op if the page type is not correct.
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
>
> It would be nice to try to untangle thus such that you can reasonably
> unshare a page in this circumstance; but given the point in the release
> cycle, making it return an error instead of crashing is probably the
> right thing to do.
You can unshare these pages, just have to do in a separate op so the locks
are taken in the right order (memshare before altp2m). Reversing the lock
order is not possible because otherwise the automatic unsharing and
propagation during runtime runs into the lock order problem without the
possibility of recovering. This way the user has the option to handle it
gracefully here.
Tamas
[-- Attachment #1.2: Type: text/html, Size: 1439 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.8] altp2m: don't attempt to unshare pages during change_altp2m_gfn op
2016-10-20 16:29 ` Tamas K Lengyel
@ 2016-10-20 16:40 ` George Dunlap
2016-10-20 16:42 ` Tamas K Lengyel
0 siblings, 1 reply; 6+ messages in thread
From: George Dunlap @ 2016-10-20 16:40 UTC (permalink / raw)
To: Tamas K Lengyel
Cc: George Dunlap, Andrew Cooper, Wei Liu, Jan Beulich, xen-devel
On 20/10/16 17:29, Tamas K Lengyel wrote:
> On Oct 20, 2016 18:18, "George Dunlap" <george.dunlap@citrix.com> wrote:
>>
>> On 14/10/16 01:00, Tamas K Lengyel wrote:
>>> Attempting to change gfn mappings with altp2m on a memory shared page
> results
>>> in a lock-order violation (mm locking order violation: 282 > 254), which
>>> crashes the hypervisor. Don't attempt to automatically unshare such
> pages and
>>> just fall back to failing the op if the page type is not correct.
>>>
>>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
>>
>> It would be nice to try to untangle thus such that you can reasonably
>> unshare a page in this circumstance; but given the point in the release
>> cycle, making it return an error instead of crashing is probably the
>> right thing to do.
>
> You can unshare these pages, just have to do in a separate op so the locks
> are taken in the right order (memshare before altp2m). Reversing the lock
> order is not possible because otherwise the automatic unsharing and
> propagation during runtime runs into the lock order problem without the
> possibility of recovering. This way the user has the option to handle it
> gracefully here.
Yay locks. :-)
It would probably be helpful to have a comment there explaining the
situation, so that people in the future don't need to re-discover this
issue.
Do you want to toss together a patch adding such a comment, or shall I?
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for-4.8] altp2m: don't attempt to unshare pages during change_altp2m_gfn op
2016-10-20 16:40 ` George Dunlap
@ 2016-10-20 16:42 ` Tamas K Lengyel
0 siblings, 0 replies; 6+ messages in thread
From: Tamas K Lengyel @ 2016-10-20 16:42 UTC (permalink / raw)
To: George Dunlap
Cc: George Dunlap, Andrew Cooper, Wei Liu, Jan Beulich, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 1696 bytes --]
On Oct 20, 2016 18:40, "George Dunlap" <george.dunlap@citrix.com> wrote:
>
> On 20/10/16 17:29, Tamas K Lengyel wrote:
> > On Oct 20, 2016 18:18, "George Dunlap" <george.dunlap@citrix.com> wrote:
> >>
> >> On 14/10/16 01:00, Tamas K Lengyel wrote:
> >>> Attempting to change gfn mappings with altp2m on a memory shared page
> > results
> >>> in a lock-order violation (mm locking order violation: 282 > 254),
which
> >>> crashes the hypervisor. Don't attempt to automatically unshare such
> > pages and
> >>> just fall back to failing the op if the page type is not correct.
> >>>
> >>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
> >>
> >> It would be nice to try to untangle thus such that you can reasonably
> >> unshare a page in this circumstance; but given the point in the release
> >> cycle, making it return an error instead of crashing is probably the
> >> right thing to do.
> >
> > You can unshare these pages, just have to do in a separate op so the
locks
> > are taken in the right order (memshare before altp2m). Reversing the
lock
> > order is not possible because otherwise the automatic unsharing and
> > propagation during runtime runs into the lock order problem without the
> > possibility of recovering. This way the user has the option to handle it
> > gracefully here.
>
> Yay locks. :-)
>
> It would probably be helpful to have a comment there explaining the
> situation, so that people in the future don't need to re-discover this
> issue.
>
> Do you want to toss together a patch adding such a comment, or shall I?
>
Please do so if you can, I'm traveling at the moment so it would be a
couple days before I could send a patch for that.
Thanks,
Tamas
[-- Attachment #1.2: Type: text/html, Size: 2324 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-20 16:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-14 0:00 [PATCH for-4.8] altp2m: don't attempt to unshare pages during change_altp2m_gfn op Tamas K Lengyel
2016-10-20 16:18 ` George Dunlap
2016-10-20 16:19 ` Wei Liu
2016-10-20 16:29 ` Tamas K Lengyel
2016-10-20 16:40 ` George Dunlap
2016-10-20 16:42 ` Tamas K Lengyel
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.