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

Re: [PATCH v2 2/3] xen/heap: Split init_heap_pages() in two


  • To: Julien Grall <julien@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Wei Chen <Wei.Chen@xxxxxxx>
  • Date: Mon, 18 Jul 2022 16:18:58 +0800
  • Arc-authentication-results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com])
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=7g6d8Ro0Yxe62ViASgmY2GXjGbikG0PCi8ly4bRu7c4=; b=MHElfvqdGDwKphFIO0+0o1nlrx0ewBhGbGnKKuoYZj37vETsTD1KB34gqYcNDr3sKLJlt/U84OavrtNqEhmlIDvrm+2Hpz3anN5KRL9iRrFdQXc8a5p/nDoOCrDkq24/NPpPTKwMXxVOCExH1tAVhpjSQRYQHlfA9m7b/fp9SzUkxA2Y6VPQ1NvhN38RF9SYB1Qq4R4mxhPNoUOG1KNlS5xYtTuUwT/goU4TuIu/+FakZMxXmWZFa0Juyg7F/J67XRE9yTeT0Kq5dcRUCQAMpwphhByPRkOnlV1bDVtqlhWk4OD1P/m/U5Ykg38GtjF8ZXtLGlQv06Q0q9eitz/iqQ==
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=7g6d8Ro0Yxe62ViASgmY2GXjGbikG0PCi8ly4bRu7c4=; b=bYtBFM2dzz3/7JMiUtX5JTnvlDE89e+co2dBG5EiMCXNworgLGbE9jShjUe1hrFfPGf4zCBa86MIZfGHDPN8ximoxMbPh1z6abjAG6m/L39Gi4k75TVjAWPncD4U9Crx8nOPTLetBbp5S6q4cbF+cEIn45GPxXtbjf1Zj2UT9gbHmVrgdGvSMGxVh8h8TESTafMtMiSYNqVOJloGqGNAJRRvspf1QXUWJcSvFRyneiNo4qUDZO/DvVJK13AZCpvKhvSxXmNjhS5ru62yi9A3DygjtoRVGzr8AqN+V+OPF9ljo4FKoyKEl9DRpw7f+TQrReiaSkhs5SlOuRHU+YaxFA==
  • Arc-seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=RoFnJVweWxPUlNbHbLQDxgqPeGWEUSPv1z3v46jWNz+x28TnZJXkCQBldFDxobQ0ApjPtZEvjR3WEZ2C+dKBu3UsnM80dMC471J3JdHELyLQDp7Tbk1Yu4kiz7xGnkaD+JjSLFiaVJISnjPqEUjPqQZ5fDqb0lIGBUhmd2FTs6Zv7TI/nMCSQXwwMtQ5ZdbG/zakw+dsur7M4vnw+sCv7DhwrDALSFSoTFhYZwQNZ0OT9mdM7paMzkdV6Qoz8hb2U3vLu34LOg4YmNseDuLwmM1UgaIF7Y/e6q5bFlcsAq7NLcY7z6v11UclOcfudVcwPwgvV669JU9EW/PgOFfGbA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=d5mDZ7UYW0t7cVPxCUPLEbl1GArb7ECOiiH4RcEUuXNKNl0kk8Av9e2hnefm/aZoVUU4rC+jlwudtg37BdYoIOdYZ5NA77lMEOOw+KnxhBXltILENcw3ZMU6l1vaHpqpXtwwL3TD0hAdT9RbUbRt20oE3LeqcDKRLM8cqrwxwUA0UdC6zAIo6tE0RHfGrk/OALb+3+zTY6ifyfodUgbxwNBKI+Zhv8pTR1Re1esA4ZDe3TN1kv9jxwWXIPrCdN/YjsV8d3acrhZ5xo7qeKhGl8yz2hQIbWkrptSrkHaOjieAwj/5iuNZEx6+EGmHVLb9BoOm0ucrG2b+91xTl85mLw==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Julien Grall <jgrall@xxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Mon, 18 Jul 2022 08:19:44 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;

Hi Julien,

On 2022/7/16 1:03, Julien Grall wrote:
From: Julien Grall <jgrall@xxxxxxxxxx>

At the moment, init_heap_pages() will call free_heap_pages() page
by page. To reduce the time to initialize the heap, we will want
to provide multiple pages at the same time.

init_heap_pages() is now split in two parts:
     - init_heap_pages(): will break down the range in multiple set
       of contiguous pages. For now, the criteria is the pages should
       belong to the same NUMA node.
     - _init_heap_pages(): will initialize a set of pages belonging to
       the same NUMA node. In a follow-up patch, new requirements will
       be added (e.g. pages should belong to the same zone). For now the
       pages are still passed one by one to free_heap_pages().

