Re: [PATCH v2 2/5] mm: memory-failure: add unmap_posioned_folio()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2024/8/21 15:40, Miaohe Lin wrote:
On 2024/8/17 16:49, Kefeng Wang wrote:
Add unmap_posioned_folio() helper which will be reused by
do_migrate_range() from memory hotplug soon.

Acked-by: David Hildenbrand <david@xxxxxxxxxx>
Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
---
  mm/internal.h       |  9 +++++++++
  mm/memory-failure.c | 43 ++++++++++++++++++++++++++-----------------
  2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index d7aac802efa5..74490b8ac63d 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1045,6 +1045,8 @@ static inline int find_next_best_node(int node, nodemask_t *used_node_mask)
  /*
   * mm/memory-failure.c
   */
+#ifdef CONFIG_MEMORY_FAILURE
+int unmap_posioned_folio(struct folio *folio, enum ttu_flags ttu);
  void shake_folio(struct folio *folio);
  extern int hwpoison_filter(struct page *p);
@@ -1065,6 +1067,13 @@ void add_to_kill_ksm(struct task_struct *tsk, struct page *p,
  		     unsigned long ksm_addr);
  unsigned long page_mapped_in_vma(struct page *page, struct vm_area_struct *vma);
+#else
+static inline int unmap_posioned_folio(struct folio *folio, enum ttu_flags ttu)
+{
+	return 0;
+}
+#endif
+
  extern unsigned long  __must_check vm_mmap_pgoff(struct file *, unsigned long,
          unsigned long, unsigned long,
          unsigned long, unsigned long);
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 353254537b54..93848330de1f 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1554,6 +1554,30 @@ static int get_hwpoison_page(struct page *p, unsigned long flags)
  	return ret;
  }
+int unmap_posioned_folio(struct folio *folio, enum ttu_flags ttu)

Should it be unmap_poisoned_folio ? i.e. s/posioned/poisoned/ :)

Oh, my bad.


+{
+	if (folio_test_hugetlb(folio) && !folio_test_anon(folio)) {
+		struct address_space *mapping;
+		/*
+		 * For hugetlb pages in shared mappings, try_to_unmap
+		 * could potentially call huge_pmd_unshare.  Because of
+		 * this, take semaphore in write mode here and set
+		 * TTU_RMAP_LOCKED to indicate we have taken the lock
+		 * at this higher level.
+		 */
+		mapping = hugetlb_folio_mapping_lock_write(folio);
+		if (!mapping)
+			return -EAGAIN;
+
+		try_to_unmap(folio, ttu|TTU_RMAP_LOCKED);
+		i_mmap_unlock_write(mapping);
+	} else {
+		try_to_unmap(folio, ttu);
+	}
+
+	return 0;

It seems return value is not really needed? We will check folio_mapped below.

It is just to keep the same as before, but


+}
+
  /*
   * Do all that is necessary to remove user space mappings. Unmap
   * the pages and send SIGBUS to the processes if the data was dirty.
@@ -1615,23 +1639,8 @@ static bool hwpoison_user_mappings(struct folio *folio, struct page *p,
  	 */
  	collect_procs(folio, p, &tokill, flags & MF_ACTION_REQUIRED);
- if (folio_test_hugetlb(folio) && !folio_test_anon(folio)) {
-		/*
-		 * For hugetlb pages in shared mappings, try_to_unmap
-		 * could potentially call huge_pmd_unshare.  Because of
-		 * this, take semaphore in write mode here and set
-		 * TTU_RMAP_LOCKED to indicate we have taken the lock
-		 * at this higher level.
-		 */
-		mapping = hugetlb_folio_mapping_lock_write(folio);
-		if (mapping) {
-			try_to_unmap(folio, ttu|TTU_RMAP_LOCKED);
-			i_mmap_unlock_write(mapping);
-		} else
-			pr_info("%#lx: could not lock mapping for mapped huge page\n", pfn);
-	} else {
-		try_to_unmap(folio, ttu);
-	}
+	if (unmap_posioned_folio(folio, ttu))
+		pr_info("%#lx: could not lock mapping for mapped huge page\n", pfn);

It might be better to pr_info inside the unmap_posioned_folio? unmap_posioned_folio might fail due
to other reasons in the future anyway.

think it again, we have another check and print when recheck
folio_mapped(), so killing the return value and this print for hugepage
is better, and in memory-hotplug, we don't pr_warn for this case.

I will remove return and this print if no objection.


Thanks.
.




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux