[PATCH v2 1/5] squashfs: Use a folio throughout squashfs_read_folio()

[PATCH v2 1/5] squashfs: Use a folio throughout squashfs_read_folio()

[Matthew Wilcox (Oracle)]

Use modern folio APIs where they exist and convert back to struct
page for the internal functions.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/squashfs/file.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index 21aaa96856c1..bc6598c3a48f 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -445,21 +445,19 @@ static int squashfs_readpage_sparse(struct page *page, int expected)
 
 static int squashfs_read_folio(struct file *file, struct folio *folio)
 {
-	struct page *page = &folio->page;
-	struct inode *inode = page->mapping->host;
+	struct inode *inode = folio->mapping->host;
 	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
-	int index = page->index >> (msblk->block_log - PAGE_SHIFT);
+	int index = folio->index >> (msblk->block_log - PAGE_SHIFT);
 	int file_end = i_size_read(inode) >> msblk->block_log;
 	int expected = index == file_end ?
 			(i_size_read(inode) & (msblk->block_size - 1)) :
 			 msblk->block_size;
 	int res = 0;
-	void *pageaddr;
 
 	TRACE("Entered squashfs_readpage, page index %lx, start block %llx\n",
-				page->index, squashfs_i(inode)->start);
+				folio->index, squashfs_i(inode)->start);
 
-	if (page->index >= ((i_size_read(inode) + PAGE_SIZE - 1) >>
+	if (folio->index >= ((i_size_read(inode) + PAGE_SIZE - 1) >>
 					PAGE_SHIFT))
 		goto out;
 
@@ -472,23 +470,18 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
 			goto out;
 
 		if (res == 0)
-			res = squashfs_readpage_sparse(page, expected);
+			res = squashfs_readpage_sparse(&folio->page, expected);
 		else
-			res = squashfs_readpage_block(page, block, res, expected);
+			res = squashfs_readpage_block(&folio->page, block, res, expected);
 	} else
-		res = squashfs_readpage_fragment(page, expected);
+		res = squashfs_readpage_fragment(&folio->page, expected);
 
 	if (!res)
 		return 0;
 
 out:
-	pageaddr = kmap_atomic(page);
-	memset(pageaddr, 0, PAGE_SIZE);
-	kunmap_atomic(pageaddr);
-	flush_dcache_page(page);
-	if (res == 0)
-		SetPageUptodate(page);
-	unlock_page(page);
+	folio_zero_segment(folio, 0, folio_size(folio));
+	folio_end_read(folio, res == 0);
 
 	return res;
 }
-- 
2.45.2

[PATCH v2 2/5] squashfs: Pass a folio to squashfs_readpage_fragment()

[Matthew Wilcox (Oracle)]

Remove an access to page->mapping.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/squashfs/file.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index bc6598c3a48f..6bd16e12493b 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -417,9 +417,9 @@ void squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
 }
 
 /* Read datablock stored packed inside a fragment (tail-end packed block) */
-static int squashfs_readpage_fragment(struct page *page, int expected)
+static int squashfs_readpage_fragment(struct folio *folio, int expected)
 {
-	struct inode *inode = page->mapping->host;
+	struct inode *inode = folio->mapping->host;
 	struct squashfs_cache_entry *buffer = squashfs_get_fragment(inode->i_sb,
 		squashfs_i(inode)->fragment_block,
 		squashfs_i(inode)->fragment_size);
@@ -430,7 +430,7 @@ static int squashfs_readpage_fragment(struct page *page, int expected)
 			squashfs_i(inode)->fragment_block,
 			squashfs_i(inode)->fragment_size);
 	else
-		squashfs_copy_cache(page, buffer, expected,
+		squashfs_copy_cache(&folio->page, buffer, expected,
 			squashfs_i(inode)->fragment_offset);
 
 	squashfs_cache_put(buffer);
@@ -474,7 +474,7 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
 		else
 			res = squashfs_readpage_block(&folio->page, block, res, expected);
 	} else
-		res = squashfs_readpage_fragment(&folio->page, expected);
+		res = squashfs_readpage_fragment(folio, expected);
 
 	if (!res)
 		return 0;
-- 
2.45.2

[PATCH v2 3/5] squashfs: Convert squashfs_readpage_block() to take a folio

[Matthew Wilcox (Oracle)]

Remove a few accesses to page->mapping.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/squashfs/file.c        |  2 +-
 fs/squashfs/file_cache.c  |  6 +++---
 fs/squashfs/file_direct.c | 11 +++++------
 fs/squashfs/squashfs.h    |  2 +-
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index 6bd16e12493b..5b81e26b1226 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -472,7 +472,7 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
 		if (res == 0)
 			res = squashfs_readpage_sparse(&folio->page, expected);
 		else
-			res = squashfs_readpage_block(&folio->page, block, res, expected);
+			res = squashfs_readpage_block(folio, block, res, expected);
 	} else
 		res = squashfs_readpage_fragment(folio, expected);
 
diff --git a/fs/squashfs/file_cache.c b/fs/squashfs/file_cache.c
index 54c17b7c85fd..0360d22a77d4 100644
--- a/fs/squashfs/file_cache.c
+++ b/fs/squashfs/file_cache.c
@@ -18,9 +18,9 @@
 #include "squashfs.h"
 
 /* Read separately compressed datablock and memcopy into page cache */
