mm/hugetlb: use helper huge_page_size() to get hugepage size
diff mbox series

Message ID 20210208082450.15716-1-linmiaohe@huawei.com
State In Next
Commit 6a5e114358e3adb5766bf0304fe127031cdde1af
Headers show
Series
  • mm/hugetlb: use helper huge_page_size() to get hugepage size
Related show

Commit Message

Miaohe Lin Feb. 8, 2021, 8:24 a.m. UTC
We can use helper huge_page_size() to get the hugepage size directly to
simplify the code slightly.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/hugetlb.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

Comments

Mike Kravetz Feb. 9, 2021, 12:45 a.m. UTC | #1
On 2/8/21 12:24 AM, Miaohe Lin wrote:
> We can use helper huge_page_size() to get the hugepage size directly to
> simplify the code slightly.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/hugetlb.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 18628f8dbfb0..6cdb59d8f663 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3199,7 +3199,7 @@ void __init hugetlb_add_hstate(unsigned int order)
>  	BUG_ON(order == 0);
>  	h = &hstates[hugetlb_max_hstate++];
>  	h->order = order;
> -	h->mask = ~((1ULL << (order + PAGE_SHIFT)) - 1);
> +	h->mask = ~(huge_page_size(h) - 1);
>  	for (i = 0; i < MAX_NUMNODES; ++i)
>  		INIT_LIST_HEAD(&h->hugepage_freelists[i]);
>  	INIT_LIST_HEAD(&h->hugepage_activelist);
> @@ -3474,7 +3474,7 @@ void hugetlb_report_meminfo(struct seq_file *m)
>  	for_each_hstate(h) {
>  		unsigned long count = h->nr_huge_pages;
>  
> -		total += (PAGE_SIZE << huge_page_order(h)) * count;
> +		total += huge_page_size(h) * count;
>  
>  		if (h == &default_hstate)
>  			seq_printf(m,
> @@ -3487,10 +3487,10 @@ void hugetlb_report_meminfo(struct seq_file *m)
>  				   h->free_huge_pages,
>  				   h->resv_huge_pages,
>  				   h->surplus_huge_pages,
> -				   (PAGE_SIZE << huge_page_order(h)) / 1024);
> +				   huge_page_size(h) / SZ_1K);
>  	}
>  
> -	seq_printf(m, "Hugetlb:        %8lu kB\n", total / 1024);
> +	seq_printf(m, "Hugetlb:        %8lu kB\n", total / SZ_1K);
>  }
>  
>  int hugetlb_report_node_meminfo(char *buf, int len, int nid)
> @@ -3524,7 +3524,7 @@ void hugetlb_show_meminfo(void)
>  				h->nr_huge_pages_node[nid],
>  				h->free_huge_pages_node[nid],
>  				h->surplus_huge_pages_node[nid],
> -				1UL << (huge_page_order(h) + PAGE_SHIFT - 10));
> +				huge_page_size(h) >> 10);

Should we change this to

				huge_page_size(h) / SZ_1K);

