[PATCH 0/4] Some cleanups and improvements for hugetlb

[PATCH 0/4] Some cleanups and improvements for hugetlb

[Baolin Wang]

Hi,

This patchset did some cleanups and improvements for the hugetlb and
hugetlb_cgroup. Please help to review. Thanks.

Baolin Wang (4):
  hugetlb_cgroup: Remove unused hugetlb_cgroup_from_counter macro
  hugetlb: Replace the obsolete hugetlb_instantiation_mutex in the
    comments
  hugetlb: Remove redundant validation in has_same_uncharge_info()
  hugetlb: Remove redundant VM_BUG_ON() in add_reservation_in_range()

 mm/hugetlb.c        | 6 ++----
 mm/hugetlb_cgroup.c | 3 ---
 2 files changed, 2 insertions(+), 7 deletions(-)

-- 
1.8.3.1

[PATCH 1/4] hugetlb_cgroup: Remove unused hugetlb_cgroup_from_counter macro

[Baolin Wang]

Since commit 726b7bbe ("hugetlb_cgroup: fix illegal access to memory"),
the hugetlb_cgroup_from_counter() macro is not used any more, remove it.

Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/hugetlb_cgroup.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
index 5383023..79d9353 100644
--- a/mm/hugetlb_cgroup.c
+++ b/mm/hugetlb_cgroup.c
@@ -27,9 +27,6 @@
 #define MEMFILE_IDX(val)	(((val) >> 16) & 0xffff)
 #define MEMFILE_ATTR(val)	((val) & 0xffff)
 
-#define hugetlb_cgroup_from_counter(counter, idx)                   \
-	container_of(counter, struct hugetlb_cgroup, hugepage[idx])
-
 static struct hugetlb_cgroup *root_h_cgroup __read_mostly;
 
 static inline struct page_counter *
-- 
1.8.3.1

Re: [PATCH 1/4] hugetlb_cgroup: Remove unused hugetlb_cgroup_from_counter macro

[Mike Kravetz]

On 10/20/21 11:31 PM, Baolin Wang wrote:
> Since commit 726b7bbe ("hugetlb_cgroup: fix illegal access to memory"),
> the hugetlb_cgroup_from_counter() macro is not used any more, remove it.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
>  mm/hugetlb_cgroup.c | 3 ---
>  1 file changed, 3 deletions(-)

Thanks!

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

> 
> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> index 5383023..79d9353 100644
> --- a/mm/hugetlb_cgroup.c
> +++ b/mm/hugetlb_cgroup.c
> @@ -27,9 +27,6 @@
>  #define MEMFILE_IDX(val)	(((val) >> 16) & 0xffff)
>  #define MEMFILE_ATTR(val)	((val) & 0xffff)
>  
> -#define hugetlb_cgroup_from_counter(counter, idx)                   \
> -	container_of(counter, struct hugetlb_cgroup, hugepage[idx])
> -
>  static struct hugetlb_cgroup *root_h_cgroup __read_mostly;
>  
>  static inline struct page_counter *
> 

[PATCH 2/4] hugetlb: Replace the obsolete hugetlb_instantiation_mutex in the comments

[Baolin Wang]

After commit 8382d914ebf7 ("mm, hugetlb: improve page-fault scalability"),
the hugetlb_instantiation_mutex lock had been replaced by
hugetlb_fault_mutex_table to serializes faults on the same logical page.
Thus update the obsolete hugetlb_instantiation_mutex related comments.

Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/hugetlb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index ee0db48..8902b07 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5012,7 +5012,7 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
 
 /*
  * Hugetlb_cow() should be called with page lock of the original hugepage held.
- * Called with hugetlb_instantiation_mutex held and pte_page locked so we
+ * Called with hugetlb_fault_mutex_table held and pte_page locked so we
  * cannot race with other handlers or page migration.
  * Keep the pte_same checks anyway to make transition from the mutex easier.
  */
-- 
1.8.3.1

