linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] hugetlbfs: don't delete error page from pagecache
@ 2022-11-07 18:05 James Houghton
  2022-11-07 18:53 ` Yang Shi
  0 siblings, 1 reply; 2+ messages in thread
From: James Houghton @ 2022-11-07 18:05 UTC (permalink / raw)
  To: Mike Kravetz, Muchun Song, Naoya Horiguchi, Miaohe Lin, Andrew Morton
  Cc: Yang Shi, Axel Rasmussen, linux-mm, linux-kernel, stable, James Houghton

This change is very similar to the change that was made for shmem [1],
and it solves the same problem but for HugeTLBFS instead.

Currently, when poison is found in a HugeTLB page, the page is removed
from the page cache. That means that attempting to map or read that
hugepage in the future will result in a new hugepage being allocated
instead of notifying the user that the page was poisoned. As [1] states,
this is effectively memory corruption.

The fix is to leave the page in the page cache. If the user attempts to
use a poisoned HugeTLB page with a syscall, the syscall will fail with
EIO, the same error code that shmem uses. For attempts to map the page,
the thread will get a BUS_MCEERR_AR SIGBUS.

[1]: commit a76054266661 ("mm: shmem: don't truncate page if memory failure happens")

Fixes: 78bb920344b8 ("mm: hwpoison: dissolve in-use hugepage in unrecoverable memory error")
Cc: <stable@vger.kernel.org>
Signed-off-by: James Houghton <jthoughton@google.com>
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Reviewed-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
Tested-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
---
 fs/hugetlbfs/inode.c | 13 ++++++-------
 mm/hugetlb.c         |  4 ++++
 mm/memory-failure.c  |  5 ++++-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index dd54f67e47fd..df7772335dc0 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -328,6 +328,12 @@ static ssize_t hugetlbfs_read_iter(struct kiocb *iocb, struct iov_iter *to)
 		} else {
 			unlock_page(page);
 
+			if (PageHWPoison(page)) {
+				put_page(page);
+				retval = -EIO;
+				break;
+			}
+
 			/*
 			 * We have the page, copy it to user space buffer.
 			 */
@@ -1111,13 +1117,6 @@ static int hugetlbfs_migrate_folio(struct address_space *mapping,
 static int hugetlbfs_error_remove_page(struct address_space *mapping,
 				struct page *page)
 {
-	struct inode *inode = mapping->host;
-	pgoff_t index = page->index;
-
-	hugetlb_delete_from_page_cache(page);
-	if (unlikely(hugetlb_unreserve_pages(inode, index, index + 1, 1)))
-		hugetlb_fix_reserve_counts(inode);
-
 	return 0;
 }
 
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 546df97c31e4..e48f8ef45b17 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6111,6 +6111,10 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm,
 
 	ptl = huge_pte_lock(h, dst_mm, dst_pte);
 
+	ret = -EIO;
+	if (PageHWPoison(page))
+		goto out_release_unlock;
+
 	/*
 	 * We allow to overwrite a pte marker: consider when both MISSING|WP
 	 * registered, we firstly wr-protect a none pte which has no page cache
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 145bb561ddb3..bead6bccc7f2 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1080,6 +1080,7 @@ static int me_huge_page(struct page_state *ps, struct page *p)
 	int res;
 	struct page *hpage = compound_head(p);
 	struct address_space *mapping;
+	bool extra_pins = false;
 
 	if (!PageHuge(hpage))
 		return MF_DELAYED;
@@ -1087,6 +1088,8 @@ static int me_huge_page(struct page_state *ps, struct page *p)
 	mapping = page_mapping(hpage);
 	if (mapping) {
 		res = truncate_error_page(hpage, page_to_pfn(p), mapping);
+		/* The page is kept in page cache. */
+		extra_pins = true;
 		unlock_page(hpage);
 	} else {
 		unlock_page(hpage);
@@ -1104,7 +1107,7 @@ static int me_huge_page(struct page_state *ps, struct page *p)
 		}
 	}
 
-	if (has_extra_refcount(ps, p, false))
+	if (has_extra_refcount(ps, p, extra_pins))
 		res = MF_FAILED;
 
 	return res;
-- 
2.38.1.431.g37b22c650d-goog


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] hugetlbfs: don't delete error page from pagecache
  2022-11-07 18:05 [PATCH v2] hugetlbfs: don't delete error page from pagecache James Houghton
@ 2022-11-07 18:53 ` Yang Shi
  0 siblings, 0 replies; 2+ messages in thread