as in hugetlb_report_meminfo above?  Or, is that one where it takes an
additional instruction to do the divide as opposed to the shift?  I would
rather add the instruction and keep everything consistent.
Miaohe Lin Feb. 9, 2021, 1:24 a.m. UTC | #2
Hi:
On 2021/2/9 8:45, Mike Kravetz wrote:
> On 2/8/21 12:24 AM, Miaohe Lin wrote:
>> We can use helper huge_page_size() to get the hugepage size directly to
>> simplify the code slightly.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>  mm/hugetlb.c | 14 ++++++--------
>>  1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 18628f8dbfb0..6cdb59d8f663 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -3199,7 +3199,7 @@ void __init hugetlb_add_hstate(unsigned int order)
>>  	BUG_ON(order == 0);
>>  	h = &hstates[hugetlb_max_hstate++];
>>  	h->order = order;
>> -	h->mask = ~((1ULL << (order + PAGE_SHIFT)) - 1);
>> +	h->mask = ~(huge_page_size(h) - 1);
>>  	for (i = 0; i < MAX_NUMNODES; ++i)
>>  		INIT_LIST_HEAD(&h->hugepage_freelists[i]);
>>  	INIT_LIST_HEAD(&h->hugepage_activelist);
>> @@ -3474,7 +3474,7 @@ void hugetlb_report_meminfo(struct seq_file *m)
>>  	for_each_hstate(h) {
>>  		unsigned long count = h->nr_huge_pages;
>>  
>> -		total += (PAGE_SIZE << huge_page_order(h)) * count;
>> +		total += huge_page_size(h) * count;
>>  
>>  		if (h == &default_hstate)
>>  			seq_printf(m,
>> @@ -3487,10 +3487,10 @@ void hugetlb_report_meminfo(struct seq_file *m)
>>  				   h->free_huge_pages,
>>  				   h->resv_huge_pages,
>>  				   h->surplus_huge_pages,
>> -				   (PAGE_SIZE << huge_page_order(h)) / 1024);
>> +				   huge_page_size(h) / SZ_1K);
>>  	}
>>  
>> -	seq_printf(m, "Hugetlb:        %8lu kB\n", total / 1024);
>> +	seq_printf(m, "Hugetlb:        %8lu kB\n", total / SZ_1K);
>>  }
>>  
>>  int hugetlb_report_node_meminfo(char *buf, int len, int nid)
>> @@ -3524,7 +3524,7 @@ void hugetlb_show_meminfo(void)
>>  				h->nr_huge_pages_node[nid],
>>  				h->free_huge_pages_node[nid],
>>  				h->surplus_huge_pages_node[nid],
>> -				1UL << (huge_page_order(h) + PAGE_SHIFT - 10));
>> +				huge_page_size(h) >> 10);
> 
> Should we change this to
> 
> 				huge_page_size(h) / SZ_1K);
> > as in hugetlb_report_meminfo above?  Or, is that one where it takes an
> additional instruction to do the divide as opposed to the shift?  I would> rather add the instruction and keep everything consistent.
> 

Yes, it takes an additional instruction to do the divide as opposed to the shift. So I did not
change this. But it seems keeping everything consistent in a function is more important. So should
I send a V2 to change this or Andrew would kindly handle this ?

Many thanks.
Mike Kravetz Feb. 9, 2021, 1:36 a.m. UTC | #3
On 2/8/21 5:24 PM, Miaohe Lin wrote:
> Hi:
> On 2021/2/9 8:45, Mike Kravetz wrote:
>> On 2/8/21 12:24 AM, Miaohe Lin wrote:
>>> We can use helper huge_page_size() to get the hugepage size directly to
>>> simplify the code slightly.
>>>
>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>>> ---
>>>  mm/hugetlb.c | 14 ++++++--------
>>>  1 file changed, 6 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>> index 18628f8dbfb0..6cdb59d8f663 100644
>>> --- a/mm/hugetlb.c
>>> +++ b/mm/hugetlb.c
>>> @@ -3199,7 +3199,7 @@ void __init hugetlb_add_hstate(unsigned int order)
>>>  	BUG_ON(order == 0);
>>>  	h = &hstates[hugetlb_max_hstate++];
>>>  	h->order = order;
>>> -	h->mask = ~((1ULL << (order + PAGE_SHIFT)) - 1);
>>> +	h->mask = ~(huge_page_size(h) - 1);
>>>  	for (i = 0; i < MAX_NUMNODES; ++i)
>>>  		INIT_LIST_HEAD(&h->hugepage_freelists[i]);
>>>  	INIT_LIST_HEAD(&h->hugepage_activelist);
>>> @@ -3474,7 +3474,7 @@ void hugetlb_report_meminfo(struct seq_file *m)
>>>  	for_each_hstate(h) {
>>>  		unsigned long count = h->nr_huge_pages;
>>>  
>>> -		total += (PAGE_SIZE << huge_page_order(h)) * count;
>>> +		total += huge_page_size(h) * count;
>>>  
>>>  		if (h == &default_hstate)
>>>  			seq_printf(m,
>>> @@ -3487,10 +3487,10 @@ void hugetlb_report_meminfo(struct seq_file *m)
>>>  				   h->free_huge_pages,
>>>  				   h->resv_huge_pages,
>>>  				   h->surplus_huge_pages,
>>> -				   (PAGE_SIZE << huge_page_order(h)) / 1024);
>>> +				   huge_page_size(h) / SZ_1K);
>>>  	}
>>>  
>>> -	seq_printf(m, "Hugetlb:        %8lu kB\n", total / 1024);
>>> +	seq_printf(m, "Hugetlb:        %8lu kB\n", total / SZ_1K);
>>>  }
>>>  
>>>  int hugetlb_report_node_meminfo(char *buf, int len, int nid)
>>> @@ -3524,7 +3524,7 @@ void hugetlb_show_meminfo(void)
>>>  				h->nr_huge_pages_node[nid],
>>>  				h->free_huge_pages_node[nid],
>>>  				h->surplus_huge_pages_node[nid],
>>> -				1UL << (huge_page_order(h) + PAGE_SHIFT - 10));
>>> +				huge_page_size(h) >> 10);
>>
>> Should we change this to
>>
>> 				huge_page_size(h) / SZ_1K);
>>> as in hugetlb_report_meminfo above?  Or, is that one where it takes an
>> additional instruction to do the divide as opposed to the shift?  I would> rather add the instruction and keep everything consistent.
>>
> 
> Yes, it takes an additional instruction to do the divide as opposed to the shift. So I did not
> change this. But it seems keeping everything consistent in a function is more important. So should
> I send a V2 to change this or Andrew would kindly handle this ?

I would go ahead and put together a v2 and let Andrew decide how he wants
to handle it.  You can include,

Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>

Thanks
Miaohe Lin Feb. 9, 2021, 1:57 a.m. UTC | #4
On 2021/2/9 9:36, Mike Kravetz wrote:
> On 2/8/21 5:24 PM, Miaohe Lin wrote:
>> Hi:
>> On 2021/2/9 8:45, Mike Kravetz wrote:
>>> On 2/8/21 12:24 AM, Miaohe Lin wrote:
>>>> We can use helper huge_page_size() to get the hugepage size directly to
>>>> simplify the code slightly.
>>>>
>>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>>>> ---
>>>>  mm/hugetlb.c | 14 ++++++--------
>>>>  1 file changed, 6 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>>> index 18628f8dbfb0..6cdb59d8f663 100644
>>>> --- a/mm/hugetlb.c
>>>> +++ b/mm/hugetlb.c
>>>> @@ -3199,7 +3199,7 @@ void __init hugetlb_add_hstate(unsigned int order)
>>>>  	BUG_ON(order == 0);
>>>>  	h = &hstates[hugetlb_max_hstate++];
>>>>  	h->order = order;
>>>> -	h->mask = ~((1ULL << (order + PAGE_SHIFT)) - 1);
>>>> +	h->mask = ~(huge_page_size(h) - 1);
>>>>  	for (i = 0; i < MAX_NUMNODES; ++i)
>>>>  		INIT_LIST_HEAD(&h->hugepage_freelists[i]);
>>>>  	INIT_LIST_HEAD(&h->hugepage_activelist);
>>>> @@ -3474,7 +3474,7 @@ void hugetlb_report_meminfo(struct seq_file *m)
>>>>  	for_each_hstate(h) {
>>>>  		unsigned long count = h->nr_huge_pages;
>>>>  
>>>> -		total += (PAGE_SIZE << huge_page_order(h)) * count;
>>>> +		total += huge_page_size(h) * count;
>>>>  
>>>>  		if (h == &default_hstate)
>>>>  			seq_printf(m,
>>>> @@ -3487,10 +3487,10 @@ void hugetlb_report_meminfo(struct seq_file *m)
>>>>  				   h->free_huge_pages,
>>>>  				   h->resv_huge_pages,
>>>>  				   h->surplus_huge_pages,
>>>> -				   (PAGE_SIZE << huge_page_order(h)) / 1024);
>>>> +				   huge_page_size(h) / SZ_1K);
>>>>  	}
>>>>  
>>>> -	seq_printf(m, "Hugetlb:        %8lu kB\n", total / 1024);
>>>> +	seq_printf(m, "Hugetlb:        %8lu kB\n", total / SZ_1K);
>>>>  }
>>>>  
>>>>  int hugetlb_report_node_meminfo(char *buf, int len, int nid)
>>>> @@ -3524,7 +3524,7 @@ void hugetlb_show_meminfo(void)
>>>>  				h->nr_huge_pages_node[nid],
>>>>  				h->free_huge_pages_node[nid],
>>>>  				h->surplus_huge_pages_node[nid],
>>>> -				1UL << (huge_page_order(h) + PAGE_SHIFT - 10));
>>>> +				huge_page_size(h) >> 10);
>>>
>>> Should we change this to
>>>
>>> 				huge_page_size(h) / SZ_1K);
>>>> as in hugetlb_report_meminfo above?  Or, is that one where it takes an
>>> additional instruction to do the divide as opposed to the shift?  I would> rather add the instruction and keep everything consistent.
>>>
>>
>> Yes, it takes an additional instruction to do the divide as opposed to the shift. So I did not
>> change this. But it seems keeping everything consistent in a function is more important. So should
>> I send a V2 to change this or Andrew would kindly handle this ?
> 
> I would go ahead and put together a v2 and let Andrew decide how he wants
> to handle it.  You can include,
> 

