[v1] mm/hwpoison: rename hwp_walk* to hwpoison_walk*

Message ID 20230713235553.4121855-1-jiaqiyan@google.com
State New
Headers
Series [v1] mm/hwpoison: rename hwp_walk* to hwpoison_walk* |

Commit Message

Jiaqi Yan July 13, 2023, 11:55 p.m. UTC
  In the discussion of "Improve hugetlbfs read on HWPOISON hugepages",
Matthew Wilcox suggests hwp is a bad abbreviation of hwpoison, as hwp
is already used as "an acronym by acpi, intel_pstate, some clock
drivers, an ethernet driver, and a scsi driver"[1].

So rename hwp_walk and hwp_walk_ops to hwpoison_walk and
hwpoison_walk_ops respectively.

raw_hwp_(page|list), *_raw_hwp, and raw_hwp_unreliable flag are other
major appearances of "hwp". However, given the "raw" hint in the name,
it is easy to differentiate them from other "hwp" acronyms. Since
renaming them is not as straightforward as renaming hwp_walk*, they
are not covered by this commit.

[1] https://lore.kernel.org/lkml/20230707201904.953262-5-jiaqiyan@google.com/T/#me6fecb8ce1ad4d5769199c9e162a44bc88f7bdec

Signed-off-by: Jiaqi Yan <jiaqiyan@google.com>
---
 mm/memory-failure.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
  

Comments

Anshuman Khandual July 14, 2023, 3:05 a.m. UTC | #1
On 7/14/23 05:25, Jiaqi Yan wrote:
> In the discussion of "Improve hugetlbfs read on HWPOISON hugepages",

A very small nit, [1] should appear here instead		   ^^^^

> Matthew Wilcox suggests hwp is a bad abbreviation of hwpoison, as hwp
> is already used as "an acronym by acpi, intel_pstate, some clock
> drivers, an ethernet driver, and a scsi driver"[1].

Some examples here might have been useful, but never mind.

> 
> So rename hwp_walk and hwp_walk_ops to hwpoison_walk and
> hwpoison_walk_ops respectively.
> 
> raw_hwp_(page|list), *_raw_hwp, and raw_hwp_unreliable flag are other
> major appearances of "hwp". However, given the "raw" hint in the name,
> it is easy to differentiate them from other "hwp" acronyms. Since
> renaming them is not as straightforward as renaming hwp_walk*, they
> are not covered by this commit.

Makes sense.

> 
> [1] https://lore.kernel.org/lkml/20230707201904.953262-5-jiaqiyan@google.com/T/#me6fecb8ce1ad4d5769199c9e162a44bc88f7bdec
> 
> Signed-off-by: Jiaqi Yan <jiaqiyan@google.com>
> ---
>  mm/memory-failure.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index e245191e6b04..cb232e41f6c0 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -721,7 +721,7 @@ static void collect_procs(struct page *page, struct list_head *tokill,
>  		collect_procs_file(page, tokill, force_early);
>  }
>  
> -struct hwp_walk {
> +struct hwpoison_walk {
>  	struct to_kill tk;
>  	unsigned long pfn;
>  	int flags;
> @@ -756,7 +756,7 @@ static int check_hwpoisoned_entry(pte_t pte, unsigned long addr, short shift,
>  
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  static int check_hwpoisoned_pmd_entry(pmd_t *pmdp, unsigned long addr,
> -				      struct hwp_walk *hwp)
> +				      struct hwpoison_walk *hwp)
>  {
>  	pmd_t pmd = *pmdp;
>  	unsigned long pfn;
> @@ -774,7 +774,7 @@ static int check_hwpoisoned_pmd_entry(pmd_t *pmdp, unsigned long addr,
>  }
>  #else
>  static int check_hwpoisoned_pmd_entry(pmd_t *pmdp, unsigned long addr,
> -				      struct hwp_walk *hwp)
> +				      struct hwpoison_walk *hwp)
>  {
>  	return 0;
>  }
> @@ -783,7 +783,7 @@ static int check_hwpoisoned_pmd_entry(pmd_t *pmdp, unsigned long addr,
>  static int hwpoison_pte_range(pmd_t *pmdp, unsigned long addr,
>  			      unsigned long end, struct mm_walk *walk)
>  {
> -	struct hwp_walk *hwp = walk->private;
> +	struct hwpoison_walk *hwp = walk->private;
>  	int ret = 0;
>  	pte_t *ptep, *mapped_pte;
>  	spinlock_t *ptl;
> @@ -817,7 +817,7 @@ static int hwpoison_hugetlb_range(pte_t *ptep, unsigned long hmask,
>  			    unsigned long addr, unsigned long end,
>  			    struct mm_walk *walk)
>  {
> -	struct hwp_walk *hwp = walk->private;
> +	struct hwpoison_walk *hwp = walk->private;
>  	pte_t pte = huge_ptep_get(ptep);
>  	struct hstate *h = hstate_vma(walk->vma);
>  
> @@ -828,7 +828,7 @@ static int hwpoison_hugetlb_range(pte_t *ptep, unsigned long hmask,
>  #define hwpoison_hugetlb_range	NULL
>  #endif
>  
> -static const struct mm_walk_ops hwp_walk_ops = {
> +static const struct mm_walk_ops hwpoison_walk_ops = {
>  	.pmd_entry = hwpoison_pte_range,
>  	.hugetlb_entry = hwpoison_hugetlb_range,
>  };
> @@ -850,7 +850,7 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn,
>  				  int flags)
>  {
>  	int ret;
> -	struct hwp_walk priv = {
> +	struct hwpoison_walk priv = {
>  		.pfn = pfn,
>  	};
>  	priv.tk.tsk = p;
> @@ -859,7 +859,7 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn,
>  		return -EFAULT;
>  
>  	mmap_read_lock(p->mm);
> -	ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwp_walk_ops,
> +	ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwpoison_walk_ops,
>  			      (void *)&priv);
>  	if (ret == 1 && priv.tk.addr)
>  		kill_proc(&priv.tk, pfn, flags);

This makes better sense, and the patch LGTM in itself. There are no residues
for "hwp_walk_ops" and "hwp_walk" symbols in the tree afterwards.

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
  
Naoya Horiguchi July 15, 2023, 4:07 a.m. UTC | #2
On Thu, Jul 13, 2023 at 11:55:53PM +0000, Jiaqi Yan wrote:
> In the discussion of "Improve hugetlbfs read on HWPOISON hugepages",
> Matthew Wilcox suggests hwp is a bad abbreviation of hwpoison, as hwp
> is already used as "an acronym by acpi, intel_pstate, some clock
> drivers, an ethernet driver, and a scsi driver"[1].
> 
> So rename hwp_walk and hwp_walk_ops to hwpoison_walk and
> hwpoison_walk_ops respectively.
> 
> raw_hwp_(page|list), *_raw_hwp, and raw_hwp_unreliable flag are other
> major appearances of "hwp". However, given the "raw" hint in the name,
> it is easy to differentiate them from other "hwp" acronyms. Since
> renaming them is not as straightforward as renaming hwp_walk*, they
> are not covered by this commit.
> 
> [1] https://lore.kernel.org/lkml/20230707201904.953262-5-jiaqiyan@google.com/T/#me6fecb8ce1ad4d5769199c9e162a44bc88f7bdec
> 
> Signed-off-by: Jiaqi Yan <jiaqiyan@google.com>

Looks good to me.  Thank you.

Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
  
Miaohe Lin July 17, 2023, 1:44 a.m. UTC | #3
On 2023/7/14 7:55, Jiaqi Yan wrote:
> In the discussion of "Improve hugetlbfs read on HWPOISON hugepages",
> Matthew Wilcox suggests hwp is a bad abbreviation of hwpoison, as hwp
> is already used as "an acronym by acpi, intel_pstate, some clock
> drivers, an ethernet driver, and a scsi driver"[1].
> 
> So rename hwp_walk and hwp_walk_ops to hwpoison_walk and
> hwpoison_walk_ops respectively.
> 
> raw_hwp_(page|list), *_raw_hwp, and raw_hwp_unreliable flag are other
> major appearances of "hwp". However, given the "raw" hint in the name,
> it is easy to differentiate them from other "hwp" acronyms. Since
> renaming them is not as straightforward as renaming hwp_walk*, they
> are not covered by this commit.
> 
> [1] https://lore.kernel.org/lkml/20230707201904.953262-5-jiaqiyan@google.com/T/#me6fecb8ce1ad4d5769199c9e162a44bc88f7bdec
> 
> Signed-off-by: Jiaqi Yan <jiaqiyan@google.com>

LGTM. Thanks.

Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
  

Patch

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index e245191e6b04..cb232e41f6c0 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -721,7 +721,7 @@  static void collect_procs(struct page *page, struct list_head *tokill,
 		collect_procs_file(page, tokill, force_early);
 }
 
-struct hwp_walk {
+struct hwpoison_walk {
 	struct to_kill tk;
 	unsigned long pfn;
 	int flags;
@@ -756,7 +756,7 @@  static int check_hwpoisoned_entry(pte_t pte, unsigned long addr, short shift,
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 static int check_hwpoisoned_pmd_entry(pmd_t *pmdp, unsigned long addr,
-				      struct hwp_walk *hwp)
+				      struct hwpoison_walk *hwp)
 {
 	pmd_t pmd = *pmdp;
 	unsigned long pfn;
@@ -774,7 +774,7 @@  static int check_hwpoisoned_pmd_entry(pmd_t *pmdp, unsigned long addr,
 }
 #else
 static int check_hwpoisoned_pmd_entry(pmd_t *pmdp, unsigned long addr,
-				      struct hwp_walk *hwp)
+				      struct hwpoison_walk *hwp)
 {
 	return 0;
 }
@@ -783,7 +783,7 @@  static int check_hwpoisoned_pmd_entry(pmd_t *pmdp, unsigned long addr,
 static int hwpoison_pte_range(pmd_t *pmdp, unsigned long addr,
 			      unsigned long end, struct mm_walk *walk)
 {
-	struct hwp_walk *hwp = walk->private;
+	struct hwpoison_walk *hwp = walk->private;
 	int ret = 0;
 	pte_t *ptep, *mapped_pte;
 	spinlock_t *ptl;
@@ -817,7 +817,7 @@  static int hwpoison_hugetlb_range(pte_t *ptep, unsigned long hmask,
 			    unsigned long addr, unsigned long end,
 			    struct mm_walk *walk)
 {
-	struct hwp_walk *hwp = walk->private;
+	struct hwpoison_walk *hwp = walk->private;
 	pte_t pte = huge_ptep_get(ptep);
 	struct hstate *h = hstate_vma(walk->vma);
 
@@ -828,7 +828,7 @@  static int hwpoison_hugetlb_range(pte_t *ptep, unsigned long hmask,
 #define hwpoison_hugetlb_range	NULL
 #endif
 
-static const struct mm_walk_ops hwp_walk_ops = {
+static const struct mm_walk_ops hwpoison_walk_ops = {
 	.pmd_entry = hwpoison_pte_range,
 	.hugetlb_entry = hwpoison_hugetlb_range,
 };
@@ -850,7 +850,7 @@  static int kill_accessing_process(struct task_struct *p, unsigned long pfn,
 				  int flags)
 {
 	int ret;
-	struct hwp_walk priv = {
+	struct hwpoison_walk priv = {
 		.pfn = pfn,
 	};
 	priv.tk.tsk = p;
@@ -859,7 +859,7 @@  static int kill_accessing_process(struct task_struct *p, unsigned long pfn,
 		return -EFAULT;
 
 	mmap_read_lock(p->mm);
-	ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwp_walk_ops,
+	ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwpoison_walk_ops,
 			      (void *)&priv);
 	if (ret == 1 && priv.tk.addr)
 		kill_proc(&priv.tk, pfn, flags);