Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1664616 > unrolled thread

[RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask

Started byMichal Hocko <mhocko@kernel.org>
First post2017-06-13 11:10 +0200
Last post2017-06-15 10:20 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask Michal Hocko <mhocko@kernel.org> - 2017-06-13 11:10 +0200
    Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to  alloc_huge_page_nodemask Vlastimil Babka <vbabka@suse.cz> - 2017-06-14 18:20 +0200
      Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to  alloc_huge_page_nodemask Michal Hocko <mhocko@kernel.org> - 2017-06-14 18:50 +0200
        Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to  alloc_huge_page_nodemask Vlastimil Babka <vbabka@suse.cz> - 2017-06-14 19:00 +0200
    Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to  alloc_huge_page_nodemask Mike Kravetz <mike.kravetz@oracle.com> - 2017-06-15 00:20 +0200
      Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to  alloc_huge_page_nodemask Mike Kravetz <mike.kravetz@oracle.com> - 2017-06-15 02:20 +0200
        Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to  alloc_huge_page_nodemask Michal Hocko <mhocko@kernel.org> - 2017-06-15 10:20 +0200

#1664616 — [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask

FromMichal Hocko <mhocko@kernel.org>
Date2017-06-13 11:10 +0200
Subject[RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask
Message-ID<tRPZL-65q-9@gated-at.bofh.it>
From: Michal Hocko <mhocko@suse.com>

alloc_huge_page_nodemask tries to allocate from any numa node in the
allowed node mask starting from lower numa nodes. This might lead to
filling up those low NUMA nodes while others are not used. We can reduce
this risk by introducing a concept of the preferred node similar to what
we have in the regular page allocator. We will start allocating from the
preferred nid and then iterate over all allowed nodes in the zonelist
order until we try them all.

This is mimicking the page allocator logic except it operates on
per-node mempools. dequeue_huge_page_vma already does this so distill
the zonelist logic into a more generic dequeue_huge_page_nodemask
and use it in alloc_huge_page_nodemask.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/hugetlb.h |   3 +-
 include/linux/migrate.h |   2 +-
 mm/hugetlb.c            | 106 +++++++++++++++++++++++++-----------------------
 3 files changed, 59 insertions(+), 52 deletions(-)

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 016831fcdca1..d4c33a8583be 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -349,7 +349,8 @@ struct page *alloc_huge_page(struct vm_area_struct *vma,
 struct page *alloc_huge_page_node(struct hstate *h, int nid);
 struct page *alloc_huge_page_noerr(struct vm_area_struct *vma,
 				unsigned long addr, int avoid_reserve);
-struct page *alloc_huge_page_nodemask(struct hstate *h, nodemask_t *nmask);
+struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
+				nodemask_t *nmask);
 int huge_add_to_page_cache(struct page *page, struct address_space *mapping,
 			pgoff_t idx);
 
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index f80c9882403a..af3ccf93efaa 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -38,7 +38,7 @@ static inline struct page *new_page_nodemask(struct page *page, int preferred_ni
 
 	if (PageHuge(page))
 		return alloc_huge_page_nodemask(page_hstate(compound_head(page)),
-				nodemask);
+				preferred_nid, nodemask);
 
 	if (PageHighMem(page)
 	    || (zone_idx(page_zone(page)) == ZONE_MOVABLE))
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 3d5f25d589b3..696de029f0fa 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -897,29 +897,58 @@ static struct page *dequeue_huge_page_node_exact(struct hstate *h, int nid)
 	return page;
 }
 
-static struct page *dequeue_huge_page_node(struct hstate *h, int nid)
+/* Movability of hugepages depends on migration support. */
+static inline gfp_t htlb_alloc_mask(struct hstate *h)
 {
-	struct page *page;
-	int node;
+	if (hugepages_treat_as_movable || hugepage_migration_supported(h))
+		return GFP_HIGHUSER_MOVABLE;
+	else
+		return GFP_HIGHUSER;
+}
 
-	if (nid != NUMA_NO_NODE)
-		return dequeue_huge_page_node_exact(h, nid);
+static struct page *dequeue_huge_page_nodemask(struct hstate *h, int nid,
+		nodemask_t *nmask)
+{
+	unsigned int cpuset_mems_cookie;
+	struct zonelist *zonelist;
+	struct page *page = NULL;
+	struct zone *zone;
+	struct zoneref *z;
+	gfp_t gfp_mask;
+	int node = -1;
+
+	gfp_mask = htlb_alloc_mask(h);
+	zonelist = node_zonelist(nid, gfp_mask);
+
+retry_cpuset:
+	cpuset_mems_cookie = read_mems_allowed_begin();
+	for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp_mask), nmask) {
+		if (!cpuset_zone_allowed(zone, gfp_mask))
+			continue;
+		/*
+		 * no need to ask again on the same node. Pool is node rather than
+		 * zone aware
+		 */
+		if (zone_to_nid(zone) == node)
+			continue;
+		node = zone_to_nid(zone);
 
-	for_each_online_node(node) {
 		page = dequeue_huge_page_node_exact(h, node);
 		if (page)
-			return page;
+			break;
 	}
+	if (unlikely(!page && read_mems_allowed_retry(cpuset_mems_cookie)))
+		goto retry_cpuset;
+
 	return NULL;
 }
 