From: Yang Shi @ 2022-11-07 18:53 UTC (permalink / raw)
  To: James Houghton
  Cc: Mike Kravetz, Muchun Song, Naoya Horiguchi, Miaohe Lin,
	Andrew Morton, Axel Rasmussen, linux-mm, linux-kernel, stable

On Mon, Nov 7, 2022 at 10:06 AM James Houghton <jthoughton@google.com> wrote:
>
> This change is very similar to the change that was made for shmem [1],
> and it solves the same problem but for HugeTLBFS instead.
>
> Currently, when poison is found in a HugeTLB page, the page is removed
> from the page cache. That means that attempting to map or read that
> hugepage in the future will result in a new hugepage being allocated
> instead of notifying the user that the page was poisoned. As [1] states,
> this is effectively memory corruption.
>
> The fix is to leave the page in the page cache. If the user attempts to
> use a poisoned HugeTLB page with a syscall, the syscall will fail with
> EIO, the same error code that shmem uses. For attempts to map the page,
> the thread will get a BUS_MCEERR_AR SIGBUS.
>
> [1]: commit a76054266661 ("mm: shmem: don't truncate page if memory failure happens")
>
> Fixes: 78bb920344b8 ("mm: hwpoison: dissolve in-use hugepage in unrecoverable memory error")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: James Houghton <jthoughton@google.com>
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> Reviewed-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
> Tested-by: Naoya Horiguchi <naoya.horiguchi@nec.com>

Reviewed-by: Yang Shi <shy828301@gmail.com>

> ---
>  fs/hugetlbfs/inode.c | 13 ++++++-------
>  mm/hugetlb.c         |  4 ++++
>  mm/memory-failure.c  |  5 ++++-
>  3 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index dd54f67e47fd..df7772335dc0 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -328,6 +328,12 @@ static ssize_t hugetlbfs_read_iter(struct kiocb *iocb, struct iov_iter *to)
>                 } else {
>                         unlock_page(page);
>
> +                       if (PageHWPoison(page)) {
> +                               put_page(page);
> +                               retval = -EIO;
> +                               break;
> +                       }
> +
>                         /*
>                          * We have the page, copy it to user space buffer.
>                          */
> @@ -1111,13 +1117,6 @@ static int hugetlbfs_migrate_folio(struct address_space *mapping,
>  static int hugetlbfs_error_remove_page(struct address_space *mapping,
>                                 struct page *page)
>  {
> -       struct inode *inode = mapping->host;
> -       pgoff_t index = page->index;
> -
> -       hugetlb_delete_from_page_cache(page);
> -       if (unlikely(hugetlb_unreserve_pages(inode, index, index + 1, 1)))
> -               hugetlb_fix_reserve_counts(inode);
> -
>         return 0;
>  }
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 546df97c31e4..e48f8ef45b17 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6111,6 +6111,10 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm,
>
>         ptl = huge_pte_lock(h, dst_mm, dst_pte);
>
> +       ret = -EIO;
> +       if (PageHWPoison(page))
> +               goto out_release_unlock;
> +
>         /*
>          * We allow to overwrite a pte marker: consider when both MISSING|WP
>          * registered, we firstly wr-protect a none pte which has no page cache
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 145bb561ddb3..bead6bccc7f2 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1080,6 +1080,7 @@ static int me_huge_page(struct page_state *ps, struct page *p)
>         int res;
>         struct page *hpage = compound_head(p);
>         struct address_space *mapping;
> +       bool extra_pins = false;
>
>         if (!PageHuge(hpage))
>                 return MF_DELAYED;
> @@ -1087,6 +1088,8 @@ static int me_huge_page(struct page_state *ps, struct page *p)
>         mapping = page_mapping(hpage);
>         if (mapping) {
>                 res = truncate_error_page(hpage, page_to_pfn(p), mapping);
> +               /* The page is kept in page cache. */
> +               extra_pins = true;
>                 unlock_page(hpage);
>         } else {
>                 unlock_page(hpage);
> @@ -1104,7 +1107,7 @@ static int me_huge_page(struct page_state *ps, struct page *p)
>                 }
>         }
>
> -       if (has_extra_refcount(ps, p, false))
> +       if (has_extra_refcount(ps, p, extra_pins))
>                 res = MF_FAILED;
>
>         return res;
> --
> 2.38.1.431.g37b22c650d-goog
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-11-07 18:54 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-07 18:05 [PATCH v2] hugetlbfs: don't delete error page from pagecache James Houghton
2022-11-07 18:53 ` Yang Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).