[PATCH v2 00/13] Some cleanups for memory-failure

[PATCH v2 00/13] Some cleanups for memory-failure

[Miaohe Lin]

Hi everyone,
This series contains a few cleanup patches to simplify unneeded function
variant, add helper macro, fix some obsolete comments and so on. More
details can be found in the respective changelogs.
Thanks!

Miaohe Lin (13):
  mm/memory-failure: simplify put_ref_page()
  mm/memory-failure: remove MF_MSG_SLAB
  mm/memory-failure: add macro GET_PAGE_MAX_RETRY_NUM
  mm/memory-failure: save some page_folio() calls
  mm/memory-failure: remove unneeded empty string
  mm/memory-failure: remove confusing initialization to count
  mm/memory-failure: simplify unneeded hwpoison_filter() variant
  mm/memory-failure: use helper macro task_pid_nr()
  mm/memory-failure: remove obsolete comment in unpoison_memory()
  mm/memory-failure: move some function declarations into internal.h
  mm/memory-failure: fix comment of get_hwpoison_page()
  mm/memory-failure: remove obsolete comment in kill_proc()
  mm/memory-failure: correct comment in me_swapcache_dirty

 include/linux/mm.h         |  8 -------
 include/linux/page-flags.h |  5 ----
 include/linux/rmap.h       |  2 --
 include/ras/ras_event.h    |  1 -
 mm/internal.h              | 10 ++++++++
 mm/memory-failure.c        | 48 +++++++++++++++-----------------------
 6 files changed, 29 insertions(+), 45 deletions(-)


base-commit: 19b8422c5bd56fb5e7085995801c6543a98bda1f
-- 
2.33.0

[PATCH v2 01/13] mm/memory-failure: simplify put_ref_page()

[Miaohe Lin]

Remove unneeded page != NULL check. pfn_to_page() won't return NULL.
No functional change intended.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index f679b579d45d..2e6038c73119 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2120,14 +2120,10 @@ static inline unsigned long folio_free_raw_hwp(struct folio *folio, bool flag)
 /* Drop the extra refcount in case we come from madvise() */
 static void put_ref_page(unsigned long pfn, int flags)
 {
-	struct page *page;
-
 	if (!(flags & MF_COUNT_INCREASED))
 		return;
 
-	page = pfn_to_page(pfn);
-	if (page)
-		put_page(page);
+	put_page(pfn_to_page(pfn));
 }
 
 static int memory_failure_dev_pagemap(unsigned long pfn, int flags,
-- 
2.33.0

[PATCH v2 02/13] mm/memory-failure: remove MF_MSG_SLAB

[Miaohe Lin]

Since commit 46df8e73a4a3 ("mm: free up PG_slab"), MF_MSG_SLAB becomes
unused. Remove it. No functional change intended.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/mm.h      | 1 -
 include/ras/ras_event.h | 1 -
 mm/memory-failure.c     | 1 -
 3 files changed, 3 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 04795a509026..ec7141fb0252 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4078,7 +4078,6 @@ enum mf_result {
 enum mf_action_page_type {
 	MF_MSG_KERNEL,
 	MF_MSG_KERNEL_HIGH_ORDER,
-	MF_MSG_SLAB,
 	MF_MSG_DIFFERENT_COMPOUND,
 	MF_MSG_HUGE,
 	MF_MSG_FREE_HUGE,
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index cf7f19b7ce64..9bc707fe8819 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -356,7 +356,6 @@ TRACE_EVENT(aer_event,
 #define MF_PAGE_TYPE		\
 	EM ( MF_MSG_KERNEL, "reserved kernel page" )			\
 	EM ( MF_MSG_KERNEL_HIGH_ORDER, "high-order kernel page" )	\
-	EM ( MF_MSG_SLAB, "kernel slab page" )				\
 	EM ( MF_MSG_DIFFERENT_COMPOUND, "different compound page after locking" ) \
 	EM ( MF_MSG_HUGE, "huge page" )					\
 	EM ( MF_MSG_FREE_HUGE, "free huge page" )			\
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 2e6038c73119..4b9a9298d478 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -911,7 +911,6 @@ static const char *action_name[] = {
 static const char * const action_page_types[] = {
 	[MF_MSG_KERNEL]			= "reserved kernel page",
 	[MF_MSG_KERNEL_HIGH_ORDER]	= "high-order kernel page",
-	[MF_MSG_SLAB]			= "kernel slab page",
 	[MF_MSG_DIFFERENT_COMPOUND]	= "different compound page after locking",
 	[MF_MSG_HUGE]			= "huge page",
 	[MF_MSG_FREE_HUGE]		= "free huge page",
-- 
2.33.0

[PATCH v2 03/13] mm/memory-failure: add macro GET_PAGE_MAX_RETRY_NUM

[Miaohe Lin]

Add helper macro GET_PAGE_MAX_RETRY_NUM to replace magic number 3.
No functional change intended.

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

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 4b9a9298d478..958b17a4b0f5 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1417,6 +1417,8 @@ static int __get_hwpoison_page(struct page *page, unsigned long flags)
 	return 0;
 }
 
+#define GET_PAGE_MAX_RETRY_NUM 3
+
 static int get_any_page(struct page *p, unsigned long flags)
 {
 	int ret = 0, pass = 0;
@@ -1431,12 +1433,12 @@ static int get_any_page(struct page *p, unsigned long flags)
 		if (!ret) {
 			if (page_count(p)) {
 				/* We raced with an allocation, retry. */
-				if (pass++ < 3)
+				if (pass++ < GET_PAGE_MAX_RETRY_NUM)
 					goto try_again;
 				ret = -EBUSY;
 			} else if (!PageHuge(p) && !is_free_buddy_page(p)) {
 				/* We raced with put_page, retry. */
-				if (pass++ < 3)
+				if (pass++ < GET_PAGE_MAX_RETRY_NUM)
 					goto try_again;
 				ret = -EIO;
 			}
@@ -1462,7 +1464,7 @@ static int get_any_page(struct page *p, unsigned long flags)
 		 * A page we cannot handle. Check whether we can turn
 		 * it into something we can handle.
 		 */
-		if (pass++ < 3) {
+		if (pass++ < GET_PAGE_MAX_RETRY_NUM) {
 			put_page(p);
 			shake_page(p);
 			count_increased = false;
-- 
2.33.0

[PATCH v2 04/13] mm/memory-failure: save some page_folio() calls

[Miaohe Lin]

Use local variable folio directly to save a page_folio() call. Also
use folio_mapped() to save more page_folio() calls. No functional
change intended.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 mm/memory-failure.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 958b17a4b0f5..470c570d779c 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1575,7 +1575,7 @@ static bool hwpoison_user_mappings(struct folio *folio, struct page *p,
 	 * This check implies we don't kill processes if their pages
 	 * are in the swap cache early. Those are always late kills.
 	 */
-	if (!page_mapped(p))
+	if (!folio_mapped(folio))
 		return true;
 
 	if (folio_test_swapcache(folio)) {
@@ -1626,10 +1626,10 @@ static bool hwpoison_user_mappings(struct folio *folio, struct page *p,
 		try_to_unmap(folio, ttu);
 	}
 
-	unmap_success = !page_mapped(p);
+	unmap_success = !folio_mapped(folio);
 	if (!unmap_success)
 		pr_err("%#lx: failed to unmap page (folio mapcount=%d)\n",
-		       pfn, folio_mapcount(page_folio(p)));
+		       pfn, folio_mapcount(folio));
 
 	/*
 	 * try_to_unmap() might put mlocked page in lru cache, so call
-- 
2.33.0

[PATCH v2 05/13] mm/memory-failure: remove unneeded empty string

[Miaohe Lin]

Remove unneeded empty string in definition of macro pr_fmt. No functional
change intended.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 470c570d779c..ec482524158e 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2518,7 +2518,7 @@ static int __init memory_failure_init(void)
 core_initcall(memory_failure_init);
 
 #undef pr_fmt
-#define pr_fmt(fmt)	"" fmt
+#define pr_fmt(fmt)	fmt
 #define unpoison_pr_info(fmt, pfn, rs)			\
 ({							\
 	if (__ratelimit(rs))				\
-- 
2.33.0

[PATCH v2 06/13] mm/memory-failure: remove confusing initialization to count

[Miaohe Lin]

It's meaningless and confusing to init local variable count to 1.
Remove it. No functional change intended.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index ec482524158e..339752d768d8 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2542,7 +2542,7 @@ int unpoison_memory(unsigned long pfn)
 	struct folio *folio;
 	struct page *p;
 	int ret = -EBUSY, ghp;
-	unsigned long count = 1;
+	unsigned long count;
 	bool huge = false;
 	static DEFINE_RATELIMIT_STATE(unpoison_rs, DEFAULT_RATELIMIT_INTERVAL,
 					DEFAULT_RATELIMIT_BURST);
-- 
2.33.0

[PATCH v2 07/13] mm/memory-failure: simplify unneeded hwpoison_filter() variant

[Miaohe Lin]

When CONFIG_HWPOISON_INJECT is not enabled, there is no user of the
hwpoison_filter() outside memory-failure. So there is no need to export
it and we can further make it 'static' and 'inline'.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202405242336.o1NEOrln-lkp@intel.com/
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 339752d768d8..6e26b8b975e5 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -294,15 +294,14 @@ int hwpoison_filter(struct page *p)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(hwpoison_filter);
 #else
-int hwpoison_filter(struct page *p)
+static inline int hwpoison_filter(struct page *p)
 {
 	return 0;
 }
 #endif
 
-EXPORT_SYMBOL_GPL(hwpoison_filter);
-
 /*
  * Kill all processes that have a poisoned page mapped and then isolate
  * the page.
-- 
2.33.0

[PATCH v2 08/13] mm/memory-failure: use helper macro task_pid_nr()

[Miaohe Lin]

Use helper macro task_pid_nr() to get the pid of a task. No functional
change intended.

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

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 6e26b8b975e5..81a544df8381 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -343,7 +343,7 @@ static int kill_proc(struct to_kill *tk, unsigned long pfn, int flags)
 	int ret = 0;
 
 	pr_err("%#lx: Sending SIGBUS to %s:%d due to hardware memory corruption\n",
-			pfn, t->comm, t->pid);
+			pfn, t->comm, task_pid_nr(t));
 
 	if ((flags & MF_ACTION_REQUIRED) && (t == current))
 		ret = force_sig_mceerr(BUS_MCEERR_AR,
@@ -361,7 +361,7 @@ static int kill_proc(struct to_kill *tk, unsigned long pfn, int flags)
 				      addr_lsb, t);
 	if (ret < 0)
 		pr_info("Error sending signal to %s:%d: %d\n",
-			t->comm, t->pid, ret);
+			t->comm, task_pid_nr(t), ret);
 	return ret;
 }
 
@@ -523,7 +523,7 @@ static void kill_procs(struct list_head *to_kill, int forcekill,
 		if (forcekill) {
 			if (tk->addr == -EFAULT) {
 				pr_err("%#lx: forcibly killing %s:%d because of failure to unmap corrupted page\n",
-				       pfn, tk->tsk->comm, tk->tsk->pid);
+				       pfn, tk->tsk->comm, task_pid_nr(tk->tsk));
 				do_send_sig_info(SIGKILL, SEND_SIG_PRIV,
 						 tk->tsk, PIDTYPE_PID);
 			}
@@ -536,7 +536,7 @@ static void kill_procs(struct list_head *to_kill, int forcekill,
 			 */
 			else if (kill_proc(tk, pfn, flags) < 0)
 				pr_err("%#lx: Cannot send advisory machine check signal to %s:%d\n",
-				       pfn, tk->tsk->comm, tk->tsk->pid);
+				       pfn, tk->tsk->comm, task_pid_nr(tk->tsk));
 		}
 		list_del(&tk->nd);
 		put_task_struct(tk->tsk);
-- 
2.33.0

[PATCH v2 09/13] mm/memory-failure: remove obsolete comment in unpoison_memory()

[Miaohe Lin]

Since commit 130d4df57390 ("mm/sl[au]b: rearrange struct slab fields to
allow larger rcu_head"), folio->_mapcount is not overloaded with SLAB.
Update corresponding comment.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 mm/memory-failure.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 81a544df8381..51df2c2e0cc2 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2584,10 +2584,6 @@ int unpoison_memory(unsigned long pfn)
 	    folio_test_reserved(folio) || folio_test_offline(folio))
 		goto unlock_mutex;
 
-	/*
-	 * Note that folio->_mapcount is overloaded in SLAB, so the simple test
-	 * in folio_mapped() has to be done after folio_test_slab() is checked.
-	 */
 	if (folio_mapped(folio)) {
 		unpoison_pr_info("Unpoison: Someone maps the hwpoison page %#lx\n",
 				 pfn, &unpoison_rs);
-- 
2.33.0

[PATCH v2 10/13] mm/memory-failure: move some function declarations into internal.h

[Miaohe Lin]

There are some functions only used inside mm. Move them into internal.h.
No functional change intended.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202405251049.hxjwX7zO-lkp@intel.com/
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/mm.h         |  7 -------
 include/linux/page-flags.h |  5 -----
 include/linux/rmap.h       |  2 --
 mm/internal.h              | 10 ++++++++++
 4 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index ec7141fb0252..5ca79aff5e39 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4011,7 +4011,6 @@ extern int __get_huge_page_for_hwpoison(unsigned long pfn, int flags,
 					bool *migratable_cleared);
 void num_poisoned_pages_inc(unsigned long pfn);
 void num_poisoned_pages_sub(unsigned long pfn, long i);
-struct task_struct *task_early_kill(struct task_struct *tsk, int force_early);
 #else
 static inline void memory_failure_queue(unsigned long pfn, int flags)
 {
@@ -4032,12 +4031,6 @@ static inline void num_poisoned_pages_sub(unsigned long pfn, long i)
 }
 #endif
 
-#if defined(CONFIG_MEMORY_FAILURE) && defined(CONFIG_KSM)
-void add_to_kill_ksm(struct task_struct *tsk, struct page *p,
-		     struct vm_area_struct *vma, struct list_head *to_kill,
-		     unsigned long ksm_addr);
-#endif
-
 #if defined(CONFIG_MEMORY_FAILURE) && defined(CONFIG_MEMORY_HOTPLUG)
 extern void memblk_nr_poison_inc(unsigned long pfn);
 extern void memblk_nr_poison_sub(unsigned long pfn, long i);
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index f04fea86324d..b4e3444f1d9b 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -616,11 +616,6 @@ PAGEFLAG_FALSE(Uncached, uncached)
 PAGEFLAG(HWPoison, hwpoison, PF_ANY)
 TESTSCFLAG(HWPoison, hwpoison, PF_ANY)
 #define __PG_HWPOISON (1UL << PG_hwpoison)
-#define MAGIC_HWPOISON	0x48575053U	/* HWPS */
-extern void SetPageHWPoisonTakenOff(struct page *page);
-extern void ClearPageHWPoisonTakenOff(struct page *page);
-extern bool take_page_off_buddy(struct page *page);
-extern bool put_page_back_buddy(struct page *page);
 #else
 PAGEFLAG_FALSE(HWPoison, hwpoison)
 #define __PG_HWPOISON 0
diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index 0fd9bebce54c..8ff042c3eae2 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -723,8 +723,6 @@ int pfn_mkclean_range(unsigned long pfn, unsigned long nr_pages, pgoff_t pgoff,
 
 void remove_migration_ptes(struct folio *src, struct folio *dst, bool locked);
 
-unsigned long page_mapped_in_vma(struct page *page, struct vm_area_struct *vma);
-
 /*
  * rmap_walk_control: To control rmap traversing for specific needs
  *
diff --git a/mm/internal.h b/mm/internal.h
index 12e95fdf61e9..7b1368d14e85 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1077,6 +1077,16 @@ extern u64 hwpoison_filter_flags_mask;
 extern u64 hwpoison_filter_flags_value;
 extern u64 hwpoison_filter_memcg;
 extern u32 hwpoison_filter_enable;
+#define MAGIC_HWPOISON	0x48575053U	/* HWPS */
+extern void SetPageHWPoisonTakenOff(struct page *page);
+extern void ClearPageHWPoisonTakenOff(struct page *page);
+extern bool take_page_off_buddy(struct page *page);
+extern bool put_page_back_buddy(struct page *page);
+struct task_struct *task_early_kill(struct task_struct *tsk, int force_early);
+void add_to_kill_ksm(struct task_struct *tsk, struct page *p,
+		     struct vm_area_struct *vma, struct list_head *to_kill,
+		     unsigned long ksm_addr);
+unsigned long page_mapped_in_vma(struct page *page, struct vm_area_struct *vma);
 
 extern unsigned long  __must_check vm_mmap_pgoff(struct file *, unsigned long,
         unsigned long, unsigned long,
-- 
2.33.0

[PATCH v2 11/13] mm/memory-failure: fix comment of get_hwpoison_page()

[Miaohe Lin]

When return value is 0, it could also means the page is free hugetlb
page or free buddy page. Fix the corresponding comment.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 51df2c2e0cc2..0d37f20968e6 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1525,7 +1525,7 @@ static int __get_unpoison_page(struct page *page)
  * the given page has PG_hwpoison. So it's never reused for other page
  * allocations, and __get_unpoison_page() never races with them.
  *
- * Return: 0 on failure,
+ * Return: 0 on failure or free buddy (hugetlb) page,
  *         1 on success for in-use pages in a well-defined state,
  *         -EIO for pages on which we can not handle memory errors,
  *         -EBUSY when get_hwpoison_page() has raced with page lifecycle
-- 
2.33.0

[PATCH v2 12/13] mm/memory-failure: remove obsolete comment in kill_proc()

[Miaohe Lin]

When user sets SIGBUS to SIG_IGN, it won't cause loop now. For action
required mce error, SIGBUS cannot be blocked. Also when a hwpoisoned
page is re-accessed, kill_accessing_process() will be called to kill
the process.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 0d37f20968e6..161bdc79b44c 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -354,8 +354,6 @@ static int kill_proc(struct to_kill *tk, unsigned long pfn, int flags)
 		 * PF_MCE_EARLY set.
 		 * Don't use force here, it's convenient if the signal
 		 * can be temporarily blocked.
-		 * This could cause a loop when the user sets SIGBUS
-		 * to SIG_IGN, but hopefully no one will do that?
 		 */
 		ret = send_sig_mceerr(BUS_MCEERR_AO, (void __user *)tk->addr,
 				      addr_lsb, t);
-- 
2.33.0

[PATCH v2 13/13] mm/memory-failure: correct comment in me_swapcache_dirty

[Miaohe Lin]

Dirty swap cache page could live both in page table (not page cache) and
swap cache when freshly swapped in. Correct comment.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 161bdc79b44c..4fd8ab8fe3b4 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1126,7 +1126,7 @@ static int me_pagecache_dirty(struct page_state *ps, struct page *p)
  * Clean and dirty swap cache.
  *
  * Dirty swap cache page is tricky to handle. The page could live both in page
- * cache and swap cache(ie. page is freshly swapped in). So it could be
+ * table and swap cache(ie. page is freshly swapped in). So it could be
  * referenced concurrently by 2 types of PTEs:
  * normal PTEs and swap PTEs. We try to handle them consistently by calling
  * try_to_unmap(!TTU_HWPOISON) to convert the normal PTEs to swap PTEs,
-- 
2.33.0

Re: [PATCH v2 01/13] mm/memory-failure: simplify put_ref_page()

[Kefeng Wang]

On 2024/6/6 14:32, Miaohe Lin wrote:
> Remove unneeded page != NULL check. pfn_to_page() won't return NULL.
> No functional change intended.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/memory-failure.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index f679b579d45d..2e6038c73119 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2120,14 +2120,10 @@ static inline unsigned long folio_free_raw_hwp(struct folio *folio, bool flag)
>   /* Drop the extra refcount in case we come from madvise() */
>   static void put_ref_page(unsigned long pfn, int flags)

Since all calllers have a valid page，better to pass the page instead of 
pfn?

>   {
> -	struct page *page;
> -
>   	if (!(flags & MF_COUNT_INCREASED))
>   		return;
>   
> -	page = pfn_to_page(pfn);
> -	if (page)
> -		put_page(page);
> +	put_page(pfn_to_page(pfn));
>   }
>   
>   static int memory_failure_dev_pagemap(unsigned long pfn, int flags,

Re: [PATCH v2 07/13] mm/memory-failure: simplify unneeded hwpoison_filter() variant

[kernel test robot]

Hi Miaohe,

kernel test robot noticed the following build errors:

[auto build test ERROR on 19b8422c5bd56fb5e7085995801c6543a98bda1f]

url:    https://github.com/intel-lab-lkp/linux/commits/Miaohe-Lin/mm-memory-failure-simplify-put_ref_page/20240606-143939
base:   19b8422c5bd56fb5e7085995801c6543a98bda1f
patch link:    https://lore.kernel.org/r/20240606063247.712575-8-linmiaohe%40huawei.com
patch subject: [PATCH v2 07/13] mm/memory-failure: simplify unneeded hwpoison_filter() variant
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240607/202406070136.hGQwVbsv-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406070136.hGQwVbsv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406070136.hGQwVbsv-lkp@intel.com/

All errors (new ones prefixed by >>):

>> mm/memory-failure.c:299:19: error: static declaration of 'hwpoison_filter' follows non-static declaration
     299 | static inline int hwpoison_filter(struct page *p)
         |                   ^~~~~~~~~~~~~~~
   In file included from mm/memory-failure.c:64:
   mm/internal.h:1072:12: note: previous declaration of 'hwpoison_filter' with type 'int(struct page *)'
    1072 | extern int hwpoison_filter(struct page *p);
         |            ^~~~~~~~~~~~~~~


vim +/hwpoison_filter +299 mm/memory-failure.c

   280	
   281	int hwpoison_filter(struct page *p)
   282	{
   283		if (!hwpoison_filter_enable)
   284			return 0;
   285	
   286		if (hwpoison_filter_dev(p))
   287			return -EINVAL;
   288	
   289		if (hwpoison_filter_flags(p))
   290			return -EINVAL;
   291	
   292		if (hwpoison_filter_task(p))
   293			return -EINVAL;
   294	
   295		return 0;
   296	}
   297	EXPORT_SYMBOL_GPL(hwpoison_filter);
   298	#else
 > 299	static inline int hwpoison_filter(struct page *p)
   300	{
   301		return 0;
   302	}
   303	#endif
   304	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 01/13] mm/memory-failure: simplify put_ref_page()

[Miaohe Lin]

On 2024/6/6 14:46, Kefeng Wang wrote:
> 
> 
> On 2024/6/6 14:32, Miaohe Lin wrote:
>> Remove unneeded page != NULL check. pfn_to_page() won't return NULL.
>> No functional change intended.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>   mm/memory-failure.c | 6 +-----
>>   1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index f679b579d45d..2e6038c73119 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -2120,14 +2120,10 @@ static inline unsigned long folio_free_raw_hwp(struct folio *folio, bool flag)
>>   /* Drop the extra refcount in case we come from madvise() */
>>   static void put_ref_page(unsigned long pfn, int flags)
> 
> Since all calllers have a valid page，better to pass the page instead of pfn?

Seems not. put_ref_page() called above memory_failure_dev_pagemap() seems don't have a valid page yet.
Also page might be NULL when calling put_ref_page() in soft_offline_page(). So it should be better to
still pass pfn. Or am I miss something?

Thanks.
.

Re: [PATCH v2 07/13] mm/memory-failure: simplify unneeded hwpoison_filter() variant

[Miaohe Lin]

On 2024/6/7 1:15, kernel test robot wrote:
> Hi Miaohe,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on 19b8422c5bd56fb5e7085995801c6543a98bda1f]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Miaohe-Lin/mm-memory-failure-simplify-put_ref_page/20240606-143939
> base:   19b8422c5bd56fb5e7085995801c6543a98bda1f
> patch link:    https://lore.kernel.org/r/20240606063247.712575-8-linmiaohe%40huawei.com
> patch subject: [PATCH v2 07/13] mm/memory-failure: simplify unneeded hwpoison_filter() variant
> config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240607/202406070136.hGQwVbsv-lkp@intel.com/config)
> compiler: aarch64-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406070136.hGQwVbsv-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202406070136.hGQwVbsv-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>>> mm/memory-failure.c:299:19: error: static declaration of 'hwpoison_filter' follows non-static declaration
>      299 | static inline int hwpoison_filter(struct page *p)
>          |                   ^~~~~~~~~~~~~~~
>    In file included from mm/memory-failure.c:64:
>    mm/internal.h:1072:12: note: previous declaration of 'hwpoison_filter' with type 'int(struct page *)'
>     1072 | extern int hwpoison_filter(struct page *p);
>          |            ^~~~~~~~~~~~~~~
> 

Thanks for reporting again. Will fix it.
Thanks.
.

Re: [PATCH v2 10/13] mm/memory-failure: move some function declarations into internal.h

[Matthew Wilcox]

On Thu, Jun 06, 2024 at 02:32:44PM +0800, Miaohe Lin wrote:
> +++ b/mm/internal.h
> @@ -1077,6 +1077,16 @@ extern u64 hwpoison_filter_flags_mask;
>  extern u64 hwpoison_filter_flags_value;
>  extern u64 hwpoison_filter_memcg;
>  extern u32 hwpoison_filter_enable;
> +#define MAGIC_HWPOISON	0x48575053U	/* HWPS */
> +extern void SetPageHWPoisonTakenOff(struct page *page);
> +extern void ClearPageHWPoisonTakenOff(struct page *page);
> +extern bool take_page_off_buddy(struct page *page);
> +extern bool put_page_back_buddy(struct page *page);

s/extern// for function declarations.

Re: [PATCH v2 01/13] mm/memory-failure: simplify put_ref_page()

[Kefeng Wang]

On 2024/6/7 11:28, Miaohe Lin wrote:
> On 2024/6/6 14:46, Kefeng Wang wrote:
>>
>>
>> On 2024/6/6 14:32, Miaohe Lin wrote:
>>> Remove unneeded page != NULL check. pfn_to_page() won't return NULL.
>>> No functional change intended.
>>>
>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>>> ---
>>>    mm/memory-failure.c | 6 +-----
>>>    1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>>> index f679b579d45d..2e6038c73119 100644
>>> --- a/mm/memory-failure.c
>>> +++ b/mm/memory-failure.c
>>> @@ -2120,14 +2120,10 @@ static inline unsigned long folio_free_raw_hwp(struct folio *folio, bool flag)
>>>    /* Drop the extra refcount in case we come from madvise() */
>>>    static void put_ref_page(unsigned long pfn, int flags)
>>
>> Since all calllers have a valid page，better to pass the page instead of pfn?
> 
> Seems not. put_ref_page() called above memory_failure_dev_pagemap() seems don't have a valid page yet.
> Also page might be NULL when calling put_ref_page() in soft_offline_page(). So it should be better to
> still pass pfn. Or am I miss something?

Yes, missing it, please ignore it
> 
> Thanks.
> .
> 

Re: [PATCH v2 10/13] mm/memory-failure: move some function declarations into internal.h

[Miaohe Lin]

On 2024/6/7 11:47, Matthew Wilcox wrote:
> On Thu, Jun 06, 2024 at 02:32:44PM +0800, Miaohe Lin wrote:
>> +++ b/mm/internal.h
>> @@ -1077,6 +1077,16 @@ extern u64 hwpoison_filter_flags_mask;
>>  extern u64 hwpoison_filter_flags_value;
>>  extern u64 hwpoison_filter_memcg;
>>  extern u32 hwpoison_filter_enable;
>> +#define MAGIC_HWPOISON	0x48575053U	/* HWPS */
>> +extern void SetPageHWPoisonTakenOff(struct page *page);
>> +extern void ClearPageHWPoisonTakenOff(struct page *page);
>> +extern bool take_page_off_buddy(struct page *page);
>> +extern bool put_page_back_buddy(struct page *page);
> 
> s/extern// for function declarations.

Will do in next version.
Thanks.
.