-int squashfs_readpage_block(struct page *page, u64 block, int bsize, int expected)
+int squashfs_readpage_block(struct folio *folio, u64 block, int bsize, int expected)
 {
-	struct inode *i = page->mapping->host;
+	struct inode *i = folio->mapping->host;
 	struct squashfs_cache_entry *buffer = squashfs_get_datablock(i->i_sb,
 		block, bsize);
 	int res = buffer->error;
@@ -29,7 +29,7 @@ int squashfs_readpage_block(struct page *page, u64 block, int bsize, int expecte
 		ERROR("Unable to read page, block %llx, size %x\n", block,
 			bsize);
 	else
-		squashfs_copy_cache(page, buffer, expected, 0);
+		squashfs_copy_cache(&folio->page, buffer, expected, 0);
 
 	squashfs_cache_put(buffer);
 	return res;
diff --git a/fs/squashfs/file_direct.c b/fs/squashfs/file_direct.c
index d19d4db74af8..2c3e809d6891 100644
--- a/fs/squashfs/file_direct.c
+++ b/fs/squashfs/file_direct.c
@@ -19,12 +19,11 @@
 #include "page_actor.h"
 
 /* Read separately compressed datablock directly into page cache */
-int squashfs_readpage_block(struct page *target_page, u64 block, int bsize,
-	int expected)
-
+int squashfs_readpage_block(struct folio *folio, u64 block, int bsize,
+		int expected)
 {
-	struct folio *folio = page_folio(target_page);
-	struct inode *inode = target_page->mapping->host;
+	struct page *target_page = &folio->page;
+	struct inode *inode = folio->mapping->host;
 	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
 	loff_t file_end = (i_size_read(inode) - 1) >> PAGE_SHIFT;
 	int mask = (1 << (msblk->block_log - PAGE_SHIFT)) - 1;
@@ -48,7 +47,7 @@ int squashfs_readpage_block(struct page *target_page, u64 block, int bsize,
 	/* Try to grab all the pages covered by the Squashfs block */
 	for (i = 0, index = start_index; index <= end_index; index++) {
 		page[i] = (index == folio->index) ? target_page :
-			grab_cache_page_nowait(target_page->mapping, index);
+			grab_cache_page_nowait(folio->mapping, index);
 
 		if (page[i] == NULL)
 			continue;
diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
index 5a756e6790b5..0f5373479516 100644
--- a/fs/squashfs/squashfs.h
+++ b/fs/squashfs/squashfs.h
@@ -72,7 +72,7 @@ void squashfs_copy_cache(struct page *, struct squashfs_cache_entry *, int,
 				int);
 
 /* file_xxx.c */
-extern int squashfs_readpage_block(struct page *, u64, int, int);
+int squashfs_readpage_block(struct folio *, u64 block, int bsize, int expected);
 
 /* id.c */
 extern int squashfs_get_id(struct super_block *, unsigned int, unsigned int *);
-- 
2.45.2

[PATCH v2 4/5] squashfs; Convert squashfs_copy_cache() to take a folio

[Matthew Wilcox (Oracle)]

Remove accesses to page->index and page->mapping.  Also use folio
APIs where available.  This code still assumes order 0 folios.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/squashfs/file.c       | 46 ++++++++++++++++++++++------------------
 fs/squashfs/file_cache.c |  2 +-
 fs/squashfs/squashfs.h   |  4 ++--
 3 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index 5b81e26b1226..1f27e8161319 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -378,13 +378,15 @@ void squashfs_fill_page(struct page *page, struct squashfs_cache_entry *buffer,
 }
 
 /* Copy data into page cache  */
-void squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
-	int bytes, int offset)
+void squashfs_copy_cache(struct folio *folio,
+		struct squashfs_cache_entry *buffer, size_t bytes,
+		size_t offset)
 {
-	struct inode *inode = page->mapping->host;
+	struct address_space *mapping = folio->mapping;
+	struct inode *inode = mapping->host;
 	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
 	int i, mask = (1 << (msblk->block_log - PAGE_SHIFT)) - 1;
-	int start_index = page->index & ~mask, end_index = start_index | mask;
+	int start_index = folio->index & ~mask, end_index = start_index | mask;
 
 	/*
 	 * Loop copying datablock into pages.  As the datablock likely covers
@@ -394,25 +396,27 @@ void squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
 	 */
 	for (i = start_index; i <= end_index && bytes > 0; i++,
 			bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
-		struct page *push_page;
-		int avail = buffer ? min_t(int, bytes, PAGE_SIZE) : 0;
+		struct folio *push_folio;
+		size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
 
-		TRACE("bytes %d, i %d, available_bytes %d\n", bytes, i, avail);
+		TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
 
-		push_page = (i == page->index) ? page :
-			grab_cache_page_nowait(page->mapping, i);
+		push_folio = (i == folio->index) ? folio :
+			__filemap_get_folio(mapping, i,
+					FGP_LOCK|FGP_CREAT|FGP_NOFS|FGP_NOWAIT,
+					mapping_gfp_mask(mapping));
 
-		if (!push_page)
+		if (!push_folio)
 			continue;
 
-		if (PageUptodate(push_page))
-			goto skip_page;
+		if (folio_test_uptodate(push_folio))
+			goto skip_folio;
 
-		squashfs_fill_page(push_page, buffer, offset, avail);
-skip_page:
-		unlock_page(push_page);
-		if (i != page->index)
-			put_page(push_page);
+		squashfs_fill_page(&push_folio->page, buffer, offset, avail);
+skip_folio:
+		folio_unlock(push_folio);
+		if (i != folio->index)
+			folio_put(push_folio);
 	}
 }
 
@@ -430,16 +434,16 @@ static int squashfs_readpage_fragment(struct folio *folio, int expected)
 			squashfs_i(inode)->fragment_block,
 			squashfs_i(inode)->fragment_size);
 	else
-		squashfs_copy_cache(&folio->page, buffer, expected,
+		squashfs_copy_cache(folio, buffer, expected,
 			squashfs_i(inode)->fragment_offset);
 
 	squashfs_cache_put(buffer);
 	return res;
 }
 
-static int squashfs_readpage_sparse(struct page *page, int expected)
+static int squashfs_readpage_sparse(struct folio *folio, int expected)
 {
-	squashfs_copy_cache(page, NULL, expected, 0);
+	squashfs_copy_cache(folio, NULL, expected, 0);
 	return 0;
 }
 
@@ -470,7 +474,7 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
 			goto out;
 
 		if (res == 0)
-			res = squashfs_readpage_sparse(&folio->page, expected);
+			res = squashfs_readpage_sparse(folio, expected);
 		else
 			res = squashfs_readpage_block(folio, block, res, expected);
 	} else
diff --git a/fs/squashfs/file_cache.c b/fs/squashfs/file_cache.c
index 0360d22a77d4..40e59a43d098 100644
--- a/fs/squashfs/file_cache.c
+++ b/fs/squashfs/file_cache.c
@@ -29,7 +29,7 @@ int squashfs_readpage_block(struct folio *folio, u64 block, int bsize, int expec
 		ERROR("Unable to read page, block %llx, size %x\n", block,
 			bsize);
 	else
-		squashfs_copy_cache(&folio->page, buffer, expected, 0);
+		squashfs_copy_cache(folio, buffer, expected, 0);
 
 	squashfs_cache_put(buffer);
 	return res;
diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
index 0f5373479516..9295556ecfd0 100644
--- a/fs/squashfs/squashfs.h
+++ b/fs/squashfs/squashfs.h
@@ -68,8 +68,8 @@ extern __le64 *squashfs_read_fragment_index_table(struct super_block *,
 
 /* file.c */
 void squashfs_fill_page(struct page *, struct squashfs_cache_entry *, int, int);
-void squashfs_copy_cache(struct page *, struct squashfs_cache_entry *, int,
-				int);
+void squashfs_copy_cache(struct folio *, struct squashfs_cache_entry *,
+		size_t bytes, size_t offset);
 
 /* file_xxx.c */
 int squashfs_readpage_block(struct folio *, u64 block, int bsize, int expected);
-- 
2.45.2

[PATCH v2 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Matthew Wilcox (Oracle)]

squashfs_fill_page is only used in this file, so make it static.
Use kmap_local instead of kmap_atomic, and return a bool so that
the caller can use folio_end_read() which saves an atomic operation
over calling folio_mark_uptodate() followed by folio_unlock().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/squashfs/file.c     | 21 ++++++++++++---------
 fs/squashfs/squashfs.h |  1 -
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index 1f27e8161319..da25d6fa45ce 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -362,19 +362,21 @@ static int read_blocklist(struct inode *inode, int index, u64 *block)
 	return squashfs_block_size(size);
 }
 
-void squashfs_fill_page(struct page *page, struct squashfs_cache_entry *buffer, int offset, int avail)
+static bool squashfs_fill_page(struct folio *folio,
+		struct squashfs_cache_entry *buffer, size_t offset,
+		size_t avail)
 {
-	int copied;
+	size_t copied;
 	void *pageaddr;
 
-	pageaddr = kmap_atomic(page);
+	pageaddr = kmap_local_folio(folio, 0);
 	copied = squashfs_copy_data(pageaddr, buffer, offset, avail);
 	memset(pageaddr + copied, 0, PAGE_SIZE - copied);
-	kunmap_atomic(pageaddr);
+	kunmap_local(pageaddr);
 
-	flush_dcache_page(page);
-	if (copied == avail)
-		SetPageUptodate(page);
+	flush_dcache_folio(folio);
+
+	return copied == avail;
 }
 
 /* Copy data into page cache  */
@@ -398,6 +400,7 @@ void squashfs_copy_cache(struct folio *folio,
 			bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
 		struct folio *push_folio;
 		size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
+		bool uptodate = true;
 
 		TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
 
@@ -412,9 +415,9 @@ void squashfs_copy_cache(struct folio *folio,
 		if (folio_test_uptodate(push_folio))
 			goto skip_folio;
 
-		squashfs_fill_page(&push_folio->page, buffer, offset, avail);
+		uptodate = squashfs_fill_page(push_folio, buffer, offset, avail);
 skip_folio:
-		folio_unlock(push_folio);
+		folio_end_read(push_folio, uptodate);
 		if (i != folio->index)
 			folio_put(push_folio);
 	}
diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
index 9295556ecfd0..37f3518a804a 100644
--- a/fs/squashfs/squashfs.h
+++ b/fs/squashfs/squashfs.h
@@ -67,7 +67,6 @@ extern __le64 *squashfs_read_fragment_index_table(struct super_block *,
 				u64, u64, unsigned int);
 
 /* file.c */
-void squashfs_fill_page(struct page *, struct squashfs_cache_entry *, int, int);
 void squashfs_copy_cache(struct folio *, struct squashfs_cache_entry *,
 		size_t bytes, size_t offset);
 
-- 
2.45.2

Re: [PATCH v2 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Phillip Lougher]

On 12/20/24 22:46, Matthew Wilcox (Oracle) wrote:
> squashfs_fill_page is only used in this file, so make it static.
> Use kmap_local instead of kmap_atomic, and return a bool so that
> the caller can use folio_end_read() which saves an atomic operation
> over calling folio_mark_uptodate() followed by folio_unlock().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/squashfs/file.c     | 21 ++++++++++++---------
>   fs/squashfs/squashfs.h |  1 -
>   2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index 1f27e8161319..da25d6fa45ce 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -362,19 +362,21 @@ static int read_blocklist(struct inode *inode, int index, u64 *block)
>   	return squashfs_block_size(size);
>   }
>   
> -void squashfs_fill_page(struct page *page, struct squashfs_cache_entry *buffer, int offset, int avail)
> +static bool squashfs_fill_page(struct folio *folio,
> +		struct squashfs_cache_entry *buffer, size_t offset,
> +		size_t avail)
>   {
> -	int copied;
> +	size_t copied;
>   	void *pageaddr;
>   
> -	pageaddr = kmap_atomic(page);
> +	pageaddr = kmap_local_folio(folio, 0);
>   	copied = squashfs_copy_data(pageaddr, buffer, offset, avail);
>   	memset(pageaddr + copied, 0, PAGE_SIZE - copied);
> -	kunmap_atomic(pageaddr);
> +	kunmap_local(pageaddr);
>   
> -	flush_dcache_page(page);
> -	if (copied == avail)
> -		SetPageUptodate(page);
> +	flush_dcache_folio(folio);
> +
> +	return copied == avail;
>   }
>   
>   /* Copy data into page cache  */
> @@ -398,6 +400,7 @@ void squashfs_copy_cache(struct folio *folio,
>   			bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
>   		struct folio *push_folio;
>   		size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
> +		bool uptodate = true;
>   
>   		TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
>   
> @@ -412,9 +415,9 @@ void squashfs_copy_cache(struct folio *folio,
>   		if (folio_test_uptodate(push_folio))
>   			goto skip_folio;
>   
> -		squashfs_fill_page(&push_folio->page, buffer, offset, avail);
> +		uptodate = squashfs_fill_page(push_folio, buffer, offset, avail);
>   skip_folio:
> -		folio_unlock(push_folio);
> +		folio_end_read(push_folio, uptodate);

Hi Matthew,

I'm still getting an oops with this (V2) patch.  The same as before, which is an
assert in folio_end_read() triggers.

Looking at the code in folio_end_read(), the assert appears to happen irrespective
of the value of success.

void folio_end_read(struct folio *folio, bool success)
{
	unsigned long mask = 1 << PG_locked;

	/* Must be in bottom byte for x86 to work */
	BUILD_BUG_ON(PG_uptodate > 7);
	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
	VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio);

	if (likely(success))
		mask |= 1 << PG_uptodate;
	if (folio_xor_flags_has_waiters(folio, mask))
		folio_wake_bit(folio, PG_locked);
}
EXPORT_SYMBOL(folio_end_read);

Thanks

Phillip

The oops is

[  977.270664][ T7696] page: refcount:2 mapcount:0 mapping:ffff8880114c1c98 index:0x100 pfn:0x2022c
[  977.271277][ T7696] memcg:ffff8880162c4000
[  977.271501][ T7696] aops:squashfs_aops ino:1f dentry name(?):".tmp_vmlinux2"
[  977.271941][ T7696] flags: 0x200000000000012d(locked|referenced|uptodate|lru|active|zone=1)
[  977.272375][ T7696] raw: 200000000000012d ffffea0000aeaf08 ffffea00009ae388 ffff8880114c1c98
[  977.272796][ T7696] raw: 0000000000000100 0000000000000000 00000002ffffffff ffff8880162c4000
[  977.273215][ T7696] page dumped because: VM_BUG_ON_FOLIO(folio_test_uptodate(folio))
[  977.273600][ T7696] page_owner tracks the page as allocated
[  977.273916][ T7696] page last allocated via order 0, migratetype Movable, gfp_mask 0x152c4a(GFP_NOFS|__GFP_HIGH0
[  977.274987][ T7696]  post_alloc_hook+0x2d0/0x350
[  977.275233][ T7696]  get_page_from_freelist+0xb39/0x22a0
[  977.275512][ T7696]  __alloc_pages_slowpath.constprop.0+0x2ff/0x2650
[  977.275872][ T7696]  __alloc_pages_noprof+0x3e6/0x480
[  977.276139][ T7696]  __folio_alloc_noprof+0x11/0x90
[  977.276392][ T7696]  page_cache_ra_unbounded+0x2e3/0x750
[  977.276779][ T7696]  page_cache_ra_order+0x8ef/0xc30
[  977.277057][ T7696]  page_cache_async_ra+0x5cb/0x820
[  977.277530][ T7696]  filemap_get_pages+0x105b/0x1bd0
[  977.277827][ T7696]  filemap_read+0x3b6/0xd50
[  977.278058][ T7696]  generic_file_read_iter+0x344/0x450
[  977.278323][ T7696]  __kernel_read+0x3b5/0xb10
[  977.278581][ T7696]  integrity_kernel_read+0x7f/0xb0
[  977.278844][ T7696]  ima_calc_file_hash_tfm+0x2bc/0x3d0
[  977.279131][ T7696]  ima_calc_file_hash+0x1ba/0x490
[  977.279415][ T7696]  ima_collect_measurement+0x848/0x9d0
[  977.279721][ T7696] page last free pid 7695 tgid 7695 stack trace:
[  977.280101][ T7696]  free_unref_page+0x619/0x10e0
[  977.280363][ T7696]  __folio_put+0x31d/0x440
[  977.280603][ T7696]  put_page+0x21d/0x280
[  977.280875][ T7696]  anon_pipe_buf_release+0x11a/0x240
[  977.281171][ T7696]  pipe_read+0x635/0x13b0
[  977.281447][ T7696]  vfs_read+0xa0c/0xba0
[  977.281761][ T7696]  ksys_read+0x1fe/0x240
[  977.282045][ T7696]  do_syscall_64+0x74/0x1c0
[  977.282314][ T7696]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  977.282679][ T7696] ------------[ cut here ]------------
[  977.283000][ T7696] kernel BUG at mm/filemap.c:1535!
[  977.283313][ T7696] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
[  977.283939][ T7696] CPU: 4 UID: 0 PID: 7696 Comm: cat Not tainted 6.13.0-rc2-00367-gbfe147475f84 #24
[  977.284605][ T7696] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[  977.285195][ T7696] RIP: 0010:folio_end_read+0x17b/0x1a0
[  977.285517][ T7696] Code: e8 1a 62 ca ff 48 c7 c6 60 17 d8 8a 48 89 ef e8 2b d2 0e 00 0f 0b e8 04 62 ca ff 48 c8
[  977.287017][ T7696] RSP: 0018:ffffc90007aa7710 EFLAGS: 00010293
[  977.287424][ T7696] RAX: 0000000000000000 RBX: 0000000000000001 RCX: ffffc90007aa75b8
[  977.287897][ T7696] RDX: ffff888026320000 RSI: ffffffff81cd65bb RDI: ffff888026320444
[  977.288356][ T7696] RBP: ffffea0000808b00 R08: 0000000000000000 R09: fffffbfff1efaf3a
[  977.288781][ T7696] R10: ffffffff8f7d79d7 R11: 0000000000000001 R12: 0000000000000001
[  977.289166][ T7696] R13: 0000000000000001 R14: 0000000000000001 R15: ffffea0000ac3440
[  977.289550][ T7696] FS:  00007f7d03bb7700(0000) GS:ffff88802da00000(0000) knlGS:0000000000000000
[  977.289982][ T7696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  977.290304][ T7696] CR2: 00007f833c9a6c68 CR3: 0000000011584000 CR4: 00000000003506f0
[  977.290688][ T7696] Call Trace:
[  977.290852][ T7696]  <TASK>
[  977.290999][ T7696]  ? die+0x31/0x80
[  977.291190][ T7696]  ? do_trap+0x232/0x430
[  977.291400][ T7696]  ? folio_end_read+0x17b/0x1a0
[  977.291644][ T7696]  ? folio_end_read+0x17b/0x1a0
[  977.291912][ T7696]  ? do_error_trap+0xf4/0x230
[  977.292172][ T7696]  ? folio_end_read+0x17b/0x1a0
[  977.292415][ T7696]  ? handle_invalid_op+0x34/0x40
[  977.292660][ T7696]  ? folio_end_read+0x17b/0x1a0
[  977.292904][ T7696]  ? exc_invalid_op+0x2d/0x40
[  977.293140][ T7696]  ? asm_exc_invalid_op+0x1a/0x20
[  977.293392][ T7696]  ? folio_end_read+0x17b/0x1a0
[  977.293635][ T7696]  ? folio_end_read+0x17b/0x1a0
[  977.293882][ T7696]  squashfs_copy_cache+0x1d7/0x550
[  977.294174][ T7696]  squashfs_read_folio+0xa13/0xc00
[  977.294483][ T7696]  ? __pfx_squashfs_read_folio+0x10/0x10
[  977.294811][ T7696]  ? __pfx_squashfs_read_folio+0x10/0x10
[  977.295154][ T7696]  filemap_read_folio+0xc0/0x2a0
[  977.295457][ T7696]  ? __pfx_filemap_read_folio+0x10/0x10
[  977.295819][ T7696]  ? page_cache_sync_ra+0x4b4/0x9c0
[  977.296171][ T7696]  filemap_get_pages+0x155c/0x1bd0
[  977.296473][ T7696]  ? current_time+0x79/0x380
[  977.296744][ T7696]  ? __pfx_filemap_get_pages+0x10/0x10
[  977.297055][ T7696]  filemap_read+0x3b6/0xd50
[  977.297315][ T7696]  ? __pfx_filemap_read+0x10/0x10
[  977.297605][ T7696]  ? pipe_write+0xf9f/0x1ae0
[  977.297854][ T7696]  ? __pfx_pipe_write+0x10/0x10
[  977.298127][ T7696]  ? lock_acquire+0x1b1/0x550
[  977.298391][ T7696]  ? __pfx_autoremove_wake_function+0x10/0x10
[  977.298714][ T7696]  generic_file_read_iter+0x344/0x450
[  977.299008][ T7696]  ? rw_verify_area+0xd0/0x700
[  977.299283][ T7696]  vfs_read+0x83e/0xba0
[  977.299526][ T7696]  ? __pfx_vfs_read+0x10/0x10
[  977.299809][ T7696]  ? __pfx_generic_fadvise+0x10/0x10
[  977.300132][ T7696]  ksys_read+0x122/0x240
[  977.300361][ T7696]  ? __pfx_ksys_read+0x10/0x10
[  977.300630][ T7696]  do_syscall_64+0x74/0x1c0
[  977.300859][ T7696]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  977.301152][ T7696] RIP: 0033:0x7f7d034dbba0
[  977.301372][ T7696] Code: 0b 31 c0 48 83 c4 08 e9 be fe ff ff 48 8d 3d 3f f0 08 00 e8 e2 ce 01 00 66 90 83 3d 34
[  977.302300][ T7696] RSP: 002b:00007fffbb0b34a8 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
[  977.302705][ T7696] RAX: ffffffffffffffda RBX: 0000000000008000 RCX: 00007f7d034dbba0
[  977.303088][ T7696] RDX: 0000000000008000 RSI: 0000000031e9c000 RDI: 0000000000000003
[  977.303473][ T7696] RBP: 0000000000008000 R08: 0000000000000003 R09: 00007f7d0344b99a
[  977.303874][ T7696] R10: 0000000000000002 R11: 0000000000000246 R12: 0000000031e9c000
[  977.304283][ T7696] R13: 0000000000000003 R14: 0000000000000000 R15: 0000000000008000
[  977.304675][ T7696]  </TASK>
[  977.304827][ T7696] Modules linked in:
[  977.305052][ T7696] ---[ end trace 0000000000000000 ]---
[  977.305346][ T7696] RIP: 0010:folio_end_read+0x17b/0x1a0
[  977.305640][ T7696] Code: e8 1a 62 ca ff 48 c7 c6 60 17 d8 8a 48 89 ef e8 2b d2 0e 00 0f 0b e8 04 62 ca ff 48 c8
[  977.306747][ T7696] RSP: 0018:ffffc90007aa7710 EFLAGS: 00010293
[  977.307125][ T7696] RAX: 0000000000000000 RBX: 0000000000000001 RCX: ffffc90007aa75b8
[  977.307610][ T7696] RDX: ffff888026320000 RSI: ffffffff81cd65bb RDI: ffff888026320444
[  977.308117][ T7696] RBP: ffffea0000808b00 R08: 0000000000000000 R09: fffffbfff1efaf3a
[  977.308541][ T7696] R10: ffffffff8f7d79d7 R11: 0000000000000001 R12: 0000000000000001
[  977.308964][ T7696] R13: 0000000000000001 R14: 0000000000000001 R15: ffffea0000ac3440
[  977.309385][ T7696] FS:  00007f7d03bb7700(0000) GS:ffff88802da00000(0000) knlGS:0000000000000000
[  977.309842][ T7696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  977.310165][ T7696] CR2: 00007f833c9a6c68 CR3: 0000000011584000 CR4: 00000000003506f0
[  977.310551][ T7696] Kernel panic - not syncing: Fatal exception
[  977.311300][ T7696] Kernel Offset: disabled
[  977.311520][ T7696] Rebooting in 86400 seconds..
  

Re: [PATCH v2 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Ryan Roberts]

Hi Matthew,

On 30/12/2024 00:45, Phillip Lougher wrote:
> 
> 
> On 12/20/24 22:46, Matthew Wilcox (Oracle) wrote:
>> squashfs_fill_page is only used in this file, so make it static.
>> Use kmap_local instead of kmap_atomic, and return a bool so that
>> the caller can use folio_end_read() which saves an atomic operation
>> over calling folio_mark_uptodate() followed by folio_unlock().
>>
>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>> ---
>>   fs/squashfs/file.c     | 21 ++++++++++++---------
>>   fs/squashfs/squashfs.h |  1 -
>>   2 files changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
>> index 1f27e8161319..da25d6fa45ce 100644
>> --- a/fs/squashfs/file.c
>> +++ b/fs/squashfs/file.c
>> @@ -362,19 +362,21 @@ static int read_blocklist(struct inode *inode, int
>> index, u64 *block)
>>       return squashfs_block_size(size);
>>   }
>>   -void squashfs_fill_page(struct page *page, struct squashfs_cache_entry
>> *buffer, int offset, int avail)
>> +static bool squashfs_fill_page(struct folio *folio,
>> +        struct squashfs_cache_entry *buffer, size_t offset,
>> +        size_t avail)
>>   {
>> -    int copied;
>> +    size_t copied;
>>       void *pageaddr;
>>   -    pageaddr = kmap_atomic(page);
>> +    pageaddr = kmap_local_folio(folio, 0);
>>       copied = squashfs_copy_data(pageaddr, buffer, offset, avail);
>>       memset(pageaddr + copied, 0, PAGE_SIZE - copied);
>> -    kunmap_atomic(pageaddr);
>> +    kunmap_local(pageaddr);
>>   -    flush_dcache_page(page);
>> -    if (copied == avail)
>> -        SetPageUptodate(page);
>> +    flush_dcache_folio(folio);
>> +
>> +    return copied == avail;
>>   }
>>     /* Copy data into page cache  */
>> @@ -398,6 +400,7 @@ void squashfs_copy_cache(struct folio *folio,
>>               bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
>>           struct folio *push_folio;
>>           size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
>> +        bool uptodate = true;
>>             TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
>>   @@ -412,9 +415,9 @@ void squashfs_copy_cache(struct folio *folio,
>>           if (folio_test_uptodate(push_folio))
>>               goto skip_folio;
>>   -        squashfs_fill_page(&push_folio->page, buffer, offset, avail);
>> +        uptodate = squashfs_fill_page(push_folio, buffer, offset, avail);
>>   skip_folio:
>> -        folio_unlock(push_folio);
>> +        folio_end_read(push_folio, uptodate);
> 
> Hi Matthew,
> 
> I'm still getting an oops with this (V2) patch.  The same as before, which is an
> assert in folio_end_read() triggers.
> 
> Looking at the code in folio_end_read(), the assert appears to happen irrespective
> of the value of success.

I've just hit the same oops. Just prodding since the original report is now 
getting on for 2 weeks old.

I believe the issue is due to calling folio_end_read() with an uptodate folio, 
and triggering VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio). Prior to this 
change, folio_unlock() was called which doesn't have this assert. It's possible 
to call this for an uptodate folio via the "skip_folio" goto.

I guess either you want to remove the assert (if it's valid to call 
folio_end_read() for already-uptodate folios) or continue to call folio_unlock() 
for the already-uptodate case?

Including my oops (from arm64 for completeness):


[    5.333160] kernel BUG at mm/filemap.c:1526!
[    5.333729] Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
[    5.334590] Modules linked in:
[    5.335020] CPU: 4 UID: 0 PID: 534 Comm: snap Tainted: G        W          6.13.0-rc4-00152-g0187b83d8f07 #30
[    5.336387] Tainted: [W]=WARN
[    5.336774] Hardware name: linux,dummy-virt (DT)
[    5.337416] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    5.338391] pc : folio_end_read+0xe0/0xf0
[    5.338973] lr : folio_end_read+0xe0/0xf0
[    5.339522] sp : ffff80008c903a00
[    5.339961] x29: ffff80008c903a00 x28: fffffdffc66cfbc0 x27: 0000000000001000
[    5.340932] x26: ffff000190224728 x25: ffff0001914a7c60 x24: 0000000000000cbf
[    5.341934] x23: 0000000000001000 x22: 0000000000000ca0 x21: fffffdffc66cd340
[    5.342936] x20: 0000000000000001 x19: fffffdffc66cfbc0 x18: 0000000000000010
[    5.343947] x17: 3130303066666666 x16: 2031303030303030 x15: 3034303030303030
[    5.345034] x14: 0000000000000000 x13: 29296f696c6f6628 x12: 657461646f747075
[    5.345983] x11: 5f747365745f6f69 x10: ffff8000832df210 x9 : ffff80008013bc6c
[    5.346970] x8 : 00000000ffffefff x7 : ffff8000832df210 x6 : 0000000000000000
[    5.347933] x5 : ffff0002fe5693c8 x4 : 0000000000000fff x3 : 0000000000000000
[    5.348899] x2 : 0000000000000000 x1 : ffff000196701180 x0 : 0000000000000040
[    5.349865] Call trace:
[    5.350195]  folio_end_read+0xe0/0xf0 (P)
[    5.350756]  squashfs_copy_cache+0xd8/0x210
[    5.351348]  squashfs_readpage_block+0x98/0xa8
[    5.351944]  squashfs_read_folio+0x164/0x2a8
[    5.352536]  filemap_read_folio+0x44/0x110
[    5.353110]  filemap_fault+0x85c/0xa10
[    5.353650]  __do_fault+0x44/0x320
[    5.354132]  do_fault+0x304/0x6d0
[    5.354605]  __handle_mm_fault+0x660/0xb38
[    5.355200]  handle_mm_fault+0xbc/0x2b0
[    5.355738]  do_page_fault+0x130/0x5c0
[    5.356269]  do_translation_fault+0xc4/0xe8
[    5.356852]  do_mem_abort+0x4c/0xa8
[    5.357350]  el0_da+0x2c/0xa0
[    5.357776]  el0t_64_sync_handler+0x134/0x168
[    5.358378]  el0t_64_sync+0x198/0x1a0
[    5.358892] Code: aa1303e0 9000efc1 910e6021 940138b5 (d4210000) 
[    5.359921] ---[ end trace 0000000000000000 ]---
[    5.360569] note: snap[534] exited with irqs disabled
[    5.361296] note: snap[534] exited with preempt_count 1
[    5.362004] ------------[ cut here ]------------
[    5.362593] WARNING: CPU: 4 PID: 0 at kernel/context_tracking.c:128 ct_kernel_exit.constprop.0+0xfc/0x118
[    5.363859] Modules linked in:
[    5.364250] CPU: 4 UID: 0 PID: 0 Comm: swapper/4 Tainted: G      D W          6.13.0-rc4-00152-g0187b83d8f07 #30
[    5.365562] Tainted: [D]=DIE, [W]=WARN
[    5.366057] Hardware name: linux,dummy-virt (DT)
[    5.366679] pstate: 204003c5 (nzCv DAIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    5.367587] pc : ct_kernel_exit.constprop.0+0xfc/0x118
[    5.368265] lr : ct_idle_enter+0x10/0x20
[    5.368783] sp : ffff800083b63dc0
[    5.369222] x29: ffff800083b63dc0 x28: 0000000000000000 x27: 0000000000000000
[    5.370288] x26: 0000000000000000 x25: ffff000181c58000 x24: 0000000000000000
[    5.371285] x23: 0000000000000000 x22: ffff800083259e20 x21: ffff8000826973f0
[    5.372236] x20: ffff800083259d00 x19: ffff0002fe578548 x18: ffffffffffffffff
[    5.373199] x17: 3430303030303030 x16: 3030303030303020 x15: 0774076e0775076f
[    5.374160] x14: 0000000000000016 x13: ffff80008327fa18 x12: 0000000000000000
[    5.375129] x11: 00000069af8f73f8 x10: 0000000000000ae0 x9 : ffff80008019e15c
[    5.376034] x8 : ffff000181c58b40 x7 : ffff00018b79cb00 x6 : ffff0002fe57d641
[    5.376967] x5 : 4000000000000002 x4 : ffff80027bee4000 x3 : ffff800083b63dc0
[    5.377904] x2 : ffff800082694548 x1 : ffff800082694548 x0 : 4000000000000000
[    5.378832] Call trace:
[    5.379168]  ct_kernel_exit.constprop.0+0xfc/0x118 (P)
[    5.379862]  ct_idle_enter+0x10/0x20
[    5.380371]  default_idle_call+0x24/0x158
[    5.380913]  do_idle+0x20c/0x270
[    5.381365]  cpu_startup_entry+0x3c/0x50
[    5.381880]  secondary_start_kernel+0x138/0x160
[    5.382518]  __secondary_switched+0xc0/0xc8
[    5.383091] ---[ end trace 0000000000000000 ]---


Thanks,
Ryan


> 
> void folio_end_read(struct folio *folio, bool success)
> {
>     unsigned long mask = 1 << PG_locked;
> 
>     /* Must be in bottom byte for x86 to work */
>     BUILD_BUG_ON(PG_uptodate > 7);
>     VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
>     VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio);
> 
>     if (likely(success))
>         mask |= 1 << PG_uptodate;
>     if (folio_xor_flags_has_waiters(folio, mask))
>         folio_wake_bit(folio, PG_locked);
> }
> EXPORT_SYMBOL(folio_end_read);
> 
> Thanks
> 
> Phillip
> 
> The oops is
> 
> [  977.270664][ T7696] page: refcount:2 mapcount:0 mapping:ffff8880114c1c98
> index:0x100 pfn:0x2022c
> [  977.271277][ T7696] memcg:ffff8880162c4000
> [  977.271501][ T7696] aops:squashfs_aops ino:1f dentry name(?):".tmp_vmlinux2"
> [  977.271941][ T7696] flags: 0x200000000000012d(locked|referenced|uptodate|lru|
> active|zone=1)
> [  977.272375][ T7696] raw: 200000000000012d ffffea0000aeaf08 ffffea00009ae388
> ffff8880114c1c98
> [  977.272796][ T7696] raw: 0000000000000100 0000000000000000 00000002ffffffff
> ffff8880162c4000
> [  977.273215][ T7696] page dumped because:
> VM_BUG_ON_FOLIO(folio_test_uptodate(folio))
> [  977.273600][ T7696] page_owner tracks the page as allocated
> [  977.273916][ T7696] page last allocated via order 0, migratetype Movable,
> gfp_mask 0x152c4a(GFP_NOFS|__GFP_HIGH0
> [  977.274987][ T7696]  post_alloc_hook+0x2d0/0x350
> [  977.275233][ T7696]  get_page_from_freelist+0xb39/0x22a0
> [  977.275512][ T7696]  __alloc_pages_slowpath.constprop.0+0x2ff/0x2650
> [  977.275872][ T7696]  __alloc_pages_noprof+0x3e6/0x480
> [  977.276139][ T7696]  __folio_alloc_noprof+0x11/0x90
> [  977.276392][ T7696]  page_cache_ra_unbounded+0x2e3/0x750
> [  977.276779][ T7696]  page_cache_ra_order+0x8ef/0xc30
> [  977.277057][ T7696]  page_cache_async_ra+0x5cb/0x820
> [  977.277530][ T7696]  filemap_get_pages+0x105b/0x1bd0
> [  977.277827][ T7696]  filemap_read+0x3b6/0xd50
> [  977.278058][ T7696]  generic_file_read_iter+0x344/0x450
> [  977.278323][ T7696]  __kernel_read+0x3b5/0xb10
> [  977.278581][ T7696]  integrity_kernel_read+0x7f/0xb0
> [  977.278844][ T7696]  ima_calc_file_hash_tfm+0x2bc/0x3d0
> [  977.279131][ T7696]  ima_calc_file_hash+0x1ba/0x490
> [  977.279415][ T7696]  ima_collect_measurement+0x848/0x9d0
> [  977.279721][ T7696] page last free pid 7695 tgid 7695 stack trace:
> [  977.280101][ T7696]  free_unref_page+0x619/0x10e0
> [  977.280363][ T7696]  __folio_put+0x31d/0x440
> [  977.280603][ T7696]  put_page+0x21d/0x280
> [  977.280875][ T7696]  anon_pipe_buf_release+0x11a/0x240
> [  977.281171][ T7696]  pipe_read+0x635/0x13b0
> [  977.281447][ T7696]  vfs_read+0xa0c/0xba0
> [  977.281761][ T7696]  ksys_read+0x1fe/0x240
> [  977.282045][ T7696]  do_syscall_64+0x74/0x1c0
> [  977.282314][ T7696]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [  977.282679][ T7696] ------------[ cut here ]------------
> [  977.283000][ T7696] kernel BUG at mm/filemap.c:1535!
> [  977.283313][ T7696] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
> [  977.283939][ T7696] CPU: 4 UID: 0 PID: 7696 Comm: cat Not tainted 6.13.0-
> rc2-00367-gbfe147475f84 #24
> [  977.284605][ T7696] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
> 1.16.2-debian-1.16.2-1 04/01/2014
> [  977.285195][ T7696] RIP: 0010:folio_end_read+0x17b/0x1a0
> [  977.285517][ T7696] Code: e8 1a 62 ca ff 48 c7 c6 60 17 d8 8a 48 89 ef e8 2b
> d2 0e 00 0f 0b e8 04 62 ca ff 48 c8
> [  977.287017][ T7696] RSP: 0018:ffffc90007aa7710 EFLAGS: 00010293
> [  977.287424][ T7696] RAX: 0000000000000000 RBX: 0000000000000001 RCX:
> ffffc90007aa75b8
> [  977.287897][ T7696] RDX: ffff888026320000 RSI: ffffffff81cd65bb RDI:
> ffff888026320444
> [  977.288356][ T7696] RBP: ffffea0000808b00 R08: 0000000000000000 R09:
> fffffbfff1efaf3a
> [  977.288781][ T7696] R10: ffffffff8f7d79d7 R11: 0000000000000001 R12:
> 0000000000000001
> [  977.289166][ T7696] R13: 0000000000000001 R14: 0000000000000001 R15:
> ffffea0000ac3440
> [  977.289550][ T7696] FS:  00007f7d03bb7700(0000) GS:ffff88802da00000(0000)
> knlGS:0000000000000000
> [  977.289982][ T7696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  977.290304][ T7696] CR2: 00007f833c9a6c68 CR3: 0000000011584000 CR4:
> 00000000003506f0
> [  977.290688][ T7696] Call Trace:
> [  977.290852][ T7696]  <TASK>
> [  977.290999][ T7696]  ? die+0x31/0x80
> [  977.291190][ T7696]  ? do_trap+0x232/0x430
> [  977.291400][ T7696]  ? folio_end_read+0x17b/0x1a0
> [  977.291644][ T7696]  ? folio_end_read+0x17b/0x1a0
> [  977.291912][ T7696]  ? do_error_trap+0xf4/0x230
> [  977.292172][ T7696]  ? folio_end_read+0x17b/0x1a0
> [  977.292415][ T7696]  ? handle_invalid_op+0x34/0x40
> [  977.292660][ T7696]  ? folio_end_read+0x17b/0x1a0
> [  977.292904][ T7696]  ? exc_invalid_op+0x2d/0x40
> [  977.293140][ T7696]  ? asm_exc_invalid_op+0x1a/0x20
> [  977.293392][ T7696]  ? folio_end_read+0x17b/0x1a0
> [  977.293635][ T7696]  ? folio_end_read+0x17b/0x1a0
> [  977.293882][ T7696]  squashfs_copy_cache+0x1d7/0x550
> [  977.294174][ T7696]  squashfs_read_folio+0xa13/0xc00
> [  977.294483][ T7696]  ? __pfx_squashfs_read_folio+0x10/0x10
> [  977.294811][ T7696]  ? __pfx_squashfs_read_folio+0x10/0x10
> [  977.295154][ T7696]  filemap_read_folio+0xc0/0x2a0
> [  977.295457][ T7696]  ? __pfx_filemap_read_folio+0x10/0x10
> [  977.295819][ T7696]  ? page_cache_sync_ra+0x4b4/0x9c0
> [  977.296171][ T7696]  filemap_get_pages+0x155c/0x1bd0
> [  977.296473][ T7696]  ? current_time+0x79/0x380
> [  977.296744][ T7696]  ? __pfx_filemap_get_pages+0x10/0x10
> [  977.297055][ T7696]  filemap_read+0x3b6/0xd50
> [  977.297315][ T7696]  ? __pfx_filemap_read+0x10/0x10
> [  977.297605][ T7696]  ? pipe_write+0xf9f/0x1ae0
> [  977.297854][ T7696]  ? __pfx_pipe_write+0x10/0x10
> [  977.298127][ T7696]  ? lock_acquire+0x1b1/0x550
> [  977.298391][ T7696]  ? __pfx_autoremove_wake_function+0x10/0x10
> [  977.298714][ T7696]  generic_file_read_iter+0x344/0x450
> [  977.299008][ T7696]  ? rw_verify_area+0xd0/0x700
> [  977.299283][ T7696]  vfs_read+0x83e/0xba0
> [  977.299526][ T7696]  ? __pfx_vfs_read+0x10/0x10
> [  977.299809][ T7696]  ? __pfx_generic_fadvise+0x10/0x10
> [  977.300132][ T7696]  ksys_read+0x122/0x240
> [  977.300361][ T7696]  ? __pfx_ksys_read+0x10/0x10
> [  977.300630][ T7696]  do_syscall_64+0x74/0x1c0
> [  977.300859][ T7696]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [  977.301152][ T7696] RIP: 0033:0x7f7d034dbba0
> [  977.301372][ T7696] Code: 0b 31 c0 48 83 c4 08 e9 be fe ff ff 48 8d 3d 3f f0
> 08 00 e8 e2 ce 01 00 66 90 83 3d 34
> [  977.302300][ T7696] RSP: 002b:00007fffbb0b34a8 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000000
> [  977.302705][ T7696] RAX: ffffffffffffffda RBX: 0000000000008000 RCX:
> 00007f7d034dbba0
> [  977.303088][ T7696] RDX: 0000000000008000 RSI: 0000000031e9c000 RDI:
> 0000000000000003
> [  977.303473][ T7696] RBP: 0000000000008000 R08: 0000000000000003 R09:
> 00007f7d0344b99a
> [  977.303874][ T7696] R10: 0000000000000002 R11: 0000000000000246 R12:
> 0000000031e9c000
> [  977.304283][ T7696] R13: 0000000000000003 R14: 0000000000000000 R15:
> 0000000000008000
> [  977.304675][ T7696]  </TASK>
> [  977.304827][ T7696] Modules linked in:
> [  977.305052][ T7696] ---[ end trace 0000000000000000 ]---
> [  977.305346][ T7696] RIP: 0010:folio_end_read+0x17b/0x1a0
> [  977.305640][ T7696] Code: e8 1a 62 ca ff 48 c7 c6 60 17 d8 8a 48 89 ef e8 2b
> d2 0e 00 0f 0b e8 04 62 ca ff 48 c8
> [  977.306747][ T7696] RSP: 0018:ffffc90007aa7710 EFLAGS: 00010293
> [  977.307125][ T7696] RAX: 0000000000000000 RBX: 0000000000000001 RCX:
> ffffc90007aa75b8
> [  977.307610][ T7696] RDX: ffff888026320000 RSI: ffffffff81cd65bb RDI:
> ffff888026320444
> [  977.308117][ T7696] RBP: ffffea0000808b00 R08: 0000000000000000 R09:
> fffffbfff1efaf3a
> [  977.308541][ T7696] R10: ffffffff8f7d79d7 R11: 0000000000000001 R12:
> 0000000000000001
> [  977.308964][ T7696] R13: 0000000000000001 R14: 0000000000000001 R15:
> ffffea0000ac3440
> [  977.309385][ T7696] FS:  00007f7d03bb7700(0000) GS:ffff88802da00000(0000)
> knlGS:0000000000000000
> [  977.309842][ T7696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  977.310165][ T7696] CR2: 00007f833c9a6c68 CR3: 0000000011584000 CR4:
> 00000000003506f0
> [  977.310551][ T7696] Kernel panic - not syncing: Fatal exception
> [  977.311300][ T7696] Kernel Offset: disabled
> [  977.311520][ T7696] Rebooting in 86400 seconds..
>  
> 

Re: [PATCH v2 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Ryan Roberts]

On 09/01/2025 15:22, Ryan Roberts wrote:
> Hi Matthew,
> 
> On 30/12/2024 00:45, Phillip Lougher wrote:
>>
>>
>> On 12/20/24 22:46, Matthew Wilcox (Oracle) wrote:
>>> squashfs_fill_page is only used in this file, so make it static.
>>> Use kmap_local instead of kmap_atomic, and return a bool so that
>>> the caller can use folio_end_read() which saves an atomic operation
>>> over calling folio_mark_uptodate() followed by folio_unlock().
>>>
>>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>>> ---
>>>   fs/squashfs/file.c     | 21 ++++++++++++---------
>>>   fs/squashfs/squashfs.h |  1 -
>>>   2 files changed, 12 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
>>> index 1f27e8161319..da25d6fa45ce 100644
>>> --- a/fs/squashfs/file.c
>>> +++ b/fs/squashfs/file.c
>>> @@ -362,19 +362,21 @@ static int read_blocklist(struct inode *inode, int
>>> index, u64 *block)
>>>       return squashfs_block_size(size);
>>>   }
>>>   -void squashfs_fill_page(struct page *page, struct squashfs_cache_entry
>>> *buffer, int offset, int avail)
>>> +static bool squashfs_fill_page(struct folio *folio,
>>> +        struct squashfs_cache_entry *buffer, size_t offset,
>>> +        size_t avail)
>>>   {
>>> -    int copied;
>>> +    size_t copied;
>>>       void *pageaddr;
>>>   -    pageaddr = kmap_atomic(page);
>>> +    pageaddr = kmap_local_folio(folio, 0);
>>>       copied = squashfs_copy_data(pageaddr, buffer, offset, avail);
>>>       memset(pageaddr + copied, 0, PAGE_SIZE - copied);
>>> -    kunmap_atomic(pageaddr);
>>> +    kunmap_local(pageaddr);
>>>   -    flush_dcache_page(page);
>>> -    if (copied == avail)
>>> -        SetPageUptodate(page);
>>> +    flush_dcache_folio(folio);
>>> +
>>> +    return copied == avail;
>>>   }
>>>     /* Copy data into page cache  */
>>> @@ -398,6 +400,7 @@ void squashfs_copy_cache(struct folio *folio,
>>>               bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
>>>           struct folio *push_folio;
>>>           size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
>>> +        bool uptodate = true;
>>>             TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
>>>   @@ -412,9 +415,9 @@ void squashfs_copy_cache(struct folio *folio,
>>>           if (folio_test_uptodate(push_folio))
>>>               goto skip_folio;
>>>   -        squashfs_fill_page(&push_folio->page, buffer, offset, avail);
>>> +        uptodate = squashfs_fill_page(push_folio, buffer, offset, avail);
>>>   skip_folio:
>>> -        folio_unlock(push_folio);
>>> +        folio_end_read(push_folio, uptodate);
>>
>> Hi Matthew,
>>
>> I'm still getting an oops with this (V2) patch.  The same as before, which is an
>> assert in folio_end_read() triggers.
>>
>> Looking at the code in folio_end_read(), the assert appears to happen irrespective
>> of the value of success.
> 
> I've just hit the same oops. Just prodding since the original report is now 
> getting on for 2 weeks old.

Sorry forgot to mention I'm hitting it against today's -next (next-20250109).

> 
> I believe the issue is due to calling folio_end_read() with an uptodate folio, 
> and triggering VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio). Prior to this 
> change, folio_unlock() was called which doesn't have this assert. It's possible 
> to call this for an uptodate folio via the "skip_folio" goto.
> 
> I guess either you want to remove the assert (if it's valid to call 
> folio_end_read() for already-uptodate folios) or continue to call folio_unlock() 
> for the already-uptodate case?
> 
> Including my oops (from arm64 for completeness):
> 
> 
> [    5.333160] kernel BUG at mm/filemap.c:1526!
> [    5.333729] Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
> [    5.334590] Modules linked in:
> [    5.335020] CPU: 4 UID: 0 PID: 534 Comm: snap Tainted: G        W          6.13.0-rc4-00152-g0187b83d8f07 #30
> [    5.336387] Tainted: [W]=WARN
> [    5.336774] Hardware name: linux,dummy-virt (DT)
> [    5.337416] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    5.338391] pc : folio_end_read+0xe0/0xf0
> [    5.338973] lr : folio_end_read+0xe0/0xf0
> [    5.339522] sp : ffff80008c903a00
> [    5.339961] x29: ffff80008c903a00 x28: fffffdffc66cfbc0 x27: 0000000000001000
> [    5.340932] x26: ffff000190224728 x25: ffff0001914a7c60 x24: 0000000000000cbf
> [    5.341934] x23: 0000000000001000 x22: 0000000000000ca0 x21: fffffdffc66cd340
> [    5.342936] x20: 0000000000000001 x19: fffffdffc66cfbc0 x18: 0000000000000010
> [    5.343947] x17: 3130303066666666 x16: 2031303030303030 x15: 3034303030303030
> [    5.345034] x14: 0000000000000000 x13: 29296f696c6f6628 x12: 657461646f747075
> [    5.345983] x11: 5f747365745f6f69 x10: ffff8000832df210 x9 : ffff80008013bc6c
> [    5.346970] x8 : 00000000ffffefff x7 : ffff8000832df210 x6 : 0000000000000000
> [    5.347933] x5 : ffff0002fe5693c8 x4 : 0000000000000fff x3 : 0000000000000000
> [    5.348899] x2 : 0000000000000000 x1 : ffff000196701180 x0 : 0000000000000040
> [    5.349865] Call trace:
> [    5.350195]  folio_end_read+0xe0/0xf0 (P)
> [    5.350756]  squashfs_copy_cache+0xd8/0x210
> [    5.351348]  squashfs_readpage_block+0x98/0xa8
> [    5.351944]  squashfs_read_folio+0x164/0x2a8
> [    5.352536]  filemap_read_folio+0x44/0x110
> [    5.353110]  filemap_fault+0x85c/0xa10
> [    5.353650]  __do_fault+0x44/0x320
> [    5.354132]  do_fault+0x304/0x6d0
> [    5.354605]  __handle_mm_fault+0x660/0xb38
> [    5.355200]  handle_mm_fault+0xbc/0x2b0
> [    5.355738]  do_page_fault+0x130/0x5c0
> [    5.356269]  do_translation_fault+0xc4/0xe8
> [    5.356852]  do_mem_abort+0x4c/0xa8
> [    5.357350]  el0_da+0x2c/0xa0
> [    5.357776]  el0t_64_sync_handler+0x134/0x168
> [    5.358378]  el0t_64_sync+0x198/0x1a0
> [    5.358892] Code: aa1303e0 9000efc1 910e6021 940138b5 (d4210000) 
> [    5.359921] ---[ end trace 0000000000000000 ]---
> [    5.360569] note: snap[534] exited with irqs disabled
> [    5.361296] note: snap[534] exited with preempt_count 1
> [    5.362004] ------------[ cut here ]------------
> [    5.362593] WARNING: CPU: 4 PID: 0 at kernel/context_tracking.c:128 ct_kernel_exit.constprop.0+0xfc/0x118
> [    5.363859] Modules linked in:
> [    5.364250] CPU: 4 UID: 0 PID: 0 Comm: swapper/4 Tainted: G      D W          6.13.0-rc4-00152-g0187b83d8f07 #30
> [    5.365562] Tainted: [D]=DIE, [W]=WARN
> [    5.366057] Hardware name: linux,dummy-virt (DT)
> [    5.366679] pstate: 204003c5 (nzCv DAIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    5.367587] pc : ct_kernel_exit.constprop.0+0xfc/0x118
> [    5.368265] lr : ct_idle_enter+0x10/0x20
> [    5.368783] sp : ffff800083b63dc0
> [    5.369222] x29: ffff800083b63dc0 x28: 0000000000000000 x27: 0000000000000000
> [    5.370288] x26: 0000000000000000 x25: ffff000181c58000 x24: 0000000000000000
> [    5.371285] x23: 0000000000000000 x22: ffff800083259e20 x21: ffff8000826973f0
> [    5.372236] x20: ffff800083259d00 x19: ffff0002fe578548 x18: ffffffffffffffff
> [    5.373199] x17: 3430303030303030 x16: 3030303030303020 x15: 0774076e0775076f
> [    5.374160] x14: 0000000000000016 x13: ffff80008327fa18 x12: 0000000000000000
> [    5.375129] x11: 00000069af8f73f8 x10: 0000000000000ae0 x9 : ffff80008019e15c
> [    5.376034] x8 : ffff000181c58b40 x7 : ffff00018b79cb00 x6 : ffff0002fe57d641
> [    5.376967] x5 : 4000000000000002 x4 : ffff80027bee4000 x3 : ffff800083b63dc0
> [    5.377904] x2 : ffff800082694548 x1 : ffff800082694548 x0 : 4000000000000000
> [    5.378832] Call trace:
> [    5.379168]  ct_kernel_exit.constprop.0+0xfc/0x118 (P)
> [    5.379862]  ct_idle_enter+0x10/0x20
> [    5.380371]  default_idle_call+0x24/0x158
> [    5.380913]  do_idle+0x20c/0x270
> [    5.381365]  cpu_startup_entry+0x3c/0x50
> [    5.381880]  secondary_start_kernel+0x138/0x160
> [    5.382518]  __secondary_switched+0xc0/0xc8
> [    5.383091] ---[ end trace 0000000000000000 ]---
> 
> 
> Thanks,
> Ryan
> 
> 
>>
>> void folio_end_read(struct folio *folio, bool success)
>> {
>>     unsigned long mask = 1 << PG_locked;
>>
>>     /* Must be in bottom byte for x86 to work */
>>     BUILD_BUG_ON(PG_uptodate > 7);
>>     VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
>>     VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio);
>>
>>     if (likely(success))
>>         mask |= 1 << PG_uptodate;
>>     if (folio_xor_flags_has_waiters(folio, mask))
>>         folio_wake_bit(folio, PG_locked);
>> }
>> EXPORT_SYMBOL(folio_end_read);
>>
>> Thanks
>>
>> Phillip
>>
>> The oops is
>>
>> [  977.270664][ T7696] page: refcount:2 mapcount:0 mapping:ffff8880114c1c98
>> index:0x100 pfn:0x2022c
>> [  977.271277][ T7696] memcg:ffff8880162c4000
>> [  977.271501][ T7696] aops:squashfs_aops ino:1f dentry name(?):".tmp_vmlinux2"
>> [  977.271941][ T7696] flags: 0x200000000000012d(locked|referenced|uptodate|lru|
>> active|zone=1)
>> [  977.272375][ T7696] raw: 200000000000012d ffffea0000aeaf08 ffffea00009ae388
>> ffff8880114c1c98
>> [  977.272796][ T7696] raw: 0000000000000100 0000000000000000 00000002ffffffff
>> ffff8880162c4000
>> [  977.273215][ T7696] page dumped because:
>> VM_BUG_ON_FOLIO(folio_test_uptodate(folio))
>> [  977.273600][ T7696] page_owner tracks the page as allocated
>> [  977.273916][ T7696] page last allocated via order 0, migratetype Movable,
>> gfp_mask 0x152c4a(GFP_NOFS|__GFP_HIGH0
>> [  977.274987][ T7696]  post_alloc_hook+0x2d0/0x350
>> [  977.275233][ T7696]  get_page_from_freelist+0xb39/0x22a0
>> [  977.275512][ T7696]  __alloc_pages_slowpath.constprop.0+0x2ff/0x2650
>> [  977.275872][ T7696]  __alloc_pages_noprof+0x3e6/0x480
>> [  977.276139][ T7696]  __folio_alloc_noprof+0x11/0x90
>> [  977.276392][ T7696]  page_cache_ra_unbounded+0x2e3/0x750
>> [  977.276779][ T7696]  page_cache_ra_order+0x8ef/0xc30
>> [  977.277057][ T7696]  page_cache_async_ra+0x5cb/0x820
>> [  977.277530][ T7696]  filemap_get_pages+0x105b/0x1bd0
>> [  977.277827][ T7696]  filemap_read+0x3b6/0xd50
>> [  977.278058][ T7696]  generic_file_read_iter+0x344/0x450
>> [  977.278323][ T7696]  __kernel_read+0x3b5/0xb10
>> [  977.278581][ T7696]  integrity_kernel_read+0x7f/0xb0
>> [  977.278844][ T7696]  ima_calc_file_hash_tfm+0x2bc/0x3d0
>> [  977.279131][ T7696]  ima_calc_file_hash+0x1ba/0x490
>> [  977.279415][ T7696]  ima_collect_measurement+0x848/0x9d0
>> [  977.279721][ T7696] page last free pid 7695 tgid 7695 stack trace:
>> [  977.280101][ T7696]  free_unref_page+0x619/0x10e0
>> [  977.280363][ T7696]  __folio_put+0x31d/0x440
>> [  977.280603][ T7696]  put_page+0x21d/0x280
>> [  977.280875][ T7696]  anon_pipe_buf_release+0x11a/0x240
>> [  977.281171][ T7696]  pipe_read+0x635/0x13b0
>> [  977.281447][ T7696]  vfs_read+0xa0c/0xba0
>> [  977.281761][ T7696]  ksys_read+0x1fe/0x240
>> [  977.282045][ T7696]  do_syscall_64+0x74/0x1c0
>> [  977.282314][ T7696]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>> [  977.282679][ T7696] ------------[ cut here ]------------
>> [  977.283000][ T7696] kernel BUG at mm/filemap.c:1535!
>> [  977.283313][ T7696] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
>> [  977.283939][ T7696] CPU: 4 UID: 0 PID: 7696 Comm: cat Not tainted 6.13.0-
>> rc2-00367-gbfe147475f84 #24
>> [  977.284605][ T7696] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
>> 1.16.2-debian-1.16.2-1 04/01/2014
>> [  977.285195][ T7696] RIP: 0010:folio_end_read+0x17b/0x1a0
>> [  977.285517][ T7696] Code: e8 1a 62 ca ff 48 c7 c6 60 17 d8 8a 48 89 ef e8 2b
>> d2 0e 00 0f 0b e8 04 62 ca ff 48 c8
>> [  977.287017][ T7696] RSP: 0018:ffffc90007aa7710 EFLAGS: 00010293
>> [  977.287424][ T7696] RAX: 0000000000000000 RBX: 0000000000000001 RCX:
>> ffffc90007aa75b8
>> [  977.287897][ T7696] RDX: ffff888026320000 RSI: ffffffff81cd65bb RDI:
>> ffff888026320444
>> [  977.288356][ T7696] RBP: ffffea0000808b00 R08: 0000000000000000 R09:
>> fffffbfff1efaf3a
>> [  977.288781][ T7696] R10: ffffffff8f7d79d7 R11: 0000000000000001 R12:
>> 0000000000000001
>> [  977.289166][ T7696] R13: 0000000000000001 R14: 0000000000000001 R15:
>> ffffea0000ac3440
>> [  977.289550][ T7696] FS:  00007f7d03bb7700(0000) GS:ffff88802da00000(0000)
>> knlGS:0000000000000000
>> [  977.289982][ T7696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [  977.290304][ T7696] CR2: 00007f833c9a6c68 CR3: 0000000011584000 CR4:
>> 00000000003506f0
>> [  977.290688][ T7696] Call Trace:
>> [  977.290852][ T7696]  <TASK>
>> [  977.290999][ T7696]  ? die+0x31/0x80
>> [  977.291190][ T7696]  ? do_trap+0x232/0x430
>> [  977.291400][ T7696]  ? folio_end_read+0x17b/0x1a0
>> [  977.291644][ T7696]  ? folio_end_read+0x17b/0x1a0
>> [  977.291912][ T7696]  ? do_error_trap+0xf4/0x230
>> [  977.292172][ T7696]  ? folio_end_read+0x17b/0x1a0
>> [  977.292415][ T7696]  ? handle_invalid_op+0x34/0x40
>> [  977.292660][ T7696]  ? folio_end_read+0x17b/0x1a0
>> [  977.292904][ T7696]  ? exc_invalid_op+0x2d/0x40
>> [  977.293140][ T7696]  ? asm_exc_invalid_op+0x1a/0x20
>> [  977.293392][ T7696]  ? folio_end_read+0x17b/0x1a0
>> [  977.293635][ T7696]  ? folio_end_read+0x17b/0x1a0
>> [  977.293882][ T7696]  squashfs_copy_cache+0x1d7/0x550
>> [  977.294174][ T7696]  squashfs_read_folio+0xa13/0xc00
>> [  977.294483][ T7696]  ? __pfx_squashfs_read_folio+0x10/0x10
>> [  977.294811][ T7696]  ? __pfx_squashfs_read_folio+0x10/0x10
>> [  977.295154][ T7696]  filemap_read_folio+0xc0/0x2a0
>> [  977.295457][ T7696]  ? __pfx_filemap_read_folio+0x10/0x10
>> [  977.295819][ T7696]  ? page_cache_sync_ra+0x4b4/0x9c0
>> [  977.296171][ T7696]  filemap_get_pages+0x155c/0x1bd0
>> [  977.296473][ T7696]  ? current_time+0x79/0x380
>> [  977.296744][ T7696]  ? __pfx_filemap_get_pages+0x10/0x10
>> [  977.297055][ T7696]  filemap_read+0x3b6/0xd50
>> [  977.297315][ T7696]  ? __pfx_filemap_read+0x10/0x10
>> [  977.297605][ T7696]  ? pipe_write+0xf9f/0x1ae0
>> [  977.297854][ T7696]  ? __pfx_pipe_write+0x10/0x10
>> [  977.298127][ T7696]  ? lock_acquire+0x1b1/0x550
>> [  977.298391][ T7696]  ? __pfx_autoremove_wake_function+0x10/0x10
>> [  977.298714][ T7696]  generic_file_read_iter+0x344/0x450
>> [  977.299008][ T7696]  ? rw_verify_area+0xd0/0x700
>> [  977.299283][ T7696]  vfs_read+0x83e/0xba0
>> [  977.299526][ T7696]  ? __pfx_vfs_read+0x10/0x10
>> [  977.299809][ T7696]  ? __pfx_generic_fadvise+0x10/0x10
>> [  977.300132][ T7696]  ksys_read+0x122/0x240
>> [  977.300361][ T7696]  ? __pfx_ksys_read+0x10/0x10
>> [  977.300630][ T7696]  do_syscall_64+0x74/0x1c0
>> [  977.300859][ T7696]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>> [  977.301152][ T7696] RIP: 0033:0x7f7d034dbba0
>> [  977.301372][ T7696] Code: 0b 31 c0 48 83 c4 08 e9 be fe ff ff 48 8d 3d 3f f0
>> 08 00 e8 e2 ce 01 00 66 90 83 3d 34
>> [  977.302300][ T7696] RSP: 002b:00007fffbb0b34a8 EFLAGS: 00000246 ORIG_RAX:
>> 0000000000000000
>> [  977.302705][ T7696] RAX: ffffffffffffffda RBX: 0000000000008000 RCX:
>> 00007f7d034dbba0
>> [  977.303088][ T7696] RDX: 0000000000008000 RSI: 0000000031e9c000 RDI:
>> 0000000000000003
>> [  977.303473][ T7696] RBP: 0000000000008000 R08: 0000000000000003 R09:
>> 00007f7d0344b99a
>> [  977.303874][ T7696] R10: 0000000000000002 R11: 0000000000000246 R12:
>> 0000000031e9c000
>> [  977.304283][ T7696] R13: 0000000000000003 R14: 0000000000000000 R15:
>> 0000000000008000
>> [  977.304675][ T7696]  </TASK>
>> [  977.304827][ T7696] Modules linked in:
>> [  977.305052][ T7696] ---[ end trace 0000000000000000 ]---
>> [  977.305346][ T7696] RIP: 0010:folio_end_read+0x17b/0x1a0
>> [  977.305640][ T7696] Code: e8 1a 62 ca ff 48 c7 c6 60 17 d8 8a 48 89 ef e8 2b
>> d2 0e 00 0f 0b e8 04 62 ca ff 48 c8
>> [  977.306747][ T7696] RSP: 0018:ffffc90007aa7710 EFLAGS: 00010293
>> [  977.307125][ T7696] RAX: 0000000000000000 RBX: 0000000000000001 RCX:
>> ffffc90007aa75b8
>> [  977.307610][ T7696] RDX: ffff888026320000 RSI: ffffffff81cd65bb RDI:
>> ffff888026320444
>> [  977.308117][ T7696] RBP: ffffea0000808b00 R08: 0000000000000000 R09:
>> fffffbfff1efaf3a
>> [  977.308541][ T7696] R10: ffffffff8f7d79d7 R11: 0000000000000001 R12:
>> 0000000000000001
>> [  977.308964][ T7696] R13: 0000000000000001 R14: 0000000000000001 R15:
>> ffffea0000ac3440
>> [  977.309385][ T7696] FS:  00007f7d03bb7700(0000) GS:ffff88802da00000(0000)
>> knlGS:0000000000000000
>> [  977.309842][ T7696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [  977.310165][ T7696] CR2: 00007f833c9a6c68 CR3: 0000000011584000 CR4:
>> 00000000003506f0
>> [  977.310551][ T7696] Kernel panic - not syncing: Fatal exception
>> [  977.311300][ T7696] Kernel Offset: disabled
>> [  977.311520][ T7696] Rebooting in 86400 seconds..
>>  
>>
> 

Re: [PATCH v2 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Ryan Roberts]

On 09/01/2025 15:24, Ryan Roberts wrote:
> On 09/01/2025 15:22, Ryan Roberts wrote:
>> Hi Matthew,
>>
>> On 30/12/2024 00:45, Phillip Lougher wrote:
>>>
>>>
>>> On 12/20/24 22:46, Matthew Wilcox (Oracle) wrote:
>>>> squashfs_fill_page is only used in this file, so make it static.
>>>> Use kmap_local instead of kmap_atomic, and return a bool so that
>>>> the caller can use folio_end_read() which saves an atomic operation
>>>> over calling folio_mark_uptodate() followed by folio_unlock().
>>>>
>>>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
>>>> ---
>>>>   fs/squashfs/file.c     | 21 ++++++++++++---------
>>>>   fs/squashfs/squashfs.h |  1 -
>>>>   2 files changed, 12 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
>>>> index 1f27e8161319..da25d6fa45ce 100644
>>>> --- a/fs/squashfs/file.c
>>>> +++ b/fs/squashfs/file.c
>>>> @@ -362,19 +362,21 @@ static int read_blocklist(struct inode *inode, int
>>>> index, u64 *block)
>>>>       return squashfs_block_size(size);
>>>>   }
>>>>   -void squashfs_fill_page(struct page *page, struct squashfs_cache_entry
>>>> *buffer, int offset, int avail)
>>>> +static bool squashfs_fill_page(struct folio *folio,
>>>> +        struct squashfs_cache_entry *buffer, size_t offset,
>>>> +        size_t avail)
>>>>   {
>>>> -    int copied;
>>>> +    size_t copied;
>>>>       void *pageaddr;
>>>>   -    pageaddr = kmap_atomic(page);
>>>> +    pageaddr = kmap_local_folio(folio, 0);
>>>>       copied = squashfs_copy_data(pageaddr, buffer, offset, avail);
>>>>       memset(pageaddr + copied, 0, PAGE_SIZE - copied);
>>>> -    kunmap_atomic(pageaddr);
>>>> +    kunmap_local(pageaddr);
>>>>   -    flush_dcache_page(page);
>>>> -    if (copied == avail)
>>>> -        SetPageUptodate(page);
>>>> +    flush_dcache_folio(folio);
>>>> +
>>>> +    return copied == avail;
>>>>   }
>>>>     /* Copy data into page cache  */
>>>> @@ -398,6 +400,7 @@ void squashfs_copy_cache(struct folio *folio,
>>>>               bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
>>>>           struct folio *push_folio;
>>>>           size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
>>>> +        bool uptodate = true;
>>>>             TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
>>>>   @@ -412,9 +415,9 @@ void squashfs_copy_cache(struct folio *folio,
>>>>           if (folio_test_uptodate(push_folio))
>>>>               goto skip_folio;
>>>>   -        squashfs_fill_page(&push_folio->page, buffer, offset, avail);
>>>> +        uptodate = squashfs_fill_page(push_folio, buffer, offset, avail);
>>>>   skip_folio:
>>>> -        folio_unlock(push_folio);
>>>> +        folio_end_read(push_folio, uptodate);
>>>
>>> Hi Matthew,
>>>
>>> I'm still getting an oops with this (V2) patch.  The same as before, which is an
>>> assert in folio_end_read() triggers.
>>>
>>> Looking at the code in folio_end_read(), the assert appears to happen irrespective
>>> of the value of success.
>>
>> I've just hit the same oops. Just prodding since the original report is now 
>> getting on for 2 weeks old.
> 
> Sorry forgot to mention I'm hitting it against today's -next (next-20250109).
> 
>>
>> I believe the issue is due to calling folio_end_read() with an uptodate folio, 
>> and triggering VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio). Prior to this 
>> change, folio_unlock() was called which doesn't have this assert. It's possible 
>> to call this for an uptodate folio via the "skip_folio" goto.
>>
>> I guess either you want to remove the assert (if it's valid to call 
>> folio_end_read() for already-uptodate folios) or continue to call folio_unlock() 
>> for the already-uptodate case?

I started getting a different oops after I fixed this issue. It turns out that
__filemap_get_folio() returns ERR_PTR() on error whereas
grab_cache_page_nowait() (used previously) returns NULL. So the continue
condition needs to change. This fixes both for me:

---8<---
diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index da25d6fa45ce..1e4b63592f6c 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -409,15 +409,17 @@ void squashfs_copy_cache(struct folio *folio,
                                        FGP_LOCK|FGP_CREAT|FGP_NOFS|FGP_NOWAIT,
                                        mapping_gfp_mask(mapping));

-               if (!push_folio)
+               if (IS_ERR(push_folio))
                        continue;

-               if (folio_test_uptodate(push_folio))
+               if (folio_test_uptodate(push_folio)) {
+                       folio_unlock(push_folio);
                        goto skip_folio;
+               }

                uptodate = squashfs_fill_page(push_folio, buffer, offset, avail);
-skip_folio:
                folio_end_read(push_folio, uptodate);
+skip_folio:
                if (i != folio->index)
                        folio_put(push_folio);
        }
---8<---

Thanks,
Ryan


>>
>> Including my oops (from arm64 for completeness):
>>
>>
>> [    5.333160] kernel BUG at mm/filemap.c:1526!
>> [    5.333729] Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
>> [    5.334590] Modules linked in:
>> [    5.335020] CPU: 4 UID: 0 PID: 534 Comm: snap Tainted: G        W          6.13.0-rc4-00152-g0187b83d8f07 #30
>> [    5.336387] Tainted: [W]=WARN
>> [    5.336774] Hardware name: linux,dummy-virt (DT)
>> [    5.337416] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [    5.338391] pc : folio_end_read+0xe0/0xf0
>> [    5.338973] lr : folio_end_read+0xe0/0xf0
>> [    5.339522] sp : ffff80008c903a00
>> [    5.339961] x29: ffff80008c903a00 x28: fffffdffc66cfbc0 x27: 0000000000001000
>> [    5.340932] x26: ffff000190224728 x25: ffff0001914a7c60 x24: 0000000000000cbf
>> [    5.341934] x23: 0000000000001000 x22: 0000000000000ca0 x21: fffffdffc66cd340
>> [    5.342936] x20: 0000000000000001 x19: fffffdffc66cfbc0 x18: 0000000000000010
>> [    5.343947] x17: 3130303066666666 x16: 2031303030303030 x15: 3034303030303030
>> [    5.345034] x14: 0000000000000000 x13: 29296f696c6f6628 x12: 657461646f747075
>> [    5.345983] x11: 5f747365745f6f69 x10: ffff8000832df210 x9 : ffff80008013bc6c
>> [    5.346970] x8 : 00000000ffffefff x7 : ffff8000832df210 x6 : 0000000000000000
>> [    5.347933] x5 : ffff0002fe5693c8 x4 : 0000000000000fff x3 : 0000000000000000
>> [    5.348899] x2 : 0000000000000000 x1 : ffff000196701180 x0 : 0000000000000040
>> [    5.349865] Call trace:
>> [    5.350195]  folio_end_read+0xe0/0xf0 (P)
>> [    5.350756]  squashfs_copy_cache+0xd8/0x210
>> [    5.351348]  squashfs_readpage_block+0x98/0xa8
>> [    5.351944]  squashfs_read_folio+0x164/0x2a8
>> [    5.352536]  filemap_read_folio+0x44/0x110
>> [    5.353110]  filemap_fault+0x85c/0xa10
>> [    5.353650]  __do_fault+0x44/0x320
>> [    5.354132]  do_fault+0x304/0x6d0
>> [    5.354605]  __handle_mm_fault+0x660/0xb38
>> [    5.355200]  handle_mm_fault+0xbc/0x2b0
>> [    5.355738]  do_page_fault+0x130/0x5c0
>> [    5.356269]  do_translation_fault+0xc4/0xe8
>> [    5.356852]  do_mem_abort+0x4c/0xa8
>> [    5.357350]  el0_da+0x2c/0xa0
>> [    5.357776]  el0t_64_sync_handler+0x134/0x168
>> [    5.358378]  el0t_64_sync+0x198/0x1a0
>> [    5.358892] Code: aa1303e0 9000efc1 910e6021 940138b5 (d4210000) 
>> [    5.359921] ---[ end trace 0000000000000000 ]---
>> [    5.360569] note: snap[534] exited with irqs disabled
>> [    5.361296] note: snap[534] exited with preempt_count 1
>> [    5.362004] ------------[ cut here ]------------
>> [    5.362593] WARNING: CPU: 4 PID: 0 at kernel/context_tracking.c:128 ct_kernel_exit.constprop.0+0xfc/0x118
>> [    5.363859] Modules linked in:
>> [    5.364250] CPU: 4 UID: 0 PID: 0 Comm: swapper/4 Tainted: G      D W          6.13.0-rc4-00152-g0187b83d8f07 #30
>> [    5.365562] Tainted: [D]=DIE, [W]=WARN
>> [    5.366057] Hardware name: linux,dummy-virt (DT)
>> [    5.366679] pstate: 204003c5 (nzCv DAIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [    5.367587] pc : ct_kernel_exit.constprop.0+0xfc/0x118
>> [    5.368265] lr : ct_idle_enter+0x10/0x20
>> [    5.368783] sp : ffff800083b63dc0
>> [    5.369222] x29: ffff800083b63dc0 x28: 0000000000000000 x27: 0000000000000000
>> [    5.370288] x26: 0000000000000000 x25: ffff000181c58000 x24: 0000000000000000
>> [    5.371285] x23: 0000000000000000 x22: ffff800083259e20 x21: ffff8000826973f0
>> [    5.372236] x20: ffff800083259d00 x19: ffff0002fe578548 x18: ffffffffffffffff
>> [    5.373199] x17: 3430303030303030 x16: 3030303030303020 x15: 0774076e0775076f
>> [    5.374160] x14: 0000000000000016 x13: ffff80008327fa18 x12: 0000000000000000
>> [    5.375129] x11: 00000069af8f73f8 x10: 0000000000000ae0 x9 : ffff80008019e15c
>> [    5.376034] x8 : ffff000181c58b40 x7 : ffff00018b79cb00 x6 : ffff0002fe57d641
>> [    5.376967] x5 : 4000000000000002 x4 : ffff80027bee4000 x3 : ffff800083b63dc0
>> [    5.377904] x2 : ffff800082694548 x1 : ffff800082694548 x0 : 4000000000000000
>> [    5.378832] Call trace:
>> [    5.379168]  ct_kernel_exit.constprop.0+0xfc/0x118 (P)
>> [    5.379862]  ct_idle_enter+0x10/0x20
>> [    5.380371]  default_idle_call+0x24/0x158
>> [    5.380913]  do_idle+0x20c/0x270
>> [    5.381365]  cpu_startup_entry+0x3c/0x50
>> [    5.381880]  secondary_start_kernel+0x138/0x160
>> [    5.382518]  __secondary_switched+0xc0/0xc8
>> [    5.383091] ---[ end trace 0000000000000000 ]---
>>
>>
>> Thanks,
>> Ryan
>>
>>
>>>
>>> void folio_end_read(struct folio *folio, bool success)
>>> {
>>>     unsigned long mask = 1 << PG_locked;
>>>
>>>     /* Must be in bottom byte for x86 to work */
>>>     BUILD_BUG_ON(PG_uptodate > 7);
>>>     VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
>>>     VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio);
>>>
>>>     if (likely(success))
>>>         mask |= 1 << PG_uptodate;
>>>     if (folio_xor_flags_has_waiters(folio, mask))
>>>         folio_wake_bit(folio, PG_locked);
>>> }
>>> EXPORT_SYMBOL(folio_end_read);
>>>
>>> Thanks
>>>
>>> Phillip
>>>
>>> The oops is
>>>
>>> [  977.270664][ T7696] page: refcount:2 mapcount:0 mapping:ffff8880114c1c98
>>> index:0x100 pfn:0x2022c
>>> [  977.271277][ T7696] memcg:ffff8880162c4000
>>> [  977.271501][ T7696] aops:squashfs_aops ino:1f dentry name(?):".tmp_vmlinux2"
>>> [  977.271941][ T7696] flags: 0x200000000000012d(locked|referenced|uptodate|lru|
>>> active|zone=1)
>>> [  977.272375][ T7696] raw: 200000000000012d ffffea0000aeaf08 ffffea00009ae388
>>> ffff8880114c1c98
>>> [  977.272796][ T7696] raw: 0000000000000100 0000000000000000 00000002ffffffff
>>> ffff8880162c4000
>>> [  977.273215][ T7696] page dumped because:
>>> VM_BUG_ON_FOLIO(folio_test_uptodate(folio))
>>> [  977.273600][ T7696] page_owner tracks the page as allocated
>>> [  977.273916][ T7696] page last allocated via order 0, migratetype Movable,
>>> gfp_mask 0x152c4a(GFP_NOFS|__GFP_HIGH0
>>> [  977.274987][ T7696]  post_alloc_hook+0x2d0/0x350
>>> [  977.275233][ T7696]  get_page_from_freelist+0xb39/0x22a0
>>> [  977.275512][ T7696]  __alloc_pages_slowpath.constprop.0+0x2ff/0x2650
>>> [  977.275872][ T7696]  __alloc_pages_noprof+0x3e6/0x480
>>> [  977.276139][ T7696]  __folio_alloc_noprof+0x11/0x90
>>> [  977.276392][ T7696]  page_cache_ra_unbounded+0x2e3/0x750
>>> [  977.276779][ T7696]  page_cache_ra_order+0x8ef/0xc30
>>> [  977.277057][ T7696]  page_cache_async_ra+0x5cb/0x820
>>> [  977.277530][ T7696]  filemap_get_pages+0x105b/0x1bd0
>>> [  977.277827][ T7696]  filemap_read+0x3b6/0xd50
>>> [  977.278058][ T7696]  generic_file_read_iter+0x344/0x450
>>> [  977.278323][ T7696]  __kernel_read+0x3b5/0xb10
>>> [  977.278581][ T7696]  integrity_kernel_read+0x7f/0xb0
>>> [  977.278844][ T7696]  ima_calc_file_hash_tfm+0x2bc/0x3d0
>>> [  977.279131][ T7696]  ima_calc_file_hash+0x1ba/0x490
>>> [  977.279415][ T7696]  ima_collect_measurement+0x848/0x9d0
>>> [  977.279721][ T7696] page last free pid 7695 tgid 7695 stack trace:
>>> [  977.280101][ T7696]  free_unref_page+0x619/0x10e0
>>> [  977.280363][ T7696]  __folio_put+0x31d/0x440
>>> [  977.280603][ T7696]  put_page+0x21d/0x280
>>> [  977.280875][ T7696]  anon_pipe_buf_release+0x11a/0x240
>>> [  977.281171][ T7696]  pipe_read+0x635/0x13b0
>>> [  977.281447][ T7696]  vfs_read+0xa0c/0xba0
>>> [  977.281761][ T7696]  ksys_read+0x1fe/0x240
>>> [  977.282045][ T7696]  do_syscall_64+0x74/0x1c0
>>> [  977.282314][ T7696]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>> [  977.282679][ T7696] ------------[ cut here ]------------
>>> [  977.283000][ T7696] kernel BUG at mm/filemap.c:1535!
>>> [  977.283313][ T7696] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
>>> [  977.283939][ T7696] CPU: 4 UID: 0 PID: 7696 Comm: cat Not tainted 6.13.0-
>>> rc2-00367-gbfe147475f84 #24
>>> [  977.284605][ T7696] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
>>> 1.16.2-debian-1.16.2-1 04/01/2014
>>> [  977.285195][ T7696] RIP: 0010:folio_end_read+0x17b/0x1a0
>>> [  977.285517][ T7696] Code: e8 1a 62 ca ff 48 c7 c6 60 17 d8 8a 48 89 ef e8 2b
>>> d2 0e 00 0f 0b e8 04 62 ca ff 48 c8
>>> [  977.287017][ T7696] RSP: 0018:ffffc90007aa7710 EFLAGS: 00010293
>>> [  977.287424][ T7696] RAX: 0000000000000000 RBX: 0000000000000001 RCX:
>>> ffffc90007aa75b8
>>> [  977.287897][ T7696] RDX: ffff888026320000 RSI: ffffffff81cd65bb RDI:
>>> ffff888026320444
>>> [  977.288356][ T7696] RBP: ffffea0000808b00 R08: 0000000000000000 R09:
>>> fffffbfff1efaf3a
>>> [  977.288781][ T7696] R10: ffffffff8f7d79d7 R11: 0000000000000001 R12:
>>> 0000000000000001
>>> [  977.289166][ T7696] R13: 0000000000000001 R14: 0000000000000001 R15:
>>> ffffea0000ac3440
>>> [  977.289550][ T7696] FS:  00007f7d03bb7700(0000) GS:ffff88802da00000(0000)
>>> knlGS:0000000000000000
>>> [  977.289982][ T7696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [  977.290304][ T7696] CR2: 00007f833c9a6c68 CR3: 0000000011584000 CR4:
>>> 00000000003506f0
>>> [  977.290688][ T7696] Call Trace:
>>> [  977.290852][ T7696]  <TASK>
>>> [  977.290999][ T7696]  ? die+0x31/0x80
>>> [  977.291190][ T7696]  ? do_trap+0x232/0x430
>>> [  977.291400][ T7696]  ? folio_end_read+0x17b/0x1a0
>>> [  977.291644][ T7696]  ? folio_end_read+0x17b/0x1a0
>>> [  977.291912][ T7696]  ? do_error_trap+0xf4/0x230
>>> [  977.292172][ T7696]  ? folio_end_read+0x17b/0x1a0
>>> [  977.292415][ T7696]  ? handle_invalid_op+0x34/0x40
>>> [  977.292660][ T7696]  ? folio_end_read+0x17b/0x1a0
>>> [  977.292904][ T7696]  ? exc_invalid_op+0x2d/0x40
>>> [  977.293140][ T7696]  ? asm_exc_invalid_op+0x1a/0x20
>>> [  977.293392][ T7696]  ? folio_end_read+0x17b/0x1a0
>>> [  977.293635][ T7696]  ? folio_end_read+0x17b/0x1a0
>>> [  977.293882][ T7696]  squashfs_copy_cache+0x1d7/0x550
>>> [  977.294174][ T7696]  squashfs_read_folio+0xa13/0xc00
>>> [  977.294483][ T7696]  ? __pfx_squashfs_read_folio+0x10/0x10
>>> [  977.294811][ T7696]  ? __pfx_squashfs_read_folio+0x10/0x10
>>> [  977.295154][ T7696]  filemap_read_folio+0xc0/0x2a0
>>> [  977.295457][ T7696]  ? __pfx_filemap_read_folio+0x10/0x10
>>> [  977.295819][ T7696]  ? page_cache_sync_ra+0x4b4/0x9c0
>>> [  977.296171][ T7696]  filemap_get_pages+0x155c/0x1bd0
>>> [  977.296473][ T7696]  ? current_time+0x79/0x380
>>> [  977.296744][ T7696]  ? __pfx_filemap_get_pages+0x10/0x10
>>> [  977.297055][ T7696]  filemap_read+0x3b6/0xd50
>>> [  977.297315][ T7696]  ? __pfx_filemap_read+0x10/0x10
>>> [  977.297605][ T7696]  ? pipe_write+0xf9f/0x1ae0
>>> [  977.297854][ T7696]  ? __pfx_pipe_write+0x10/0x10
>>> [  977.298127][ T7696]  ? lock_acquire+0x1b1/0x550
>>> [  977.298391][ T7696]  ? __pfx_autoremove_wake_function+0x10/0x10
>>> [  977.298714][ T7696]  generic_file_read_iter+0x344/0x450
>>> [  977.299008][ T7696]  ? rw_verify_area+0xd0/0x700
>>> [  977.299283][ T7696]  vfs_read+0x83e/0xba0
>>> [  977.299526][ T7696]  ? __pfx_vfs_read+0x10/0x10
>>> [  977.299809][ T7696]  ? __pfx_generic_fadvise+0x10/0x10
>>> [  977.300132][ T7696]  ksys_read+0x122/0x240
>>> [  977.300361][ T7696]  ? __pfx_ksys_read+0x10/0x10
>>> [  977.300630][ T7696]  do_syscall_64+0x74/0x1c0
>>> [  977.300859][ T7696]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>> [  977.301152][ T7696] RIP: 0033:0x7f7d034dbba0
>>> [  977.301372][ T7696] Code: 0b 31 c0 48 83 c4 08 e9 be fe ff ff 48 8d 3d 3f f0
>>> 08 00 e8 e2 ce 01 00 66 90 83 3d 34
>>> [  977.302300][ T7696] RSP: 002b:00007fffbb0b34a8 EFLAGS: 00000246 ORIG_RAX:
>>> 0000000000000000
>>> [  977.302705][ T7696] RAX: ffffffffffffffda RBX: 0000000000008000 RCX:
>>> 00007f7d034dbba0
>>> [  977.303088][ T7696] RDX: 0000000000008000 RSI: 0000000031e9c000 RDI:
>>> 0000000000000003
>>> [  977.303473][ T7696] RBP: 0000000000008000 R08: 0000000000000003 R09:
>>> 00007f7d0344b99a
>>> [  977.303874][ T7696] R10: 0000000000000002 R11: 0000000000000246 R12:
>>> 0000000031e9c000
>>> [  977.304283][ T7696] R13: 0000000000000003 R14: 0000000000000000 R15:
>>> 0000000000008000
>>> [  977.304675][ T7696]  </TASK>
>>> [  977.304827][ T7696] Modules linked in:
>>> [  977.305052][ T7696] ---[ end trace 0000000000000000 ]---
>>> [  977.305346][ T7696] RIP: 0010:folio_end_read+0x17b/0x1a0
>>> [  977.305640][ T7696] Code: e8 1a 62 ca ff 48 c7 c6 60 17 d8 8a 48 89 ef e8 2b
>>> d2 0e 00 0f 0b e8 04 62 ca ff 48 c8
>>> [  977.306747][ T7696] RSP: 0018:ffffc90007aa7710 EFLAGS: 00010293
>>> [  977.307125][ T7696] RAX: 0000000000000000 RBX: 0000000000000001 RCX:
>>> ffffc90007aa75b8
>>> [  977.307610][ T7696] RDX: ffff888026320000 RSI: ffffffff81cd65bb RDI:
>>> ffff888026320444
>>> [  977.308117][ T7696] RBP: ffffea0000808b00 R08: 0000000000000000 R09:
>>> fffffbfff1efaf3a
>>> [  977.308541][ T7696] R10: ffffffff8f7d79d7 R11: 0000000000000001 R12:
>>> 0000000000000001
>>> [  977.308964][ T7696] R13: 0000000000000001 R14: 0000000000000001 R15:
>>> ffffea0000ac3440
>>> [  977.309385][ T7696] FS:  00007f7d03bb7700(0000) GS:ffff88802da00000(0000)
>>> knlGS:0000000000000000
>>> [  977.309842][ T7696] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [  977.310165][ T7696] CR2: 00007f833c9a6c68 CR3: 0000000011584000 CR4:
>>> 00000000003506f0
>>> [  977.310551][ T7696] Kernel panic - not syncing: Fatal exception
>>> [  977.311300][ T7696] Kernel Offset: disabled
>>> [  977.311520][ T7696] Rebooting in 86400 seconds..
>>>  
>>>
>>
> 

Re: [PATCH v2 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Matthew Wilcox]

On Thu, Jan 09, 2025 at 05:34:43PM +0000, Ryan Roberts wrote:
> I started getting a different oops after I fixed this issue. It turns out that
> __filemap_get_folio() returns ERR_PTR() on error whereas
> grab_cache_page_nowait() (used previously) returns NULL. So the continue
> condition needs to change. This fixes both for me:

Hey Ryan, can you try this amalgam of three patches?
If it works out, I'll send the two which aren't yet in akpm's tree to
him.

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index da25d6fa45ce..5ca2baa16dc2 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -400,7 +400,7 @@ void squashfs_copy_cache(struct folio *folio,
 			bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
 		struct folio *push_folio;
 		size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
-		bool uptodate = true;
+		bool updated = false;
 
 		TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
 
@@ -409,15 +409,15 @@ void squashfs_copy_cache(struct folio *folio,
 					FGP_LOCK|FGP_CREAT|FGP_NOFS|FGP_NOWAIT,
 					mapping_gfp_mask(mapping));
 
-		if (!push_folio)
+		if (IS_ERR(push_folio))
 			continue;
 
 		if (folio_test_uptodate(push_folio))
 			goto skip_folio;
 
-		uptodate = squashfs_fill_page(push_folio, buffer, offset, avail);
+		updated = squashfs_fill_page(push_folio, buffer, offset, avail);
 skip_folio:
-		folio_end_read(push_folio, uptodate);
+		folio_end_read(push_folio, updated);
 		if (i != folio->index)
 			folio_put(push_folio);
 	}
diff --git a/mm/filemap.c b/mm/filemap.c
index 12ba297bb85e..3b1eefa9aab8 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1505,7 +1505,7 @@ void folio_end_read(struct folio *folio, bool success)
 	/* Must be in bottom byte for x86 to work */
 	BUILD_BUG_ON(PG_uptodate > 7);
 	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
-	VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio);
+	VM_BUG_ON_FOLIO(success && folio_test_uptodate(folio), folio);
 
 	if (likely(success))
 		mask |= 1 << PG_uptodate;

Re: [PATCH v2 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Ryan Roberts]

On 10/01/2025 02:05, Matthew Wilcox wrote:
> On Thu, Jan 09, 2025 at 05:34:43PM +0000, Ryan Roberts wrote:
>> I started getting a different oops after I fixed this issue. It turns out that
>> __filemap_get_folio() returns ERR_PTR() on error whereas
>> grab_cache_page_nowait() (used previously) returns NULL. So the continue
>> condition needs to change. This fixes both for me:
> 
> Hey Ryan, can you try this amalgam of three patches?
> If it works out, I'll send the two which aren't yet in akpm's tree to
> him.

Yep that fixes it. Feel free to add:

Tested-by: Ryan Roberts <ryan.roberts@arm.com>


> 
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index da25d6fa45ce..5ca2baa16dc2 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -400,7 +400,7 @@ void squashfs_copy_cache(struct folio *folio,
>  			bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
>  		struct folio *push_folio;
>  		size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
> -		bool uptodate = true;
> +		bool updated = false;
>  
>  		TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
>  
> @@ -409,15 +409,15 @@ void squashfs_copy_cache(struct folio *folio,
>  					FGP_LOCK|FGP_CREAT|FGP_NOFS|FGP_NOWAIT,
>  					mapping_gfp_mask(mapping));
>  
> -		if (!push_folio)
> +		if (IS_ERR(push_folio))
>  			continue;
>  
>  		if (folio_test_uptodate(push_folio))
>  			goto skip_folio;
>  
> -		uptodate = squashfs_fill_page(push_folio, buffer, offset, avail);
> +		updated = squashfs_fill_page(push_folio, buffer, offset, avail);
>  skip_folio:
> -		folio_end_read(push_folio, uptodate);
> +		folio_end_read(push_folio, updated);
>  		if (i != folio->index)
>  			folio_put(push_folio);
>  	}
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 12ba297bb85e..3b1eefa9aab8 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1505,7 +1505,7 @@ void folio_end_read(struct folio *folio, bool success)
>  	/* Must be in bottom byte for x86 to work */
>  	BUILD_BUG_ON(PG_uptodate > 7);
>  	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
> -	VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio);
> +	VM_BUG_ON_FOLIO(success && folio_test_uptodate(folio), folio);
>  
>  	if (likely(success))
>  		mask |= 1 << PG_uptodate;

Re: [PATCH v2 1/5] squashfs: Use a folio throughout squashfs_read_folio()

[Phillip Lougher]

On 12/20/24 22:46, Matthew Wilcox (Oracle) wrote:
> Use modern folio APIs where they exist and convert back to struct
> page for the internal functions.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Phillip Lougher <phillip@squashfs.org.uk>
Tested-by: Phillip Lougher <phillip@squashfs.org.uk>

> ---
>   fs/squashfs/file.c | 25 +++++++++----------------
>   1 file changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index 21aaa96856c1..bc6598c3a48f 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -445,21 +445,19 @@ static int squashfs_readpage_sparse(struct page *page, int expected)
>   
>   static int squashfs_read_folio(struct file *file, struct folio *folio)
>   {
> -	struct page *page = &folio->page;
> -	struct inode *inode = page->mapping->host;
> +	struct inode *inode = folio->mapping->host;
>   	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
> -	int index = page->index >> (msblk->block_log - PAGE_SHIFT);
> +	int index = folio->index >> (msblk->block_log - PAGE_SHIFT);
>   	int file_end = i_size_read(inode) >> msblk->block_log;
>   	int expected = index == file_end ?
>   			(i_size_read(inode) & (msblk->block_size - 1)) :
>   			 msblk->block_size;
>   	int res = 0;
> -	void *pageaddr;
>   
>   	TRACE("Entered squashfs_readpage, page index %lx, start block %llx\n",
> -				page->index, squashfs_i(inode)->start);
> +				folio->index, squashfs_i(inode)->start);
>   
> -	if (page->index >= ((i_size_read(inode) + PAGE_SIZE - 1) >>
> +	if (folio->index >= ((i_size_read(inode) + PAGE_SIZE - 1) >>
>   					PAGE_SHIFT))
>   		goto out;
>   
> @@ -472,23 +470,18 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
>   			goto out;
>   
>   		if (res == 0)
> -			res = squashfs_readpage_sparse(page, expected);
> +			res = squashfs_readpage_sparse(&folio->page, expected);
>   		else
> -			res = squashfs_readpage_block(page, block, res, expected);
> +			res = squashfs_readpage_block(&folio->page, block, res, expected);
>   	} else
> -		res = squashfs_readpage_fragment(page, expected);
> +		res = squashfs_readpage_fragment(&folio->page, expected);
>   
>   	if (!res)
>   		return 0;
>   
>   out:
> -	pageaddr = kmap_atomic(page);
> -	memset(pageaddr, 0, PAGE_SIZE);
> -	kunmap_atomic(pageaddr);
> -	flush_dcache_page(page);
> -	if (res == 0)
> -		SetPageUptodate(page);
> -	unlock_page(page);
> +	folio_zero_segment(folio, 0, folio_size(folio));
> +	folio_end_read(folio, res == 0);
>   
>   	return res;
>   }

Re: [PATCH v2 2/5] squashfs: Pass a folio to squashfs_readpage_fragment()

[Phillip Lougher]

On 12/20/24 22:46, Matthew Wilcox (Oracle) wrote:
> Remove an access to page->mapping.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Phillip Lougher <phillip@squashfs.org.uk>
Tested-by: Phillip Lougher <phillip@squashfs.org.uk>

> ---
>   fs/squashfs/file.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index bc6598c3a48f..6bd16e12493b 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -417,9 +417,9 @@ void squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
>   }
>   
>   /* Read datablock stored packed inside a fragment (tail-end packed block) */
> -static int squashfs_readpage_fragment(struct page *page, int expected)
> +static int squashfs_readpage_fragment(struct folio *folio, int expected)
>   {
> -	struct inode *inode = page->mapping->host;
> +	struct inode *inode = folio->mapping->host;
>   	struct squashfs_cache_entry *buffer = squashfs_get_fragment(inode->i_sb,
>   		squashfs_i(inode)->fragment_block,
>   		squashfs_i(inode)->fragment_size);
> @@ -430,7 +430,7 @@ static int squashfs_readpage_fragment(struct page *page, int expected)
>   			squashfs_i(inode)->fragment_block,
>   			squashfs_i(inode)->fragment_size);
>   	else
> -		squashfs_copy_cache(page, buffer, expected,
> +		squashfs_copy_cache(&folio->page, buffer, expected,
>   			squashfs_i(inode)->fragment_offset);
>   
>   	squashfs_cache_put(buffer);
> @@ -474,7 +474,7 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
>   		else
>   			res = squashfs_readpage_block(&folio->page, block, res, expected);
>   	} else
> -		res = squashfs_readpage_fragment(&folio->page, expected);
> +		res = squashfs_readpage_fragment(folio, expected);
>   
>   	if (!res)
>   		return 0;

Re: [PATCH v2 3/5] squashfs: Convert squashfs_readpage_block() to take a folio

[Phillip Lougher]

On 12/20/24 22:46, Matthew Wilcox (Oracle) wrote:
> Remove a few accesses to page->mapping.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Phillip Lougher <phillip@squashfs.org.uk>
Tested-by: Phillip Lougher <phillip@squashfs.org.uk>

> ---
>   fs/squashfs/file.c        |  2 +-
>   fs/squashfs/file_cache.c  |  6 +++---
>   fs/squashfs/file_direct.c | 11 +++++------
>   fs/squashfs/squashfs.h    |  2 +-
>   4 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index 6bd16e12493b..5b81e26b1226 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -472,7 +472,7 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
>   		if (res == 0)
>   			res = squashfs_readpage_sparse(&folio->page, expected);
>   		else
> -			res = squashfs_readpage_block(&folio->page, block, res, expected);
> +			res = squashfs_readpage_block(folio, block, res, expected);
>   	} else
>   		res = squashfs_readpage_fragment(folio, expected);
>   
> diff --git a/fs/squashfs/file_cache.c b/fs/squashfs/file_cache.c
> index 54c17b7c85fd..0360d22a77d4 100644
> --- a/fs/squashfs/file_cache.c
> +++ b/fs/squashfs/file_cache.c
> @@ -18,9 +18,9 @@
>   #include "squashfs.h"
>   
>   /* Read separately compressed datablock and memcopy into page cache */
> -int squashfs_readpage_block(struct page *page, u64 block, int bsize, int expected)
> +int squashfs_readpage_block(struct folio *folio, u64 block, int bsize, int expected)
>   {
> -	struct inode *i = page->mapping->host;
> +	struct inode *i = folio->mapping->host;
>   	struct squashfs_cache_entry *buffer = squashfs_get_datablock(i->i_sb,
>   		block, bsize);
>   	int res = buffer->error;
> @@ -29,7 +29,7 @@ int squashfs_readpage_block(struct page *page, u64 block, int bsize, int expecte
>   		ERROR("Unable to read page, block %llx, size %x\n", block,
>   			bsize);
>   	else
> -		squashfs_copy_cache(page, buffer, expected, 0);
> +		squashfs_copy_cache(&folio->page, buffer, expected, 0);
>   
>   	squashfs_cache_put(buffer);
>   	return res;
> diff --git a/fs/squashfs/file_direct.c b/fs/squashfs/file_direct.c
> index d19d4db74af8..2c3e809d6891 100644
> --- a/fs/squashfs/file_direct.c
> +++ b/fs/squashfs/file_direct.c
> @@ -19,12 +19,11 @@
>   #include "page_actor.h"
>   
>   /* Read separately compressed datablock directly into page cache */
> -int squashfs_readpage_block(struct page *target_page, u64 block, int bsize,
> -	int expected)
> -
> +int squashfs_readpage_block(struct folio *folio, u64 block, int bsize,
> +		int expected)
>   {
> -	struct folio *folio = page_folio(target_page);
> -	struct inode *inode = target_page->mapping->host;
> +	struct page *target_page = &folio->page;
> +	struct inode *inode = folio->mapping->host;
>   	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
>   	loff_t file_end = (i_size_read(inode) - 1) >> PAGE_SHIFT;
>   	int mask = (1 << (msblk->block_log - PAGE_SHIFT)) - 1;
> @@ -48,7 +47,7 @@ int squashfs_readpage_block(struct page *target_page, u64 block, int bsize,
>   	/* Try to grab all the pages covered by the Squashfs block */
>   	for (i = 0, index = start_index; index <= end_index; index++) {
>   		page[i] = (index == folio->index) ? target_page :
> -			grab_cache_page_nowait(target_page->mapping, index);
> +			grab_cache_page_nowait(folio->mapping, index);
>   
>   		if (page[i] == NULL)
>   			continue;
> diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
> index 5a756e6790b5..0f5373479516 100644
> --- a/fs/squashfs/squashfs.h
> +++ b/fs/squashfs/squashfs.h
> @@ -72,7 +72,7 @@ void squashfs_copy_cache(struct page *, struct squashfs_cache_entry *, int,
>   				int);
>   
>   /* file_xxx.c */
> -extern int squashfs_readpage_block(struct page *, u64, int, int);
> +int squashfs_readpage_block(struct folio *, u64 block, int bsize, int expected);
>   
>   /* id.c */
>   extern int squashfs_get_id(struct super_block *, unsigned int, unsigned int *);

Re: [PATCH v2 4/5] squashfs; Convert squashfs_copy_cache() to take a folio

[Phillip Lougher]

On 12/20/24 22:46, Matthew Wilcox (Oracle) wrote:
> Remove accesses to page->index and page->mapping.  Also use folio
> APIs where available.  This code still assumes order 0 folios.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Phillip Lougher <phillip@squashfs.org.uk>
Tested-by: Phillip Lougher <phillip@squashfs.org.uk>

> ---
>   fs/squashfs/file.c       | 46 ++++++++++++++++++++++------------------
>   fs/squashfs/file_cache.c |  2 +-
>   fs/squashfs/squashfs.h   |  4 ++--
>   3 files changed, 28 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index 5b81e26b1226..1f27e8161319 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -378,13 +378,15 @@ void squashfs_fill_page(struct page *page, struct squashfs_cache_entry *buffer,
>   }
>   
>   /* Copy data into page cache  */
> -void squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
> -	int bytes, int offset)
> +void squashfs_copy_cache(struct folio *folio,
> +		struct squashfs_cache_entry *buffer, size_t bytes,
> +		size_t offset)
>   {
> -	struct inode *inode = page->mapping->host;
> +	struct address_space *mapping = folio->mapping;
> +	struct inode *inode = mapping->host;
>   	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
>   	int i, mask = (1 << (msblk->block_log - PAGE_SHIFT)) - 1;
> -	int start_index = page->index & ~mask, end_index = start_index | mask;
> +	int start_index = folio->index & ~mask, end_index = start_index | mask;
>   
>   	/*
>   	 * Loop copying datablock into pages.  As the datablock likely covers
> @@ -394,25 +396,27 @@ void squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
>   	 */
>   	for (i = start_index; i <= end_index && bytes > 0; i++,
>   			bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
> -		struct page *push_page;
> -		int avail = buffer ? min_t(int, bytes, PAGE_SIZE) : 0;
> +		struct folio *push_folio;
> +		size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
>   
> -		TRACE("bytes %d, i %d, available_bytes %d\n", bytes, i, avail);
> +		TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
>   
> -		push_page = (i == page->index) ? page :
> -			grab_cache_page_nowait(page->mapping, i);
> +		push_folio = (i == folio->index) ? folio :
> +			__filemap_get_folio(mapping, i,
> +					FGP_LOCK|FGP_CREAT|FGP_NOFS|FGP_NOWAIT,
> +					mapping_gfp_mask(mapping));
>   
> -		if (!push_page)
> +		if (!push_folio)
>   			continue;
>   
> -		if (PageUptodate(push_page))
> -			goto skip_page;
> +		if (folio_test_uptodate(push_folio))
> +			goto skip_folio;
>   
> -		squashfs_fill_page(push_page, buffer, offset, avail);
> -skip_page:
> -		unlock_page(push_page);
> -		if (i != page->index)
> -			put_page(push_page);
> +		squashfs_fill_page(&push_folio->page, buffer, offset, avail);
> +skip_folio:
> +		folio_unlock(push_folio);
> +		if (i != folio->index)
> +			folio_put(push_folio);
>   	}
>   }
>   
> @@ -430,16 +434,16 @@ static int squashfs_readpage_fragment(struct folio *folio, int expected)
>   			squashfs_i(inode)->fragment_block,
>   			squashfs_i(inode)->fragment_size);
>   	else
> -		squashfs_copy_cache(&folio->page, buffer, expected,
> +		squashfs_copy_cache(folio, buffer, expected,
>   			squashfs_i(inode)->fragment_offset);
>   
>   	squashfs_cache_put(buffer);
>   	return res;
>   }
>   
> -static int squashfs_readpage_sparse(struct page *page, int expected)
> +static int squashfs_readpage_sparse(struct folio *folio, int expected)
>   {
> -	squashfs_copy_cache(page, NULL, expected, 0);
> +	squashfs_copy_cache(folio, NULL, expected, 0);
>   	return 0;
>   }
>   
> @@ -470,7 +474,7 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
>   			goto out;
>   
>   		if (res == 0)
> -			res = squashfs_readpage_sparse(&folio->page, expected);
> +			res = squashfs_readpage_sparse(folio, expected);
>   		else
>   			res = squashfs_readpage_block(folio, block, res, expected);
>   	} else
> diff --git a/fs/squashfs/file_cache.c b/fs/squashfs/file_cache.c
> index 0360d22a77d4..40e59a43d098 100644
> --- a/fs/squashfs/file_cache.c
> +++ b/fs/squashfs/file_cache.c
> @@ -29,7 +29,7 @@ int squashfs_readpage_block(struct folio *folio, u64 block, int bsize, int expec
>   		ERROR("Unable to read page, block %llx, size %x\n", block,
>   			bsize);
>   	else
> -		squashfs_copy_cache(&folio->page, buffer, expected, 0);
> +		squashfs_copy_cache(folio, buffer, expected, 0);
>   
>   	squashfs_cache_put(buffer);
>   	return res;
> diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
> index 0f5373479516..9295556ecfd0 100644
> --- a/fs/squashfs/squashfs.h
> +++ b/fs/squashfs/squashfs.h
> @@ -68,8 +68,8 @@ extern __le64 *squashfs_read_fragment_index_table(struct super_block *,
>   
>   /* file.c */
>   void squashfs_fill_page(struct page *, struct squashfs_cache_entry *, int, int);
> -void squashfs_copy_cache(struct page *, struct squashfs_cache_entry *, int,
> -				int);
> +void squashfs_copy_cache(struct folio *, struct squashfs_cache_entry *,
> +		size_t bytes, size_t offset);
>   
>   /* file_xxx.c */
>   int squashfs_readpage_block(struct folio *, u64 block, int bsize, int expected);

Re: [PATCH v2 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Phillip Lougher]

On 12/20/24 22:46, Matthew Wilcox (Oracle) wrote:
> squashfs_fill_page is only used in this file, so make it static.
> Use kmap_local instead of kmap_atomic, and return a bool so that
> the caller can use folio_end_read() which saves an atomic operation
> over calling folio_mark_uptodate() followed by folio_unlock().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Phillip Lougher <phillip@squashfs.org.uk>
Tested-by: Phillip Lougher <phillip@squashfs.org.uk>

> ---
>   fs/squashfs/file.c     | 21 ++++++++++++---------
>   fs/squashfs/squashfs.h |  1 -
>   2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
> index 1f27e8161319..da25d6fa45ce 100644
> --- a/fs/squashfs/file.c
> +++ b/fs/squashfs/file.c
> @@ -362,19 +362,21 @@ static int read_blocklist(struct inode *inode, int index, u64 *block)
>   	return squashfs_block_size(size);
>   }
>   
> -void squashfs_fill_page(struct page *page, struct squashfs_cache_entry *buffer, int offset, int avail)
> +static bool squashfs_fill_page(struct folio *folio,
> +		struct squashfs_cache_entry *buffer, size_t offset,
> +		size_t avail)
>   {
> -	int copied;
> +	size_t copied;
>   	void *pageaddr;
>   
> -	pageaddr = kmap_atomic(page);
> +	pageaddr = kmap_local_folio(folio, 0);
>   	copied = squashfs_copy_data(pageaddr, buffer, offset, avail);
>   	memset(pageaddr + copied, 0, PAGE_SIZE - copied);
> -	kunmap_atomic(pageaddr);
> +	kunmap_local(pageaddr);
>   
> -	flush_dcache_page(page);
> -	if (copied == avail)
> -		SetPageUptodate(page);
> +	flush_dcache_folio(folio);
> +
> +	return copied == avail;
>   }
>   
>   /* Copy data into page cache  */
> @@ -398,6 +400,7 @@ void squashfs_copy_cache(struct folio *folio,
>   			bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
>   		struct folio *push_folio;
>   		size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
> +		bool uptodate = true;
>   
>   		TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
>   
> @@ -412,9 +415,9 @@ void squashfs_copy_cache(struct folio *folio,
>   		if (folio_test_uptodate(push_folio))
>   			goto skip_folio;
>   
> -		squashfs_fill_page(&push_folio->page, buffer, offset, avail);
> +		uptodate = squashfs_fill_page(push_folio, buffer, offset, avail);
>   skip_folio:
> -		folio_unlock(push_folio);
> +		folio_end_read(push_folio, uptodate);
>   		if (i != folio->index)
>   			folio_put(push_folio);
>   	}
> diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
> index 9295556ecfd0..37f3518a804a 100644
> --- a/fs/squashfs/squashfs.h
> +++ b/fs/squashfs/squashfs.h
> @@ -67,7 +67,6 @@ extern __le64 *squashfs_read_fragment_index_table(struct super_block *,
>   				u64, u64, unsigned int);
>   
>   /* file.c */
> -void squashfs_fill_page(struct page *, struct squashfs_cache_entry *, int, int);
>   void squashfs_copy_cache(struct folio *, struct squashfs_cache_entry *,
>   		size_t bytes, size_t offset);
>