All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mina Almasry <almasrymina@google.com>
To: mike.kravetz@oracle.com
Cc: shuah@kernel.org, almasrymina@google.com, rientjes@google.com,
	shakeelb@google.com, gthelen@google.com,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, linux-kselftest@vger.kernel.org,
	cgroups@vger.kernel.org
Subject: [PATCH v12 7/9] hugetlb: support file_region coalescing again
Date: Tue, 11 Feb 2020 13:31:26 -0800	[thread overview]
Message-ID: <20200211213128.73302-7-almasrymina@google.com> (raw)
In-Reply-To: <20200211213128.73302-1-almasrymina@google.com>

An earlier patch in this series disabled file_region coalescing in order
to hang the hugetlb_cgroup uncharge info on the file_region entries.

This patch re-adds support for coalescing of file_region entries.
Essentially everytime we add an entry, we call a recursive function that
tries to coalesce the added region with the regions next to it. The
worst case call depth for this function is 3: one to coalesce with the
region next to it, one to coalesce to the region prev, and one to reach
the base case.

This is an important performance optimization as private mappings add
their entries page by page, and we could incur big performance costs for
large mappings with lots of file_region entries in their resv_map.

Signed-off-by: Mina Almasry <almasrymina@google.com>

---

Changes in v12:
- Changed logic for coalescing. Instead of checking inline to coalesce
with only the region on next or prev, we now have a recursive function
that takes care of coalescing in both directions.
- For testing this code I added a bunch of debug code that checks that
the entries in the resv_map are coalesced appropriately. This passes
with libhugetlbfs tests.

---
 mm/hugetlb.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2d62dd35399db..45219cb58ac71 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -276,6 +276,86 @@ static void record_hugetlb_cgroup_uncharge_info(struct hugetlb_cgroup *h_cg,
 #endif
 }