-/* Movability of hugepages depends on migration support. */
-static inline gfp_t htlb_alloc_mask(struct hstate *h)
+static struct page *dequeue_huge_page_node(struct hstate *h, int nid)
 {
-	if (hugepages_treat_as_movable || hugepage_migration_supported(h))
-		return GFP_HIGHUSER_MOVABLE;
-	else
-		return GFP_HIGHUSER;
+	if (nid != NUMA_NO_NODE)
+		return dequeue_huge_page_node_exact(h, nid);
+
+	return dequeue_huge_page_nodemask(h, nid, NULL);
 }
 
 static struct page *dequeue_huge_page_vma(struct hstate *h,
@@ -927,15 +956,10 @@ static struct page *dequeue_huge_page_vma(struct hstate *h,
 				unsigned long address, int avoid_reserve,
 				long chg)
 {
-	struct page *page = NULL;
+	struct page *page;
 	struct mempolicy *mpol;
 	nodemask_t *nodemask;
-	gfp_t gfp_mask;
 	int nid;
-	struct zonelist *zonelist;
-	struct zone *zone;
-	struct zoneref *z;
-	unsigned int cpuset_mems_cookie;
 
 	/*
 	 * A child process with MAP_PRIVATE mappings created by their parent
@@ -950,32 +974,14 @@ static struct page *dequeue_huge_page_vma(struct hstate *h,
 	if (avoid_reserve && h->free_huge_pages - h->resv_huge_pages == 0)
 		goto err;
 
-retry_cpuset:
-	cpuset_mems_cookie = read_mems_allowed_begin();
-	gfp_mask = htlb_alloc_mask(h);
-	nid = huge_node(vma, address, gfp_mask, &mpol, &nodemask);
-	zonelist = node_zonelist(nid, gfp_mask);
-
-	for_each_zone_zonelist_nodemask(zone, z, zonelist,
-						MAX_NR_ZONES - 1, nodemask) {
-		if (cpuset_zone_allowed(zone, gfp_mask)) {
-			page = dequeue_huge_page_node(h, zone_to_nid(zone));
-			if (page) {
-				if (avoid_reserve)
-					break;
-				if (!vma_has_reserves(vma, chg))
-					break;
-
-				SetPagePrivate(page);
-				h->resv_huge_pages--;
-				break;
-			}
-		}
+	nid = huge_node(vma, address, htlb_alloc_mask(h), &mpol, &nodemask);
+	page = dequeue_huge_page_nodemask(h, nid, nodemask);
+	if (page && !avoid_reserve && vma_has_reserves(vma, chg)) {
+		SetPagePrivate(page);
+		h->resv_huge_pages--;
 	}
 
 	mpol_cond_put(mpol);
-	if (unlikely(!page && read_mems_allowed_retry(cpuset_mems_cookie)))
-		goto retry_cpuset;
 	return page;
 
 err:
@@ -1655,25 +1661,25 @@ struct page *alloc_huge_page_node(struct hstate *h, int nid)
 	return page;
 }
 
-struct page *alloc_huge_page_nodemask(struct hstate *h, nodemask_t *nmask)
+
+struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
+		nodemask_t *nmask)
 {
 	struct page *page = NULL;
-	int node;
 
 	spin_lock(&hugetlb_lock);
 	if (h->free_huge_pages - h->resv_huge_pages > 0) {
-		for_each_node_mask(node, *nmask) {
-			page = dequeue_huge_page_node_exact(h, node);
-			if (page)
-				break;
-		}
+		page = dequeue_huge_page_nodemask(h, preferred_nid, nmask);
+		if (page)
+			goto unlock;
 	}
+unlock:
 	spin_unlock(&hugetlb_lock);
 	if (page)
 		return page;
 
 	/* No reservations, try to overcommit */
-	return __alloc_buddy_huge_page(h, NUMA_NO_NODE, nmask);
+	return __alloc_buddy_huge_page(h, preferred_nid, nmask);
 }
 
 /*
-- 
2.11.0

[toc] | [next] | [standalone]


#1666038 — Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask

FromVlastimil Babka <vbabka@suse.cz>
Date2017-06-14 18:20 +0200
SubjectRe: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask
Message-ID<tSjbs-7qT-27@gated-at.bofh.it>
In reply to#1664616
On 06/13/2017 11:00 AM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> alloc_huge_page_nodemask tries to allocate from any numa node in the
> allowed node mask starting from lower numa nodes. This might lead to
> filling up those low NUMA nodes while others are not used. We can reduce
> this risk by introducing a concept of the preferred node similar to what
> we have in the regular page allocator. We will start allocating from the
> preferred nid and then iterate over all allowed nodes in the zonelist
> order until we try them all.
> 
> This is mimicking the page allocator logic except it operates on
> per-node mempools. dequeue_huge_page_vma already does this so distill
> the zonelist logic into a more generic dequeue_huge_page_nodemask
> and use it in alloc_huge_page_nodemask.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>

I've reviewed the current version in git, where patch 3/4 is folded.

Noticed some things below, but after fixing:
Acked-by: Vlastimil Babka <vbabka@suse.cz>


> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -897,29 +897,58 @@ static struct page *dequeue_huge_page_node_exact(struct hstate *h, int nid)
>  	return page;
>  }
>  
> -static struct page *dequeue_huge_page_node(struct hstate *h, int nid)
> +/* Movability of hugepages depends on migration support. */
> +static inline gfp_t htlb_alloc_mask(struct hstate *h)
>  {
> -	struct page *page;
> -	int node;
> +	if (hugepages_treat_as_movable || hugepage_migration_supported(h))
> +		return GFP_HIGHUSER_MOVABLE;
> +	else
> +		return GFP_HIGHUSER;
> +}
>  
> -	if (nid != NUMA_NO_NODE)
> -		return dequeue_huge_page_node_exact(h, nid);
> +static struct page *dequeue_huge_page_nodemask(struct hstate *h, int nid,
> +		nodemask_t *nmask)
> +{
> +	unsigned int cpuset_mems_cookie;
> +	struct zonelist *zonelist;
> +	struct page *page = NULL;
> +	struct zone *zone;
> +	struct zoneref *z;
> +	gfp_t gfp_mask;
> +	int node = -1;
> +
> +	gfp_mask = htlb_alloc_mask(h);
> +	zonelist = node_zonelist(nid, gfp_mask);
> +
> +retry_cpuset:
> +	cpuset_mems_cookie = read_mems_allowed_begin();
> +	for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp_mask), nmask) {
> +		if (!cpuset_zone_allowed(zone, gfp_mask))
> +			continue;
> +		/*
> +		 * no need to ask again on the same node. Pool is node rather than
> +		 * zone aware
> +		 */
> +		if (zone_to_nid(zone) == node)
> +			continue;
> +		node = zone_to_nid(zone);
>  
> -	for_each_online_node(node) {
>  		page = dequeue_huge_page_node_exact(h, node);
>  		if (page)
> -			return page;
> +			break;

Either keep return page here...

>  	}
> +	if (unlikely(!page && read_mems_allowed_retry(cpuset_mems_cookie)))
> +		goto retry_cpuset;
> +
>  	return NULL;

... or return page here.

>  }
>  
> -/* Movability of hugepages depends on migration support. */
> -static inline gfp_t htlb_alloc_mask(struct hstate *h)
> +static struct page *dequeue_huge_page_node(struct hstate *h, int nid)
>  {
> -	if (hugepages_treat_as_movable || hugepage_migration_supported(h))
> -		return GFP_HIGHUSER_MOVABLE;
> -	else
> -		return GFP_HIGHUSER;
> +	if (nid != NUMA_NO_NODE)
> +		return dequeue_huge_page_node_exact(h, nid);
> +
> +	return dequeue_huge_page_nodemask(h, nid, NULL);
>  }
>  

