Re: [PATCH] mm: fix the incorrect hugepages count

[Naoya Horiguchi]

On Tue, Aug 09, 2016 at 06:32:39PM +0800, zhong jiang wrote:
> On 2016/8/9 1:14, Mike Kravetz wrote:
> > On 08/07/2016 07:49 PM, zhongjiang wrote:
> >> From: zhong jiang 
> >>
> >> when memory hotplug enable, free hugepages will be freed if movable node 
> >> offline.
> >> therefore, /proc/sys/vm/nr_hugepages will be incorrect.

This sounds a bit odd to me because /proc/sys/vm/nr_hugepages returns
h->nr_huge_pages or h->nr_huge_pages_node[nid], which is already
considered in dissolve_free_huge_page (via update_and_free_page).

I think that h->max_huge_pages effectively means the pool size, and
h->nr_huge_pages means total hugepage number (which can be greater than
the pool size when there's overcommiting/surplus.)

dissolve_free_huge_page intends to break a hugepage into buddy, and
the destination hugepage is supposed to be allocated from the pool of
the destination node, so the system-wide pool size is reduced.
So adding h->max_huge_pages-- makes sense to me.

Acked-by: Naoya Horiguchi 

> >>
> >> The patch fix it by reduce the max_huge_pages when the node offline.
> >>
> >> Signed-off-by: zhong jiang 
> >> ---
> >>  mm/hugetlb.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> >> index f904246..3356e3a 100644
> >> --- a/mm/hugetlb.c
> >> +++ b/mm/hugetlb.c
> >> @@ -1448,6 +1448,7 @@ static void dissolve_free_huge_page(struct page 
> >> *page)
> >>list_del(>lru);
> >>h->free_huge_pages--;
> >>h->free_huge_pages_node[nid]--;
> >> +  h->max_huge_pages--;
> >>update_and_free_page(h, page);
> >>}
> >>spin_unlock(_lock);
> >>
> > Adding Naoya as he was the original author of this code.
> >
> > >From quick look it appears that the huge page will be migrated (allocated
> > on another node).  If my understanding is correct, then max_huge_pages
> > should not be adjusted here.
> >
>   we need to take free hugetlb pages into account.  of course, the allocated 
> huge pages is no
>   need to reduce.  The patch just reduce the free hugetlb pages count.

I

Re: [PATCH] mm: fix the incorrect hugepages count

[Naoya Horiguchi]

On Tue, Aug 09, 2016 at 06:32:39PM +0800, zhong jiang wrote:
> On 2016/8/9 1:14, Mike Kravetz wrote:
> > On 08/07/2016 07:49 PM, zhongjiang wrote:
> >> From: zhong jiang 
> >>
> >> when memory hotplug enable, free hugepages will be freed if movable node 
> >> offline.
> >> therefore, /proc/sys/vm/nr_hugepages will be incorrect.

This sounds a bit odd to me because /proc/sys/vm/nr_hugepages returns
h->nr_huge_pages or h->nr_huge_pages_node[nid], which is already
considered in dissolve_free_huge_page (via update_and_free_page).

I think that h->max_huge_pages effectively means the pool size, and
h->nr_huge_pages means total hugepage number (which can be greater than
the pool size when there's overcommiting/surplus.)

dissolve_free_huge_page intends to break a hugepage into buddy, and
the destination hugepage is supposed to be allocated from the pool of
the destination node, so the system-wide pool size is reduced.
So adding h->max_huge_pages-- makes sense to me.

Acked-by: Naoya Horiguchi 

> >>
> >> The patch fix it by reduce the max_huge_pages when the node offline.
> >>
> >> Signed-off-by: zhong jiang 
> >> ---
> >>  mm/hugetlb.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> >> index f904246..3356e3a 100644
> >> --- a/mm/hugetlb.c
> >> +++ b/mm/hugetlb.c
> >> @@ -1448,6 +1448,7 @@ static void dissolve_free_huge_page(struct page 
> >> *page)
> >>list_del(>lru);
> >>h->free_huge_pages--;
> >>h->free_huge_pages_node[nid]--;
> >> +  h->max_huge_pages--;
> >>update_and_free_page(h, page);
> >>}
> >>spin_unlock(_lock);
> >>
> > Adding Naoya as he was the original author of this code.
> >
> > >From quick look it appears that the huge page will be migrated (allocated
> > on another node).  If my understanding is correct, then max_huge_pages
> > should not be adjusted here.
> >
>   we need to take free hugetlb pages into account.  of course, the allocated 
> huge pages is no
>   need to reduce.  The patch just reduce the free hugetlb pages count.

I

Re: [PATCH] mm: fix the incorrect hugepages count

[zhong jiang]

On 2016/8/9 1:14, Mike Kravetz wrote:
> On 08/07/2016 07:49 PM, zhongjiang wrote:
>> From: zhong jiang 
>>
>> when memory hotplug enable, free hugepages will be freed if movable node 
>> offline.
>> therefore, /proc/sys/vm/nr_hugepages will be incorrect.
>>
>> The patch fix it by reduce the max_huge_pages when the node offline.
>>
>> Signed-off-by: zhong jiang 
>> ---
>>  mm/hugetlb.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index f904246..3356e3a 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1448,6 +1448,7 @@ static void dissolve_free_huge_page(struct page *page)
>>  list_del(>lru);
>>  h->free_huge_pages--;
>>  h->free_huge_pages_node[nid]--;
>> +h->max_huge_pages--;
>>  update_and_free_page(h, page);
>>  }
>>  spin_unlock(_lock);
>>
> Adding Naoya as he was the original author of this code.
>
> >From quick look it appears that the huge page will be migrated (allocated
> on another node).  If my understanding is correct, then max_huge_pages
> should not be adjusted here.
>
  we need to take free hugetlb pages into account.  of course, the allocated 
