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

Re: [Xen-devel] [PATCH v2 1/2] x86/mm: fix a potential race condition in map_pages_to_xen().





On 11/10/2017 5:49 PM, Jan Beulich wrote:
On 10.11.17 at 08:18, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4844,9 +4844,19 @@ int map_pages_to_xen(
              {
                  unsigned long base_mfn;
- pl1e = l2e_to_l1e(*pl2e);
                  if ( locking )
                      spin_lock(&map_pgdir_lock);
+
+                /* Skip the re-consolidation if it's done on another CPU. */
+                if ( l2e_get_flags(*pl2e) & _PAGE_PSE )
+                {
+                    if ( locking )
+                        spin_unlock(&map_pgdir_lock);
+                    goto check_l3;
+                }
+
+                ol2e = *pl2e;
This wants moving up ahead of the if(), so you can use the local
variable inside that if().

Got it. Thanks.

@@ -4880,6 +4889,15 @@ int map_pages_to_xen(
if ( locking )
                  spin_lock(&map_pgdir_lock);
+
+            /* Skip the re-consolidation if it's done on another CPU. */
+            if ( l3e_get_flags(*pl3e) & _PAGE_PSE )
+            {
+                if ( locking )
+                    spin_unlock(&map_pgdir_lock);
+                continue;
+            }
+
              ol3e = *pl3e;
Same here - move the if() below here and use ol3e in there.

With that
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

I'm not certain this is important enough a fix to consider for 4.10,
and you seem to think it's good enough if this gets applied only
after the tree would be branched, as you didn't Cc Julien. Please
indicate if you actually simply weren't aware, and you indeed
there's an important aspect to this that I'm overlooking.

Well, at first I have not expected this to be accepted for 4.10. But since we have met this issue in practice, when running a graphic application which consumes memory intensively in dom0, I think it also makes sense if we can fix it in xen's release as early as possible. Do you think this is a reasonable requirement? :-)

Also Cc to Julien. Julien, any comment? :-)

Yu
Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

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