...

> @@ -1655,25 +1661,25 @@ struct page *alloc_huge_page_node(struct hstate *h, int nid)
>  	return page;
>  }
>  
> -struct page *alloc_huge_page_nodemask(struct hstate *h, nodemask_t *nmask)
> +
> +struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
> +		nodemask_t *nmask)
>  {
>  	struct page *page = NULL;
> -	int node;
>  
>  	spin_lock(&hugetlb_lock);
>  	if (h->free_huge_pages - h->resv_huge_pages > 0) {
> -		for_each_node_mask(node, *nmask) {
> -			page = dequeue_huge_page_node_exact(h, node);
> -			if (page)
> -				break;
> -		}
> +		page = dequeue_huge_page_nodemask(h, preferred_nid, nmask);



> +		if (page)
> +			goto unlock;
>  	}
> +unlock:

This doesn't seem needed?

>  	spin_unlock(&hugetlb_lock);
>  	if (page)
>  		return page;
>  
>  	/* No reservations, try to overcommit */
> -	return __alloc_buddy_huge_page(h, NUMA_NO_NODE, nmask);
> +	return __alloc_buddy_huge_page(h, preferred_nid, nmask);
>  }
>  
>  /*
> 

[toc] | [prev] | [next] | [standalone]


#1666064 — Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask

FromMichal Hocko <mhocko@kernel.org>
Date2017-06-14 18:50 +0200
SubjectRe: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask
Message-ID<tSjEt-7AG-7@gated-at.bofh.it>
In reply to#1666038
On Wed 14-06-17 18:17:18, Vlastimil Babka wrote:
> On 06/13/2017 11:00 AM, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@suse.com>
> > 
> > alloc_huge_page_nodemask tries to allocate from any numa node in the
> > allowed node mask starting from lower numa nodes. This might lead to
> > filling up those low NUMA nodes while others are not used. We can reduce
> > this risk by introducing a concept of the preferred node similar to what
> > we have in the regular page allocator. We will start allocating from the
> > preferred nid and then iterate over all allowed nodes in the zonelist
> > order until we try them all.
> > 
> > This is mimicking the page allocator logic except it operates on
> > per-node mempools. dequeue_huge_page_vma already does this so distill
> > the zonelist logic into a more generic dequeue_huge_page_nodemask
> > and use it in alloc_huge_page_nodemask.
> > 
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> 
> I've reviewed the current version in git, where patch 3/4 is folded.
> 
> Noticed some things below, but after fixing:
> Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!

[...]
> > +retry_cpuset:
> > +	cpuset_mems_cookie = read_mems_allowed_begin();
> > +	for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp_mask), nmask) {
> > +		if (!cpuset_zone_allowed(zone, gfp_mask))
> > +			continue;
> > +		/*
> > +		 * no need to ask again on the same node. Pool is node rather than
> > +		 * zone aware
> > +		 */
> > +		if (zone_to_nid(zone) == node)
> > +			continue;
> > +		node = zone_to_nid(zone);
> >  
> > -	for_each_online_node(node) {
> >  		page = dequeue_huge_page_node_exact(h, node);
> >  		if (page)
> > -			return page;
> > +			break;
> 
> Either keep return page here...
> 
> >  	}
> > +	if (unlikely(!page && read_mems_allowed_retry(cpuset_mems_cookie)))
> > +		goto retry_cpuset;
> > +
> >  	return NULL;
> 
> ... or return page here.

