[PATCH] mm: shmem: fallback to page size splice if large folio has poisoned subpages

[PATCH] mm: shmem: fallback to page size splice if large folio has poisoned subpages

[Baolin Wang]

The tmpfs has already supported the PMD-sized large folios, and splice()
can not read any subpages if the large folio has a poisoned subpage,
which is not good as we discussed in previous mail[1].

Thus adding a fallback to the PAGE_SIZE splice() still allows reading
normal subpages if the large folio has hwpoisoned subpages.

[1] https://lore.kernel.org/all/Zw_d0EVAJkpNJEbA@casper.infradead.org/
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/shmem.c | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 1bef6e32a1fa..79010e636056 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3291,11 +3291,16 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
 	len = min_t(size_t, len, npages * PAGE_SIZE);
 
 	do {
+		bool fallback_page_splice = false;
+		struct page *page = NULL;
+		pgoff_t index;
+		size_t size;
+
 		if (*ppos >= i_size_read(inode))
 			break;
 
-		error = shmem_get_folio(inode, *ppos / PAGE_SIZE, 0, &folio,
-					SGP_READ);
+		index = *ppos >> PAGE_SHIFT;
+		error = shmem_get_folio(inode, index, 0, &folio, SGP_READ);
 		if (error) {
 			if (error == -EINVAL)
 				error = 0;
@@ -3304,12 +3309,15 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
 		if (folio) {
 			folio_unlock(folio);
 
-			if (folio_test_hwpoison(folio) ||
-			    (folio_test_large(folio) &&
-			     folio_test_has_hwpoisoned(folio))) {
+			page = folio_file_page(folio, index);
+			if (PageHWPoison(page)) {
 				error = -EIO;
 				break;
 			}
+
+			if (folio_test_large(folio) &&
+			    folio_test_has_hwpoisoned(folio))
+				fallback_page_splice = true;
 		}
 
 		/*
@@ -3323,7 +3331,18 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
 		isize = i_size_read(inode);
 		if (unlikely(*ppos >= isize))
 			break;
-		part = min_t(loff_t, isize - *ppos, len);
+		/*
+		 * Fallback to PAGE_SIZE splice if the large folio has hwpoisoned
+		 * subpages.
+		 */
+		if (likely(!fallback_page_splice)) {
+			size = len;
+		} else {
+			size_t offset = *ppos & ~PAGE_MASK;
+
+			size = min_t(loff_t, PAGE_SIZE - offset, len);
+		}
+		part = min_t(loff_t, isize - *ppos, size);
 
 		if (folio) {
 			/*
@@ -3331,8 +3350,12 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
 			 * virtual addresses, take care about potential aliasing
 			 * before reading the page on the kernel side.
 			 */
-			if (mapping_writably_mapped(mapping))
-				flush_dcache_folio(folio);
+			if (mapping_writably_mapped(mapping)) {
+				if (likely(!fallback_page_splice))
+					flush_dcache_folio(folio);
+				else
+					flush_dcache_page(page);
+			}
 			folio_mark_accessed(folio);
 			/*
 			 * Ok, we have the page, and it's up-to-date, so we can
-- 
2.39.3

Re: [PATCH] mm: shmem: fallback to page size splice if large folio has poisoned subpages

[Matthew Wilcox]

On Fri, Oct 25, 2024 at 11:26:39AM +0800, Baolin Wang wrote:
> The tmpfs has already supported the PMD-sized large folios, and splice()
> can not read any subpages if the large folio has a poisoned subpage,
> which is not good as we discussed in previous mail[1].

folios do not have subpages.  folios have pages.  do not use the term
"subpage" anywhere.  ever.

Re: [PATCH] mm: shmem: fallback to page size splice if large folio has poisoned subpages

[Baolin Wang]

On 2024/10/25 11:31, Matthew Wilcox wrote:
> On Fri, Oct 25, 2024 at 11:26:39AM +0800, Baolin Wang wrote:
>> The tmpfs has already supported the PMD-sized large folios, and splice()
>> can not read any subpages if the large folio has a poisoned subpage,
>> which is not good as we discussed in previous mail[1].
> 
> folios do not have subpages.  folios have pages.  do not use the term
> "subpage" anywhere.  ever.

OK. This is my previous habit of naming it. Will change 'subpages' to 
'pages' for folios.

Re: [PATCH] mm: shmem: fallback to page size splice if large folio has poisoned subpages

[Andrew Morton]

On Fri, 25 Oct 2024 11:41:28 +0800 Baolin Wang <baolin.wang@linux.alibaba.com> wrote:

> 
> 
> On 2024/10/25 11:31, Matthew Wilcox wrote:
> > On Fri, Oct 25, 2024 at 11:26:39AM +0800, Baolin Wang wrote:
> >> The tmpfs has already supported the PMD-sized large folios, and splice()
> >> can not read any subpages if the large folio has a poisoned subpage,
> >> which is not good as we discussed in previous mail[1].
> > 
> > folios do not have subpages.  folios have pages.  do not use the term
> > "subpage" anywhere.  ever.
> 
> OK. This is my previous habit of naming it. Will change 'subpages' to 
> 'pages' for folios.

While at it, please try to avoid depending upon references to previous
email discussions.  The links may be bad 10 years from now, and it's
laborious for readers to trawl through the online discussion archives
to extract the information they need.

Including the link is fine, and potentially useful.  But please also
include the relevant information right here in the changelog.

Re: [PATCH] mm: shmem: fallback to page size splice if large folio has poisoned subpages

[Baolin Wang]

On 2024/10/26 06:45, Andrew Morton wrote:
> On Fri, 25 Oct 2024 11:41:28 +0800 Baolin Wang <baolin.wang@linux.alibaba.com> wrote:
> 
>>
>>
>> On 2024/10/25 11:31, Matthew Wilcox wrote:
>>> On Fri, Oct 25, 2024 at 11:26:39AM +0800, Baolin Wang wrote:
>>>> The tmpfs has already supported the PMD-sized large folios, and splice()
>>>> can not read any subpages if the large folio has a poisoned subpage,
>>>> which is not good as we discussed in previous mail[1].
>>>
>>> folios do not have subpages.  folios have pages.  do not use the term
>>> "subpage" anywhere.  ever.
>>
>> OK. This is my previous habit of naming it. Will change 'subpages' to
>> 'pages' for folios.
> 
> While at it, please try to avoid depending upon references to previous
> email discussions.  The links may be bad 10 years from now, and it's
> laborious for readers to trawl through the online discussion archives
> to extract the information they need.
> 
> Including the link is fine, and potentially useful.  But please also
> include the relevant information right here in the changelog.

Sure. Will do in v2.