huge pages is no
  need to reduce.  The patch just reduce the free hugetlb pages count.

  Thanks
 zhongjiang

Re: [PATCH] mm: fix the incorrect hugepages count

[zhong jiang]

On 2016/8/9 1:14, Mike Kravetz wrote:
> On 08/07/2016 07:49 PM, zhongjiang wrote:
>> From: zhong jiang 
>>
>> when memory hotplug enable, free hugepages will be freed if movable node 
>> offline.
>> therefore, /proc/sys/vm/nr_hugepages will be incorrect.
>>
>> The patch fix it by reduce the max_huge_pages when the node offline.
>>
>> Signed-off-by: zhong jiang 
>> ---
>>  mm/hugetlb.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index f904246..3356e3a 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1448,6 +1448,7 @@ static void dissolve_free_huge_page(struct page *page)
>>  list_del(>lru);
>>  h->free_huge_pages--;
>>  h->free_huge_pages_node[nid]--;
>> +h->max_huge_pages--;
>>  update_and_free_page(h, page);
>>  }
>>  spin_unlock(_lock);
>>
> Adding Naoya as he was the original author of this code.
>
> >From quick look it appears that the huge page will be migrated (allocated
> on another node).  If my understanding is correct, then max_huge_pages
> should not be adjusted here.
>
  we need to take free hugetlb pages into account.  of course, the allocated 
huge pages is no
  need to reduce.  The patch just reduce the free hugetlb pages count.

  Thanks
 zhongjiang

Re: [PATCH] mm: fix the incorrect hugepages count

[Mike Kravetz]

On 08/07/2016 07:49 PM, zhongjiang wrote:
> From: zhong jiang 
> 
> when memory hotplug enable, free hugepages will be freed if movable node 
> offline.
> therefore, /proc/sys/vm/nr_hugepages will be incorrect.
> 
> The patch fix it by reduce the max_huge_pages when the node offline.
> 
> Signed-off-by: zhong jiang 
> ---
>  mm/hugetlb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index f904246..3356e3a 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1448,6 +1448,7 @@ static void dissolve_free_huge_page(struct page *page)
>   list_del(>lru);
>   h->free_huge_pages--;
>   h->free_huge_pages_node[nid]--;
> + h->max_huge_pages--;
>   update_and_free_page(h, page);
>   }
>   spin_unlock(_lock);
> 

Adding Naoya as he was the original author of this code.

>From quick look it appears that the huge page will be migrated (allocated
on another node).  If my understanding is correct, then max_huge_pages
should not be adjusted here.

-- 
Mike Kravetz

Re: [PATCH] mm: fix the incorrect hugepages count

[Mike Kravetz]

On 08/07/2016 07:49 PM, zhongjiang wrote:
> From: zhong jiang 
> 
> when memory hotplug enable, free hugepages will be freed if movable node 
> offline.
> therefore, /proc/sys/vm/nr_hugepages will be incorrect.
> 
> The patch fix it by reduce the max_huge_pages when the node offline.
> 
> Signed-off-by: zhong jiang 
> ---
>  mm/hugetlb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index f904246..3356e3a 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1448,6 +1448,7 @@ static void dissolve_free_huge_page(struct page *page)
>   list_del(>lru);
>   h->free_huge_pages--;
>   h->free_huge_pages_node[nid]--;
> + h->max_huge_pages--;
>   update_and_free_page(h, page);
>   }
>   spin_unlock(_lock);
> 

Adding Naoya as he was the original author of this code.

>From quick look it appears that the huge page will be migrated (allocated
on another node).  If my understanding is correct, then max_huge_pages
should not be adjusted here.

-- 
Mike Kravetz

[PATCH] mm: fix the incorrect hugepages count

[zhongjiang]

From: zhong jiang 

when memory hotplug enable, free hugepages will be freed if movable node 
offline.
therefore, /proc/sys/vm/nr_hugepages will be incorrect.

The patch fix it by reduce the max_huge_pages when the node offline.

Signed-off-by: zhong jiang 
---
 mm/hugetlb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f904246..3356e3a 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1448,6 +1448,7 @@ static void dissolve_free_huge_page(struct page *page)
list_del(>lru);
h->free_huge_pages--;
h->free_huge_pages_node[nid]--;
+   h->max_huge_pages--;
update_and_free_page(h, page);
}
spin_unlock(_lock);
-- 
1.8.3.1

[PATCH] mm: fix the incorrect hugepages count

[zhongjiang]

From: zhong jiang 

when memory hotplug enable, free hugepages will be freed if movable node 
offline.
therefore, /proc/sys/vm/nr_hugepages will be incorrect.

The patch fix it by reduce the max_huge_pages when the node offline.

Signed-off-by: zhong jiang 
---
 mm/hugetlb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f904246..3356e3a 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1448,6 +1448,7 @@ static void dissolve_free_huge_page(struct page *page)
list_del(>lru);
h->free_huge_pages--;
h->free_huge_pages_node[nid]--;
+   h->max_huge_pages--;
update_and_free_page(h, page);
}
spin_unlock(_lock);
-- 
1.8.3.1