Skip to contentSkip to conversation

x86: fix preemptable page type handling

PATCH In review Jan Beulich 2 participants 2 messages Oct 30, 2008

lore
Share & tools
lore

Jan Beulich

author
#0 lore raw

PATCH 1/1 x86: fix preemptable page type handling

- retain a page reference when PGT_partial is set on a page (and drop it when clearing that flag)
- don't drop a page reference never acquired when freeing the page type of a page where the allocation of the type got preempted (and never completed)
- don't acquire a page reference when allocating the page type of a page where freeing the type got preempted (and never completed, and hence didn't drop the respective reference)

(Will apply properly only with the previously sent patch using the preemption flags in domain cleanup applied first.)

Signed-off-by Jan Beulich <[email protected]>
xen/arch/x86/domain.c +7 -1
Index: 2008-10-27/xen/arch/x86/domain.c
===================================================================
@@ -1683,18 +1683,24 @@ static int relinquish_memory(
                     break;
                 case -EINTR:
                     page->u.inuse.type_info |= PGT_validated;
+                    if ( x & PGT_partial )
+                        put_page(page);
                     put_page(page);
                     ret = -EAGAIN;
                     goto out;
                 case -EAGAIN:
                     page->u.inuse.type_info |= PGT_partial;
-                    put_page(page);
+                    if ( x & PGT_partial )
+                        put_page(page);
                     goto out;
                 default:
                     BUG();
                 }
                 if ( x & PGT_partial )
+                {
                     page->u.inuse.type_info--;
+                    put_page(page);
+                }
                 break;
             }
         }
xen/arch/x86/mm.c +91 -41

390 lines not on this page · raw message

xen/include/asm-x86/mm.h +5 -3
Index: 2008-10-27/xen/include/asm-x86/mm.h
===================================================================
@@ -61,12 +61,14 @@ struct page_info
         /*
          * When PGT_partial is true then this field is valid and indicates
          * that PTEs in the range [0, @nr_validated_ptes) have been validated.
-         * If @partial_pte is true then PTE at @nr_validated_ptes+1 has been
-         * partially validated.
+         * If @partial_pte is positive then PTE at @nr_validated_ptes+1 has
+         * been partially validated.
+         * If @partial_pte is negative then PTE at @nr_validated_ptes+1 has
+         * been partially invalidated.
          */
         struct {
             u16 nr_validated_ptes;
-            bool_t partial_pte;
+            s8 partial_pte;
         };
 
         /*
#1 lore raw
Quoted 8 lines · Jan Beulich

- retain a page reference when PGT_partial is set on a page (and drop it when clearing that flag)
- don't drop a page reference never acquired when freeing the page type of a page where the allocation of the type got preempted (and never completed)
- don't acquire a page reference when allocating the page type of a page where freeing the type got preempted (and never completed, and hence didn't drop the respective reference)

I don't understand exactly what's being explained here, so nobody else really stands a chance. Reference counting (both general count and type count) and its interaction with preemption is now complicated enough that it needs an explanation from first principles. That should go both in the changeset comment and in a code comment somewhere appropriate (I would say in the comment in mm.h that your patch already modifies) -- it'll be a rather annoyingly large code comment, but it will have significant value in describing something that will otherwise make people's brains bleed. If you can have a go at bashing something out that is basically comprehensive, I can knock off any rough edges in the explanation before checking in.

Thanks,
Keir