ups I went with the former.

[...]

> > -struct page *alloc_huge_page_nodemask(struct hstate *h, nodemask_t *nmask)
> > +
> > +struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
> > +		nodemask_t *nmask)
> >  {
> >  	struct page *page = NULL;
> > -	int node;
> >  
> >  	spin_lock(&hugetlb_lock);
> >  	if (h->free_huge_pages - h->resv_huge_pages > 0) {
> > -		for_each_node_mask(node, *nmask) {
> > -			page = dequeue_huge_page_node_exact(h, node);
> > -			if (page)
> > -				break;
> > -		}
> > +		page = dequeue_huge_page_nodemask(h, preferred_nid, nmask);
> 
> 
> 
> > +		if (page)
> > +			goto unlock;
> >  	}
> > +unlock:
> 
> This doesn't seem needed?

This on top?
---
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 9ac0ae725c5e..f9868e095afa 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -902,7 +902,6 @@ static struct page *dequeue_huge_page_nodemask(struct hstate *h, gfp_t gfp_mask,
 {
 	unsigned int cpuset_mems_cookie;
 	struct zonelist *zonelist;
-	struct page *page = NULL;
 	struct zone *zone;
 	struct zoneref *z;
 	int node = -1;
@@ -912,6 +911,8 @@ static struct page *dequeue_huge_page_nodemask(struct hstate *h, gfp_t gfp_mask,
 retry_cpuset:
 	cpuset_mems_cookie = read_mems_allowed_begin();
 	for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp_mask), nmask) {
+		struct page *page;
+
 		if (!cpuset_zone_allowed(zone, gfp_mask))
 			continue;
 		/*
@@ -924,9 +925,9 @@ static struct page *dequeue_huge_page_nodemask(struct hstate *h, gfp_t gfp_mask,
 
 		page = dequeue_huge_page_node_exact(h, node);
 		if (page)
-			break;
+			return page;
 	}
-	if (unlikely(!page && read_mems_allowed_retry(cpuset_mems_cookie)))
+	if (unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
 		goto retry_cpuset;
 
 	return NULL;
@@ -1655,18 +1656,18 @@ struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
 		nodemask_t *nmask)
 {
 	gfp_t gfp_mask = htlb_alloc_mask(h);
-	struct page *page = NULL;
 
 	spin_lock(&hugetlb_lock);
 	if (h->free_huge_pages - h->resv_huge_pages > 0) {
+		struct page *page;
+
 		page = dequeue_huge_page_nodemask(h, gfp_mask, preferred_nid, nmask);
-		if (page)
-			goto unlock;
+		if (page) {
+			spin_unlock(&hugetlb_lock);
+			return page;
+		}
 	}
-unlock:
 	spin_unlock(&hugetlb_lock);
-	if (page)
-		return page;
 
 	/* No reservations, try to overcommit */
 
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1666071 — Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask

FromVlastimil Babka <vbabka@suse.cz>
Date2017-06-14 19:00 +0200
SubjectRe: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask
Message-ID<tSjO9-7DO-5@gated-at.bofh.it>
In reply to#1666064
On 06/14/2017 06:41 PM, Michal Hocko wrote:
> 
> This on top?
> ---
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 9ac0ae725c5e..f9868e095afa 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -902,7 +902,6 @@ static struct page *dequeue_huge_page_nodemask(struct hstate *h, gfp_t gfp_mask,
>  {
>  	unsigned int cpuset_mems_cookie;
>  	struct zonelist *zonelist;
> -	struct page *page = NULL;
>  	struct zone *zone;
>  	struct zoneref *z;
>  	int node = -1;
> @@ -912,6 +911,8 @@ static struct page *dequeue_huge_page_nodemask(struct hstate *h, gfp_t gfp_mask,
>  retry_cpuset:
>  	cpuset_mems_cookie = read_mems_allowed_begin();
>  	for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp_mask), nmask) {
> +		struct page *page;
> +
>  		if (!cpuset_zone_allowed(zone, gfp_mask))
>  			continue;
>  		/*
> @@ -924,9 +925,9 @@ static struct page *dequeue_huge_page_nodemask(struct hstate *h, gfp_t gfp_mask,
>  
>  		page = dequeue_huge_page_node_exact(h, node);
>  		if (page)
> -			break;
> +			return page;
>  	}
> -	if (unlikely(!page && read_mems_allowed_retry(cpuset_mems_cookie)))
> +	if (unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
>  		goto retry_cpuset;
>  
>  	return NULL;

OK

> @@ -1655,18 +1656,18 @@ struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
>  		nodemask_t *nmask)
>  {
>  	gfp_t gfp_mask = htlb_alloc_mask(h);
> -	struct page *page = NULL;
>  
>  	spin_lock(&hugetlb_lock);
>  	if (h->free_huge_pages - h->resv_huge_pages > 0) {
> +		struct page *page;
> +
>  		page = dequeue_huge_page_nodemask(h, gfp_mask, preferred_nid, nmask);
> -		if (page)
> -			goto unlock;
> +		if (page) {
> +			spin_unlock(&hugetlb_lock);
> +			return page;
> +		}

I thought you would just continue after the if (this is not a for-loop
after all), but this works too.

>  	}
> -unlock:
>  	spin_unlock(&hugetlb_lock);
> -	if (page)
> -		return page;
>  
>  	/* No reservations, try to overcommit */
>  
> 

[toc] | [prev] | [next] | [standalone]


#1666263 — Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask

FromMike Kravetz <mike.kravetz@oracle.com>
Date2017-06-15 00:20 +0200
SubjectRe: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask
Message-ID<tSoNP-2uY-9@gated-at.bofh.it>
In reply to#1664616
On 06/13/2017 02:00 AM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> alloc_huge_page_nodemask tries to allocate from any numa node in the
> allowed node mask starting from lower numa nodes. This might lead to
> filling up those low NUMA nodes while others are not used. We can reduce
> this risk by introducing a concept of the preferred node similar to what
> we have in the regular page allocator. We will start allocating from the
> preferred nid and then iterate over all allowed nodes in the zonelist
> order until we try them all.
> 
> This is mimicking the page allocator logic except it operates on
> per-node mempools. dequeue_huge_page_vma already does this so distill
> the zonelist logic into a more generic dequeue_huge_page_nodemask
> and use it in alloc_huge_page_nodemask.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---


I built attempts/hugetlb-zonelists, threw it on a test machine, ran the
libhugetlbfs test suite and saw failures.  The failures started with this
patch: commit 7e8b09f14495 in your tree.  I have not yet started to look
into the failures.  It is even possible that the tests are making bad
assumptions, but there certainly appears to be changes in behavior visible
to the application(s).

FYI - My 'test machine' is an x86 KVM insatnce with 8GB memory simulating
2 nodes.  Huge page allocations before running tests:
node0
512	free_hugepages
512	nr_hugepages
0	surplus_hugepages
node1
512	free_hugepages
512	nr_hugepages
0	surplus_hugepages

I can take a closer look at the failures tomorrow.
-- 
Mike Kravetz

[toc] | [prev] | [next] | [standalone]


#1666324 — Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask

FromMike Kravetz <mike.kravetz@oracle.com>
Date2017-06-15 02:20 +0200
SubjectRe: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask
Message-ID<tSqFY-3Fp-7@gated-at.bofh.it>
In reply to#1666263
On 06/14/2017 03:12 PM, Mike Kravetz wrote:
> On 06/13/2017 02:00 AM, Michal Hocko wrote:
>> From: Michal Hocko <mhocko@suse.com>
>>
>> alloc_huge_page_nodemask tries to allocate from any numa node in the
>> allowed node mask starting from lower numa nodes. This might lead to
>> filling up those low NUMA nodes while others are not used. We can reduce
>> this risk by introducing a concept of the preferred node similar to what
>> we have in the regular page allocator. We will start allocating from the
>> preferred nid and then iterate over all allowed nodes in the zonelist
>> order until we try them all.
>>
>> This is mimicking the page allocator logic except it operates on
>> per-node mempools. dequeue_huge_page_vma already does this so distill
>> the zonelist logic into a more generic dequeue_huge_page_nodemask
>> and use it in alloc_huge_page_nodemask.
>>
>> Signed-off-by: Michal Hocko <mhocko@suse.com>
>> ---
> 
> 
> I built attempts/hugetlb-zonelists, threw it on a test machine, ran the
> libhugetlbfs test suite and saw failures.  The failures started with this
> patch: commit 7e8b09f14495 in your tree.  I have not yet started to look
> into the failures.  It is even possible that the tests are making bad
> assumptions, but there certainly appears to be changes in behavior visible
> to the application(s).

nm.  The failures were the result of dequeue_huge_page_nodemask() always
returning NULL.  Vlastimil already noticed this issue and provided a
solution.

-- 
Mike Kravetz

> 
> FYI - My 'test machine' is an x86 KVM insatnce with 8GB memory simulating
> 2 nodes.  Huge page allocations before running tests:
> node0
> 512	free_hugepages
> 512	nr_hugepages
> 0	surplus_hugepages
> node1
> 512	free_hugepages
> 512	nr_hugepages
> 0	surplus_hugepages
> 
> I can take a closer look at the failures tomorrow.
> 

[toc] | [prev] | [next] | [standalone]


#1666547 — Re: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask

FromMichal Hocko <mhocko@kernel.org>
Date2017-06-15 10:20 +0200
SubjectRe: [RFC PATCH 2/4] hugetlb: add support for preferred node to alloc_huge_page_nodemask
Message-ID<tSyat-8uk-5@gated-at.bofh.it>
In reply to#1666324
On Wed 14-06-17 17:12:31, Mike Kravetz wrote:
> On 06/14/2017 03:12 PM, Mike Kravetz wrote:
> > On 06/13/2017 02:00 AM, Michal Hocko wrote:
> >> From: Michal Hocko <mhocko@suse.com>
> >>
> >> alloc_huge_page_nodemask tries to allocate from any numa node in the
> >> allowed node mask starting from lower numa nodes. This might lead to
> >> filling up those low NUMA nodes while others are not used. We can reduce
> >> this risk by introducing a concept of the preferred node similar to what
> >> we have in the regular page allocator. We will start allocating from the
> >> preferred nid and then iterate over all allowed nodes in the zonelist
> >> order until we try them all.
> >>
> >> This is mimicking the page allocator logic except it operates on
> >> per-node mempools. dequeue_huge_page_vma already does this so distill
> >> the zonelist logic into a more generic dequeue_huge_page_nodemask
> >> and use it in alloc_huge_page_nodemask.
> >>
> >> Signed-off-by: Michal Hocko <mhocko@suse.com>
> >> ---
> > 
> > 
> > I built attempts/hugetlb-zonelists, threw it on a test machine, ran the
> > libhugetlbfs test suite and saw failures.  The failures started with this
> > patch: commit 7e8b09f14495 in your tree.  I have not yet started to look
> > into the failures.  It is even possible that the tests are making bad
> > assumptions, but there certainly appears to be changes in behavior visible
> > to the application(s).
> 
> nm.  The failures were the result of dequeue_huge_page_nodemask() always
> returning NULL.  Vlastimil already noticed this issue and provided a
> solution.

I have pushed my current version to the same branch.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web