Note that the comment before init_heap_pages() is heavily outdated and
does not reflect the current code. So update it.

This patch is a merge/rework of patches from David Woodhouse and
Hongyan Xia.

Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>

---

Interestingly, I was expecting this patch to perform worse. However,
from testing there is a small increase in perf.

That said, I mainly plit the patch because it keeps refactoring and
optimization separated.

Changes in v2:
     - Rename init_contig_pages() to _init_heap_pages()
     - Fold is_contig_page()
---
  xen/common/page_alloc.c | 77 ++++++++++++++++++++++++++---------------
  1 file changed, 50 insertions(+), 27 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 078c2990041d..eedb2fed77c3 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1778,16 +1778,44 @@ int query_page_offline(mfn_t mfn, uint32_t *status)
  }
/*
- * Hand the specified arbitrary page range to the specified heap zone
- * checking the node_id of the previous page.  If they differ and the
- * latter is not on a MAX_ORDER boundary, then we reserve the page by
- * not freeing it to the buddy allocator.
+ * This function should only be called with valid pages from the same NUMA
+ * node.
   */
+static void _init_heap_pages(const struct page_info *pg,
+                             unsigned long nr_pages,
+                             bool need_scrub)
+{
+    unsigned long s, e;
+    unsigned int nid = phys_to_nid(page_to_maddr(pg));
+
+    s = mfn_x(page_to_mfn(pg));
+    e = mfn_x(mfn_add(page_to_mfn(pg + nr_pages - 1), 1));
+    if ( unlikely(!avail[nid]) )
+    {
+        bool use_tail = IS_ALIGNED(s, 1UL << MAX_ORDER) &&
+                        (find_first_set_bit(e) <= find_first_set_bit(s));
+        unsigned long n;
+
+        n = init_node_heap(nid, s, nr_pages, &use_tail);
+        BUG_ON(n > nr_pages);
+        if ( use_tail )
+            e -= n;
+        else
+            s += n;
+    }
+
+    while ( s < e )
+    {
+        free_heap_pages(mfn_to_page(_mfn(s)), 0, need_scrub);
+        s += 1UL;
+    }
+}
+
  static void init_heap_pages(
      struct page_info *pg, unsigned long nr_pages)
  {
      unsigned long i;
-    bool idle_scrub = false;
+    bool need_scrub = scrub_debug;

You have changed idle_scrub to need_scrub, but haven't mentioned this
in commit log, and I also haven't found related discussion in v1. I
am very clear about this change.

Cheers,
Wei Chen

      /*
       * Keep MFN 0 away from the buddy allocator to avoid crossing zone
@@ -1812,35 +1840,30 @@ static void init_heap_pages(
      spin_unlock(&heap_lock);
if ( system_state < SYS_STATE_active && opt_bootscrub == BOOTSCRUB_IDLE )
-        idle_scrub = true;
+        need_scrub = true;
- for ( i = 0; i < nr_pages; i++ )
+    for ( i = 0; i < nr_pages; )
      {
-        unsigned int nid = phys_to_nid(page_to_maddr(pg+i));
+        unsigned int nid = phys_to_nid(page_to_maddr(pg));
+        unsigned long left = nr_pages - i;
+        unsigned long contig_pages;
- if ( unlikely(!avail[nid]) )
+        /*
+         * _init_heap_pages() is only able to accept range following
+         * specific property (see comment on top of _init_heap_pages()).
+         *
+         * So break down the range in smaller set.
+         */
+        for ( contig_pages = 1; contig_pages < left; contig_pages++ )
          {
-            unsigned long s = mfn_x(page_to_mfn(pg + i));
-            unsigned long e = mfn_x(mfn_add(page_to_mfn(pg + nr_pages - 1), 
1));
-            bool use_tail = (nid == phys_to_nid(pfn_to_paddr(e - 1))) &&
-                            IS_ALIGNED(s, 1UL << MAX_ORDER) &&
-                            (find_first_set_bit(e) <= find_first_set_bit(s));
-            unsigned long n;
-
-            n = init_node_heap(nid, mfn_x(page_to_mfn(pg + i)), nr_pages - i,
-                               &use_tail);
-            BUG_ON(i + n > nr_pages);
-            if ( n && !use_tail )
-            {
-                i += n - 1;
-                continue;
-            }
-            if ( i + n == nr_pages )
+            if ( nid != (phys_to_nid(page_to_maddr(pg))) )
                  break;
-            nr_pages -= n;
          }
- free_heap_pages(pg + i, 0, scrub_debug || idle_scrub);
+        _init_heap_pages(pg, contig_pages, need_scrub);
+
+        pg += contig_pages;
+        i += contig_pages;
      }
  }



 


Rackspace

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