Re: [PATCH 2/4] hugetlb: Replace the obsolete hugetlb_instantiation_mutex in the comments

[Mike Kravetz]

On 10/20/21 11:31 PM, Baolin Wang wrote:
> After commit 8382d914ebf7 ("mm, hugetlb: improve page-fault scalability"),
> the hugetlb_instantiation_mutex lock had been replaced by
> hugetlb_fault_mutex_table to serializes faults on the same logical page.
> Thus update the obsolete hugetlb_instantiation_mutex related comments.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
>  mm/hugetlb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks!

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

-- 
Mike Kravetz

> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index ee0db48..8902b07 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -5012,7 +5012,7 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
>  
>  /*
>   * Hugetlb_cow() should be called with page lock of the original hugepage held.
> - * Called with hugetlb_instantiation_mutex held and pte_page locked so we
> + * Called with hugetlb_fault_mutex_table held and pte_page locked so we
>   * cannot race with other handlers or page migration.
>   * Keep the pte_same checks anyway to make transition from the mutex easier.
>   */
> 

[PATCH 3/4] hugetlb: Remove redundant validation in has_same_uncharge_info()

[Baolin Wang]

The callers of has_same_uncharge_info() has accessed the original file_region
and new file_region, and they are impossible to be NULL now. So we can remove
the file_region validation in has_same_uncharge_info() to simplify the code.

Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/hugetlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 8902b07..5922629 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -333,8 +333,7 @@ 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 &&
+	return rg->reservation_counter == org->reservation_counter &&
 	       rg->css == org->css;
 
 #else
-- 
1.8.3.1

Re: [PATCH 3/4] hugetlb: Remove redundant validation in has_same_uncharge_info()

[Mike Kravetz]

On 10/20/21 11:31 PM, Baolin Wang wrote:
> The callers of has_same_uncharge_info() has accessed the original file_region
> and new file_region, and they are impossible to be NULL now. So we can remove
> the file_region validation in has_same_uncharge_info() to simplify the code.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
>  mm/hugetlb.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Thanks!

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

-- 
Mike Kravetz

> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 8902b07..5922629 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -333,8 +333,7 @@ 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 &&
> +	return rg->reservation_counter == org->reservation_counter &&
>  	       rg->css == org->css;
>  
>  #else
> 

[PATCH 4/4] hugetlb: Remove redundant VM_BUG_ON() in add_reservation_in_range()

[Baolin Wang]

When calling hugetlb_resv_map_add(), we've guaranteed that the parameter
'to' is always larger than 'from', so it never returns a negative value
from hugetlb_resv_map_add(). Thus remove the redundant VM_BUG_ON().

Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/hugetlb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 5922629..166cfa1 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -446,7 +446,6 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,
 		add += hugetlb_resv_map_add(resv, rg, last_accounted_offset,
 					    t, h, h_cg, regions_needed);
 
-	VM_BUG_ON(add < 0);
 	return add;
 }
 
-- 
1.8.3.1

Re: [PATCH 4/4] hugetlb: Remove redundant VM_BUG_ON() in add_reservation_in_range()

[Mike Kravetz]

On 10/20/21 11:31 PM, Baolin Wang wrote:
> When calling hugetlb_resv_map_add(), we've guaranteed that the parameter
> 'to' is always larger than 'from', so it never returns a negative value
> from hugetlb_resv_map_add(). Thus remove the redundant VM_BUG_ON().
> 
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
>  mm/hugetlb.c | 1 -
>  1 file changed, 1 deletion(-)

Thanks!  You are correct, we could never trigger that VM_BUG_ON.

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

-- 
Mike Kravetz

> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 5922629..166cfa1 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -446,7 +446,6 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,
>  		add += hugetlb_resv_map_add(resv, rg, last_accounted_offset,
>  					    t, h, h_cg, regions_needed);
>  
> -	VM_BUG_ON(add < 0);
>  	return add;
>  }
>  
>