diff mbox series

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

Message ID fd3893f318493a3720dc1a4b1c33f0f692ddf125.1729825743.git.baolin.wang@linux.alibaba.com (mailing list archive)
State New
Headers show
Series mm: shmem: fallback to page size splice if large folio has poisoned subpages | expand

Commit Message

Baolin Wang Oct. 25, 2024, 3:26 a.m. UTC
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(-)

Comments

Matthew Wilcox Oct. 25, 2024, 3:31 a.m. UTC | #1
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.
Baolin Wang Oct. 25, 2024, 3:41 a.m. UTC | #2
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.
Andrew Morton Oct. 25, 2024, 10:45 p.m. UTC | #3
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.
Baolin Wang Oct. 26, 2024, 4:01 a.m. UTC | #4
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.
diff mbox series

Patch

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