Will do. Thanks a lot.

> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> 
> Thanks
>

Patch
diff mbox series

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 18628f8dbfb0..6cdb59d8f663 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3199,7 +3199,7 @@  void __init hugetlb_add_hstate(unsigned int order)
 	BUG_ON(order == 0);
 	h = &hstates[hugetlb_max_hstate++];
 	h->order = order;
-	h->mask = ~((1ULL << (order + PAGE_SHIFT)) - 1);
+	h->mask = ~(huge_page_size(h) - 1);
 	for (i = 0; i < MAX_NUMNODES; ++i)
 		INIT_LIST_HEAD(&h->hugepage_freelists[i]);
 	INIT_LIST_HEAD(&h->hugepage_activelist);
@@ -3474,7 +3474,7 @@  void hugetlb_report_meminfo(struct seq_file *m)
 	for_each_hstate(h) {
 		unsigned long count = h->nr_huge_pages;
 
-		total += (PAGE_SIZE << huge_page_order(h)) * count;
+		total += huge_page_size(h) * count;
 
 		if (h == &default_hstate)
 			seq_printf(m,
@@ -3487,10 +3487,10 @@  void hugetlb_report_meminfo(struct seq_file *m)
 				   h->free_huge_pages,
 				   h->resv_huge_pages,
 				   h->surplus_huge_pages,
-				   (PAGE_SIZE << huge_page_order(h)) / 1024);
+				   huge_page_size(h) / SZ_1K);
 	}
 
-	seq_printf(m, "Hugetlb:        %8lu kB\n", total / 1024);
+	seq_printf(m, "Hugetlb:        %8lu kB\n", total / SZ_1K);
 }
 
 int hugetlb_report_node_meminfo(char *buf, int len, int nid)
@@ -3524,7 +3524,7 @@  void hugetlb_show_meminfo(void)
 				h->nr_huge_pages_node[nid],
 				h->free_huge_pages_node[nid],
 				h->surplus_huge_pages_node[nid],
-				1UL << (huge_page_order(h) + PAGE_SHIFT - 10));
+				huge_page_size(h) >> 10);
 }
 
 void hugetlb_report_usage(struct seq_file *m, struct mm_struct *mm)
@@ -3647,9 +3647,7 @@  static int hugetlb_vm_op_split(struct vm_area_struct *vma, unsigned long addr)
 
 static unsigned long hugetlb_vm_op_pagesize(struct vm_area_struct *vma)
 {
-	struct hstate *hstate = hstate_vma(vma);
-
-	return 1UL << huge_page_shift(hstate);
+	return huge_page_size(hstate_vma(vma));
 }
 
 /*