+static bool has_same_uncharge_info(struct file_region *rg,
+				   struct file_region *org)
+{
+#ifdef CONFIG_CGROUP_HUGETLB
+	return rg && org &&
+	       rg->reservation_counter == org->reservation_counter &&
+	       rg->css == org->css;
+
+#else
+	return true;
+#endif
+}
+
+#ifdef CONFIG_DEBUG_VM
+static void dump_resv_map(struct resv_map *resv)
+{
+	struct list_head *head = &resv->regions;
+	struct file_region *rg = NULL;
+
+	pr_err("--------- start print resv_map ---------\n");
+	list_for_each_entry(rg, head, link) {
+		pr_err("rg->from=%ld, rg->to=%ld, rg->reservation_counter=%px, rg->css=%px\n",
+		       rg->from, rg->to, rg->reservation_counter, rg->css);
+	}
+	pr_err("--------- end print resv_map ---------\n");
+}
+
+/* Debug function to loop over the resv_map and make sure that coalescing is
+ * working.
+ */
+static void check_coalesce_bug(struct resv_map *resv)
+{
+	struct list_head *head = &resv->regions;
+	struct file_region *rg = NULL, *nrg = NULL;
+
+	list_for_each_entry(rg, head, link) {
+		nrg = list_next_entry(rg, link);
+
+		if (&nrg->link == head)
+			break;
+
+		if (nrg->reservation_counter && nrg->from == rg->to &&
+		    nrg->reservation_counter == rg->reservation_counter &&
+		    nrg->css == rg->css) {
+			dump_resv_map(resv);
+			VM_BUG_ON(true);
+		}
+	}
+}
+#endif
+
+static void coalesce_file_region(struct resv_map *resv, struct file_region *rg)
+{
+	struct file_region *nrg = NULL, *prg = NULL;
+
+	prg = list_prev_entry(rg, link);
+	if (&prg->link != &resv->regions && prg->to == rg->from &&
+	    has_same_uncharge_info(prg, rg)) {
+		prg->to = rg->to;
+
+		list_del(&rg->link);
+		kfree(rg);
+
+		coalesce_file_region(resv, prg);
+		return;
+	}
+
+	nrg = list_next_entry(rg, link);
+	if (&nrg->link != &resv->regions && nrg->from == rg->to &&
+	    has_same_uncharge_info(nrg, rg)) {
+		nrg->from = rg->from;
+
+		list_del(&rg->link);
+		kfree(rg);
+
+		coalesce_file_region(resv, nrg);
+		return;
+	}
+}
+
 /* Must be called with resv->lock held. Calling this with count_only == true
  * will count the number of pages to be added but will not modify the linked
  * list. If regions_needed != NULL and count_only == true, then regions_needed
@@ -327,6 +407,7 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,
 				record_hugetlb_cgroup_uncharge_info(h_cg, h,
 								    resv, nrg);
 				list_add(&nrg->link, rg->link.prev);
+				coalesce_file_region(resv, nrg);
 			} else if (regions_needed)
 				*regions_needed += 1;
 		}
@@ -344,11 +425,15 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,
 				resv, last_accounted_offset, t);
 			record_hugetlb_cgroup_uncharge_info(h_cg, h, resv, nrg);
 			list_add(&nrg->link, rg->link.prev);
+			coalesce_file_region(resv, nrg);
 		} else if (regions_needed)
 			*regions_needed += 1;
 	}

 	VM_BUG_ON(add < 0);
+#ifdef CONFIG_DEBUG_VM
+	check_coalesce_bug(resv);
+#endif
 	return add;
 }

--
2.25.0.225.g125e21ebc7-goog

WARNING: multiple messages have this Message-ID (diff)
From: Mina Almasry <almasrymina-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
To: mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org
Cc: shuah-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	almasrymina-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
	rientjes-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
	shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
	gthelen-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
	linux-kselftest-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: [PATCH v12 7/9] hugetlb: support file_region coalescing again
Date: Tue, 11 Feb 2020 13:31:26 -0800	[thread overview]
Message-ID: <20200211213128.73302-7-almasrymina@google.com> (raw)
In-Reply-To: <20200211213128.73302-1-almasrymina-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

An earlier patch in this series disabled file_region coalescing in order
to hang the hugetlb_cgroup uncharge info on the file_region entries.

This patch re-adds support for coalescing of file_region entries.
Essentially everytime we add an entry, we call a recursive function that
tries to coalesce the added region with the regions next to it. The
worst case call depth for this function is 3: one to coalesce with the
region next to it, one to coalesce to the region prev, and one to reach
the base case.

This is an important performance optimization as private mappings add
their entries page by page, and we could incur big performance costs for
large mappings with lots of file_region entries in their resv_map.

Signed-off-by: Mina Almasry <almasrymina-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

---

Changes in v12:
- Changed logic for coalescing. Instead of checking inline to coalesce
with only the region on next or prev, we now have a recursive function
that takes care of coalescing in both directions.
- For testing this code I added a bunch of debug code that checks that
the entries in the resv_map are coalesced appropriately. This passes
with libhugetlbfs tests.

---
 mm/hugetlb.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2d62dd35399db..45219cb58ac71 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -276,6 +276,86 @@ static void record_hugetlb_cgroup_uncharge_info(struct hugetlb_cgroup *h_cg,
 #endif
 }

+static bool has_same_uncharge_info(struct file_region *rg,
+				   struct file_region *org)
+{
+#ifdef CONFIG_CGROUP_HUGETLB
+	return rg && org &&
+	       rg->reservation_counter == org->reservation_counter &&
+	       rg->css == org->css;
+
+#else
+	return true;
+#endif
+}
+
+#ifdef CONFIG_DEBUG_VM
+static void dump_resv_map(struct resv_map *resv)
+{
+	struct list_head *head = &resv->regions;
+	struct file_region *rg = NULL;
+
+	pr_err("--------- start print resv_map ---------\n");
+	list_for_each_entry(rg, head, link) {
+		pr_err("rg->from=%ld, rg->to=%ld, rg->reservation_counter=%px, rg->css=%px\n",
+		       rg->from, rg->to, rg->reservation_counter, rg->css);
+	}
+	pr_err("--------- end print resv_map ---------\n");
+}
+
+/* Debug function to loop over the resv_map and make sure that coalescing is
+ * working.
+ */
+static void check_coalesce_bug(struct resv_map *resv)
+{
+	struct list_head *head = &resv->regions;
+	struct file_region *rg = NULL, *nrg = NULL;
+
+	list_for_each_entry(rg, head, link) {
+		nrg = list_next_entry(rg, link);
+
+		if (&nrg->link == head)
+			break;
+
+		if (nrg->reservation_counter && nrg->from == rg->to &&
+		    nrg->reservation_counter == rg->reservation_counter &&
+		    nrg->css == rg->css) {
+			dump_resv_map(resv);
+			VM_BUG_ON(true);
+		}
+	}
+}
+#endif
+
+static void coalesce_file_region(struct resv_map *resv, struct file_region *rg)
+{
+	struct file_region *nrg = NULL, *prg = NULL;
+
+	prg = list_prev_entry(rg, link);
+	if (&prg->link != &resv->regions && prg->to == rg->from &&
+	    has_same_uncharge_info(prg, rg)) {
+		prg->to = rg->to;
+
+		list_del(&rg->link);
+		kfree(rg);
+
+		coalesce_file_region(resv, prg);
+		return;
+	}
+
+	nrg = list_next_entry(rg, link);
+	if (&nrg->link != &resv->regions && nrg->from == rg->to &&
+	    has_same_uncharge_info(nrg, rg)) {
+		nrg->from = rg->from;
+
+		list_del(&rg->link);
+		kfree(rg);
+
+		coalesce_file_region(resv, nrg);
+		return;
+	}
+}
+
 /* Must be called with resv->lock held. Calling this with count_only == true
  * will count the number of pages to be added but will not modify the linked
  * list. If regions_needed != NULL and count_only == true, then regions_needed
@@ -327,6 +407,7 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,
 				record_hugetlb_cgroup_uncharge_info(h_cg, h,
 								    resv, nrg);
 				list_add(&nrg->link, rg->link.prev);
+				coalesce_file_region(resv, nrg);
 			} else if (regions_needed)
 				*regions_needed += 1;
 		}
@@ -344,11 +425,15 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,
 				resv, last_accounted_offset, t);
 			record_hugetlb_cgroup_uncharge_info(h_cg, h, resv, nrg);
 			list_add(&nrg->link, rg->link.prev);
+			coalesce_file_region(resv, nrg);
 		} else if (regions_needed)
 			*regions_needed += 1;
 	}

 	VM_BUG_ON(add < 0);
+#ifdef CONFIG_DEBUG_VM
+	check_coalesce_bug(resv);
+#endif
 	return add;
 }

  parent reply	other threads:[~2020-02-11 21:32 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-11 21:31 [PATCH v12 1/9] hugetlb_cgroup: Add hugetlb_cgroup reservation counter Mina Almasry
2020-02-11 21:31 ` Mina Almasry
2020-02-11 21:31 ` Mina Almasry
2020-02-11 21:31 ` [PATCH v12 2/9] hugetlb_cgroup: add interface for charge/uncharge hugetlb reservations Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-15  0:50   ` Mike Kravetz
2020-02-16  1:21     ` David Rientjes
2020-02-16  1:21       ` David Rientjes
2020-02-11 21:31 ` [PATCH v12 3/9] hugetlb_cgroup: add reservation accounting for private mappings Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-11 21:31 ` [PATCH v12 4/9] hugetlb: disable region_add file_region coalescing Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-15  1:27   ` Mike Kravetz
2020-02-15  1:27     ` Mike Kravetz
2020-02-16  1:25   ` David Rientjes
2020-02-16  1:25     ` David Rientjes
2020-02-16  1:25     ` David Rientjes
2020-02-11 21:31 ` [PATCH v12 5/9] hugetlb_cgroup: add accounting for shared mappings Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-16  1:29   ` David Rientjes
2020-02-16  1:29     ` David Rientjes
2020-02-18 18:07   ` Mike Kravetz
2020-02-18 18:07     ` Mike Kravetz
2020-02-11 21:31 ` [PATCH v12 6/9] hugetlb_cgroup: support noreserve mappings Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-18 20:57   ` Mike Kravetz
2020-02-18 20:57     ` Mike Kravetz
2020-02-11 21:31 ` Mina Almasry [this message]
2020-02-11 21:31   ` [PATCH v12 7/9] hugetlb: support file_region coalescing again Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-16  1:29   ` David Rientjes
2020-02-16  1:29     ` David Rientjes
2020-02-19  3:28   ` Mike Kravetz
2020-02-19  7:54     ` Mina Almasry
2020-02-19  7:54       ` Mina Almasry
2020-02-19 23:36     ` [PATCH] hugetlb: Remove check_coalesce_bug debug code Mina Almasry
2020-02-19 23:36       ` Mina Almasry
2020-02-20  0:07       ` Mike Kravetz
2020-02-11 21:31 ` [PATCH v12 8/9] hugetlb_cgroup: Add hugetlb_cgroup reservation tests Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-12  8:50   ` Sandipan Das
2020-02-20  0:05     ` Mina Almasry
2020-02-20  0:05       ` Mina Almasry
2020-02-20  0:05       ` Mina Almasry
2020-02-21  0:52   ` Mike Kravetz
2020-02-21  0:52     ` Mike Kravetz
2020-02-11 21:31 ` [PATCH v12 9/9] hugetlb_cgroup: Add hugetlb_cgroup reservation docs Mina Almasry
2020-02-11 21:31   ` Mina Almasry
2020-02-20  0:03   ` Mina Almasry
2020-02-20  0:03     ` Mina Almasry
2020-02-20  0:03     ` Mina Almasry
2020-02-20  0:18   ` Mike Kravetz
2020-02-20  0:18     ` Mike Kravetz
2020-02-11 23:19 ` [PATCH v12 1/9] hugetlb_cgroup: Add hugetlb_cgroup reservation counter Andrew Morton
2020-02-11 23:19   ` Andrew Morton
2020-02-18 14:21   ` Qian Cai
2020-02-18 14:21     ` Qian Cai
2020-02-18 14:21     ` Qian Cai
2020-02-18 18:35     ` Mina Almasry
2020-02-18 18:35       ` Mina Almasry
2020-02-18 18:35       ` Mina Almasry
2020-02-18 18:41       ` Qian Cai
2020-02-18 18:41         ` Qian Cai
2020-02-18 19:14       ` Mike Kravetz
2020-02-18 19:14         ` Mike Kravetz
2020-02-18 19:25         ` Mina Almasry
2020-02-18 19:25           ` Mina Almasry
2020-02-18 21:36           ` Mina Almasry
2020-02-18 21:36             ` Mina Almasry
2020-02-18 21:41             ` Mike Kravetz
2020-02-18 21:41               ` Mike Kravetz
2020-02-18 22:27               ` Mina Almasry
2020-02-18 22:27                 ` Mina Almasry
2020-02-18 22:27                 ` Mina Almasry
2020-02-19 19:05   ` Mina Almasry
2020-02-19 19:05     ` Mina Almasry
2020-02-19 21:06     ` Andrew Morton
2020-02-19 21:06       ` Andrew Morton
2020-02-20 19:22       ` Mina Almasry
2020-02-20 19:22         ` Mina Almasry
2020-02-21  0:28         ` Andrew Morton
2020-02-21  0:41           ` Mike Kravetz
2020-02-21  0:41             ` Mike Kravetz
2020-02-21  1:52             ` Mina Almasry
2020-02-21  1:52               ` Mina Almasry
2020-02-21  1:52               ` Mina Almasry
2020-02-21 20:19       ` Mina Almasry
2020-02-21 20:19         ` Mina Almasry
2020-02-21 20:19         ` Mina Almasry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200211213128.73302-7-almasrymina@google.com \
    --to=almasrymina@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=gthelen@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=rientjes@google.com \
    --cc=shakeelb@google.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.