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

[PATCH 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 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 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 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 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..d363fb26c2c8 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 filled = false;
 
 		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);
+		filled = squashfs_fill_page(push_folio, buffer, offset, avail);
 skip_folio:
-		folio_unlock(push_folio);
+		folio_end_read(folio, filled);
 		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 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Phillip Lougher]

On 16/12/2024 16:26, 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>

Hi Matthew,

This patch causes 2 oops (VM_BUG_ON_FOLIO).

The cause and fix for the first oops follows (second oops in the next
email).

> ---
>   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..d363fb26c2c8 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 filled = false;
>   
>   		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);
> +		filled = squashfs_fill_page(push_folio, buffer, offset, avail);
>   skip_folio:
> -		folio_unlock(push_folio);
> +		folio_end_read(folio, filled);

This should be

		folio_end_read(push_folio, filled)

Without this folio_end_read() is repeatedly called on folio,
generating the following oops

[  187.271288][ T8043] page: refcount:2 mapcount:0 mapping:ffff888116ae0798 index:0x100 pfn:0x1296c7
[  187.272129][ T8043] memcg:ffff8881016cc000
[  187.272458][ T8043] aops:squashfs_aops ino:1b dentry name(?):".tmp_vmlinux1"
[  187.273013][ T8043] flags: 0x4000000000000028(uptodate|lru|zone=2)
[  187.273500][ T8043] raw: 4000000000000028 ffffea0004655308 ffffea0004ac2e08 ffff888116ae0798
[  187.274160][ T8043] raw: 0000000000000100 0000000000000000 00000002ffffffff ffff8881016cc000
[  187.274809][ T8043] page dumped because: VM_BUG_ON_FOLIO(!folio_test_locked(folio))
[  187.275348][ T8043] page_owner tracks the page as allocated
[  187.275787][ T8043] page last allocated via order 0, migratetype Movable, gfp_mask 0x152c4a(GFP_N0
[  187.277262][ T8043]  post_alloc_hook+0x2d0/0x350
[  187.277586][ T8043]  get_page_from_freelist+0xb39/0x22a0
[  187.277941][ T8043]  __alloc_pages_noprof+0x1bc/0x480
[  187.278279][ T8043]  __folio_alloc_noprof+0x11/0x90
[  187.278600][ T8043]  page_cache_ra_unbounded+0x2e3/0x750
[  187.278939][ T8043]  page_cache_ra_order+0x8ef/0xc30
[  187.279252][ T8043]  page_cache_async_ra+0x5cb/0x820
[  187.279567][ T8043]  filemap_get_pages+0x105b/0x1bd0
[  187.279880][ T8043]  filemap_read+0x3b6/0xd50
[  187.280157][ T8043]  generic_file_read_iter+0x344/0x450
[  187.280481][ T8043]  __kernel_read+0x3b5/0xb10
[  187.280774][ T8043]  integrity_kernel_read+0x7f/0xb0
[  187.281091][ T8043]  ima_calc_file_hash_tfm+0x2bc/0x3d0
[  187.281425][ T8043]  ima_calc_file_hash+0x1ba/0x490
[  187.281756][ T8043]  ima_collect_measurement+0x848/0x9d0
[  187.282088][ T8043]  process_measurement+0x126b/0x2390
[  187.282409][ T8043] page last free pid 4949 tgid 4949 stack trace:
[  187.282788][ T8043]  free_unref_folios+0x8ea/0x13e0
[  187.283095][ T8043]  folios_put_refs+0x589/0x7b0
[  187.283386][ T8043]  free_pages_and_swap_cache+0x23d/0x490
[  187.283727][ T8043]  __tlb_batch_free_encoded_pages+0xf9/0x290
[  187.284115][ T8043]  tlb_finish_mmu+0x168/0x7b0
[  187.284438][ T8043]  vms_clear_ptes.part.0+0x4b7/0x690
[  187.284853][ T8043]  vms_complete_munmap_vmas+0x6c3/0x9e0
[  187.285276][ T8043]  do_vmi_align_munmap+0x600/0x870
[  187.285679][ T8043]  __do_sys_brk+0x888/0xa50
[  187.286001][ T8043]  do_syscall_64+0x74/0x1c0
[  187.286325][ T8043]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  187.286768][ T8043] ------------[ cut here ]------------
[  187.287114][ T8043] kernel BUG at mm/filemap.c:1534!
[  187.287464][ T8043] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
[  187.287979][ T8043] CPU: 13 UID: 0 PID: 8043 Comm: wc Not tainted 6.13.0-rc2-00368-g673506e97177 5
[  187.288717][ T8043] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.24
[  187.289495][ T8043] RIP: 0010:folio_end_read+0x165/0x1a0
[  187.289910][ T8043] Code: 89 ef 31 f6 e8 1c c6 ff ff 5b 5d 41 5c 41 5d 41 5e e9 1f 62 ca ff e8 1a0
[  187.291452][ T8043] RSP: 0018:ffffc9000a0cea00 EFLAGS: 00010293
[  187.291867][ T8043] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffc9000a0ce8a8
[  187.292442][ T8043] RDX: ffff888118375880 RSI: ffffffff81cd65a5 RDI: ffff888118375cc4
[  187.293006][ T8043] RBP: ffffea0004a5b1c0 R08: 0000000000000000 R09: fffffbfff1efaefa
[  187.293632][ T8043] R10: ffffffff8f7d77d7 R11: 0000000000000001 R12: 0000000000000001
[  187.294228][ T8043] R13: 0000000000000001 R14: 0000000000000101 R15: ffffea0004a5b1c0
[  187.294808][ T8043] FS:  00007f2509ad0700(0000) GS:ffff888159480000(0000) knlGS:0000000000000000
[  187.295485][ T8043] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  187.295993][ T8043] CR2: 00007fff92c60000 CR3: 0000000124c28000 CR4: 00000000003506f0
[  187.296600][ T8043] Call Trace:
[  187.296837][ T8043]  <TASK>
[  187.297052][ T8043]  ? die+0x31/0x80
[  187.297332][ T8043]  ? do_trap+0x232/0x430
[  187.297668][ T8043]  ? folio_end_read+0x165/0x1a0
[  187.298045][ T8043]  ? folio_end_read+0x165/0x1a0
[  187.298414][ T8043]  ? do_error_trap+0xf4/0x230
[  187.298741][ T8043]  ? folio_end_read+0x165/0x1a0
[  187.299130][ T8043]  ? handle_invalid_op+0x34/0x40
[  187.299442][ T8043]  ? folio_end_read+0x165/0x1a0
[  187.299748][ T8043]  ? exc_invalid_op+0x2d/0x40
[  187.300104][ T8043]  ? asm_exc_invalid_op+0x1a/0x20
[  187.300487][ T8043]  ? folio_end_read+0x165/0x1a0
[  187.300821][ T8043]  ? folio_end_read+0x165/0x1a0
[  187.301164][ T8043]  squashfs_copy_cache+0x1d4/0x540
[  187.301532][ T8043]  squashfs_read_folio+0xa13/0xc00
[  187.301869][ T8043]  ? __pfx_squashfs_read_folio+0x10/0x10
[  187.302219][ T8043]  ? __pfx_squashfs_read_folio+0x10/0x10
[  187.302566][ T8043]  filemap_read_folio+0xc0/0x2a0
[  187.302886][ T8043]  ? __pfx_filemap_read_folio+0x10/0x10
[  187.303257][ T8043]  ? page_cache_async_ra+0x5cb/0x820
[  187.303624][ T8043]  filemap_get_pages+0x155c/0x1bd0
[  187.303968][ T8043]  ? __pfx_make_vfsgid+0x10/0x10
[  187.304279][ T8043]  ? __pfx_filemap_get_pages+0x10/0x10
[  187.304626][ T8043]  filemap_read+0x3b6/0xd50
[  187.304944][ T8043]  ? __pfx_filemap_read+0x10/0x10
[  187.305280][ T8043]  ? kernel_text_address+0x8d/0x100
[  187.305643][ T8043]  ? pvh_start_xen+0x1/0x108
[  187.305964][ T8043]  ? __kernel_text_address+0xd/0x40
[  187.306309][ T8043]  ? unwind_get_return_address+0x59/0xa0
[  187.306697][ T8043]  ? __pfx_stack_trace_consume_entry+0x10/0x10
[  187.307104][ T8043]  ? arch_stack_walk+0x9d/0xf0
[  187.307413][ T8043]  ? stack_trace_save+0x95/0xd0
[  187.307737][ T8043]  ? __pfx_stack_trace_save+0x10/0x10
[  187.308133][ T8043]  ? stack_depot_save_flags+0x28/0x9d0
[  187.308652][ T8043]  generic_file_read_iter+0x344/0x450
[  187.309054][ T8043]  __kernel_read+0x3b5/0xb10
[  187.309430][ T8043]  ? __pfx___kernel_read+0x10/0x10
[  187.309855][ T8043]  integrity_kernel_read+0x7f/0xb0
[  187.310279][ T8043]  ? __pfx_integrity_kernel_read+0x10/0x10
[  187.310737][ T8043]  ? _sha256_update+0x93/0x220
[  187.311140][ T8043]  ? __pfx_sha256_transform_rorx+0x10/0x10
[  187.311579][ T8043]  ? kasan_save_track+0x14/0x30
[  187.311956][ T8043]  ima_calc_file_hash_tfm+0x2bc/0x3d0
[  187.312362][ T8043]  ? __pfx_ima_calc_file_hash_tfm+0x10/0x10
[  187.312780][ T8043]  ? squashfs_xattr_handler_get+0x522/0x6c0
[  187.313171][ T8043]  ? vfs_getxattr_alloc+0x1b9/0x340
[  187.313541][ T8043]  ? ima_read_xattr+0x38/0x60
[  187.313868][ T8043]  ? process_measurement+0x11e2/0x2390
[  187.314237][ T8043]  ? ima_file_check+0xb4/0x100
[  187.314556][ T8043]  ? security_file_post_open+0x8e/0x210
[  187.314933][ T8043]  ? path_openat+0x13f9/0x2d50
[  187.315261][ T8043]  ? mark_lock+0x101/0x1780
[  187.315558][ T8043]  ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  187.315949][ T8043]  ? mark_held_locks+0x9f/0xe0
[  187.316266][ T8043]  ? __pfx_mark_lock+0x10/0x10
[  187.316590][ T8043]  ? lockdep_hardirqs_on+0x7c/0x100
[  187.317283][ T8043]  ? _raw_spin_unlock_irqrestore+0x3b/0x80
[  187.317722][ T8043]  ? debug_check_no_obj_freed+0x2eb/0x5c0
[  187.318090][ T8043]  ? make_vfsgid+0xf2/0x140
[  187.318389][ T8043]  ? __pfx_make_vfsgid+0x10/0x10
[  187.318719][ T8043]  ? __pfx_debug_check_no_obj_freed+0x10/0x10
[  187.319113][ T8043]  ? ima_alloc_tfm+0x21d/0x2d0
[  187.319430][ T8043]  ? generic_fillattr+0x6bf/0x940
[  187.319762][ T8043]  ima_calc_file_hash+0x1ba/0x490
[  187.320096][ T8043]  ima_collect_measurement+0x848/0x9d0
[  187.320459][ T8043]  ? __pfx_ima_collect_measurement+0x10/0x10
[  187.320855][ T8043]  ? squashfs_xattr_handler_get+0x527/0x6c0
[  187.321253][ T8043]  ? process_measurement+0x86d/0x2390
[  187.321630][ T8043]  ? find_held_lock+0x2d/0x110
[  187.321973][ T8043]  ? xattr_resolve_name+0x27b/0x3f0
[  187.322333][ T8043]  ? __pfx_squashfs_xattr_handler_get+0x10/0x10
[  187.322762][ T8043]  ? vfs_getxattr_alloc+0xf1/0x340
[  187.323118][ T8043]  ? ima_get_hash_algo+0x27d/0x410
[  187.323474][ T8043]  ? __pfx_ima_get_hash_algo+0x10/0x10
[  187.323895][ T8043]  ? process_measurement+0x126b/0x2390
[  187.324268][ T8043]  process_measurement+0x126b/0x2390
[  187.324661][ T8043]  ? __pfx_process_measurement+0x10/0x10
[  187.325046][ T8043]  ? tomoyo_check_open_permission+0x1f0/0x3a0
[  187.325463][ T8043]  ? __pfx_smack_log+0x10/0x10
[  187.325796][ T8043]  ? smk_access+0x39a/0x5e0
[  187.326105][ T8043]  ? smk_tskacc+0x340/0x430
[  187.326409][ T8043]  ? smack_file_open+0x1c1/0x250
[  187.326741][ T8043]  ? __pfx_smack_file_open+0x10/0x10
[  187.327087][ T8043]  ? inode_to_bdi+0x9e/0x160
[  187.327389][ T8043]  ima_file_check+0xb4/0x100
[  187.327687][ T8043]  ? __pfx_ima_file_check+0x10/0x10
[  187.328056][ T8043]  security_file_post_open+0x8e/0x210
[  187.328427][ T8043]  path_openat+0x13f9/0x2d50
[  187.328740][ T8043]  ? __pfx_path_openat+0x10/0x10
[  187.329057][ T8043]  ? __pfx___lock_acquire+0x10/0x10
[  187.329402][ T8043]  ? lock_acquire+0x1b1/0x550
[  187.329732][ T8043]  ? find_held_lock+0x2d/0x110
[  187.330052][ T8043]  do_filp_open+0x1f8/0x460
[  187.330343][ T8043]  ? __pfx_do_filp_open+0x10/0x10
[  187.330666][ T8043]  ? find_held_lock+0x2d/0x110
[  187.330979][ T8043]  ? do_raw_spin_lock+0x12d/0x2b0
[  187.331318][ T8043]  ? __pfx_do_raw_spin_lock+0x10/0x10
[  187.331661][ T8043]  ? __phys_addr_symbol+0x30/0x70
[  187.331999][ T8043]  ? __check_object_size+0x4f7/0x7d0
[  187.332349][ T8043]  ? _raw_spin_unlock+0x28/0x50
[  187.332662][ T8043]  ? alloc_fd+0x41f/0x760
[  187.332963][ T8043]  do_sys_openat2+0x160/0x1c0
[  187.333296][ T8043]  ? __pfx_do_sys_openat2+0x10/0x10
[  187.333655][ T8043]  ? kmem_cache_free+0x39e/0x5b0
[  187.333979][ T8043]  ? security_file_free+0xb9/0x180
[  187.334304][ T8043]  ? __fput+0x686/0xb60
[  187.334561][ T8043]  __x64_sys_open+0x154/0x1e0
[  187.334855][ T8043]  ? __pfx___x64_sys_open+0x10/0x10
[  187.335209][ T8043]  do_syscall_64+0x74/0x1c0
[  187.335503][ T8043]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  187.335882][ T8043] RIP: 0033:0x7f25094db960
[  187.336176][ T8043] Code: 48 8b 15 2b 95 2c 00 f7 d8 64 89 02 48 83 c8 ff c3 66 0f 1f 84 00 00 004
[  187.337420][ T8043] RSP: 002b:00007fff92c63888 EFLAGS: 00000246 ORIG_RAX: 0000000000000002
[  187.337967][ T8043] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f25094db960
[  187.338505][ T8043] RDX: 00007f2509a74e50 RSI: 0000000000000000 RDI: 00007fff92c6a173
[  187.339015][ T8043] RBP: 00007fff92c6a173 R08: 6f2e736d79736c6c R09: 00007f250944b99a
[  187.339511][ T8043] R10: 00007f25097a46a0 R11: 0000000000000246 R12: 00007f2509a74e50
[  187.340044][ T8043] R13: 0000000000000993 R14: 0000000000000001 R15: 00007fff92c6a173
[  187.340523][ T8043]  </TASK>
[  187.340707][ T8043] Modules linked in:
[  187.340976][ T8043] ---[ end trace 0000000000000000 ]---
[  187.341417][ T8043] RIP: 0010:folio_end_read+0x165/0x1a0
[  187.341807][ T8043] Code: 89 ef 31 f6 e8 1c c6 ff ff 5b 5d 41 5c 41 5d 41 5e e9 1f 62 ca ff e8 1a0
[  187.343080][ T8043] RSP: 0018:ffffc9000a0cea00 EFLAGS: 00010293
[  187.343461][ T8043] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffc9000a0ce8a8
[  187.343989][ T8043] RDX: ffff888118375880 RSI: ffffffff81cd65a5 RDI: ffff888118375cc4
[  187.344527][ T8043] RBP: ffffea0004a5b1c0 R08: 0000000000000000 R09: fffffbfff1efaefa
[  187.345049][ T8043] R10: ffffffff8f7d77d7 R11: 0000000000000001 R12: 0000000000000001
[  187.345598][ T8043] R13: 0000000000000001 R14: 0000000000000101 R15: ffffea0004a5b1c0
[  187.346144][ T8043] FS:  00007f2509ad0700(0000) GS:ffff888159480000(0000) knlGS:0000000000000000
[  187.346802][ T8043] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  187.347288][ T8043] CR2: 00007fff92c60000 CR3: 0000000124c28000 CR4: 00000000003506f0
[  187.347888][ T8043] Kernel panic - not syncing: Fatal exception
[  187.348594][ T8043] Kernel Offset: disabled
[  187.348906][ T8043] Rebooting in 86400 seconds..

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

[Phillip Lougher]

On 16/12/2024 16:26, 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>

Hi Matthew,

Once the first oops is fixed, a second oops (VM_BUG_ON_FOLIO) will
occasionally occur.  The cause and oops follows:

> ---
>   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..d363fb26c2c8 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 filled = false;
>   
>   		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;

If the folio is uptodate the code jumps to skip_folio, where folio_end_read() is called

>   
> -		squashfs_fill_page(&push_folio->page, buffer, offset, avail);
> +		filled = squashfs_fill_page(push_folio, buffer, offset, avail);
>   skip_folio:
> -		folio_unlock(push_folio);
> +		folio_end_read(push_folio, filled);

This triggers the following oops

[  446.179884][ T7451] page: refcount:2 mapcount:0 mapping:ffff888000841218 ind8
[  446.180418][ T7451] memcg:ffff8880162c4000
[  446.180648][ T7451] aops:squashfs_aops ino:1f dentry name(?):".tmp_vmlinux2"
[  446.181006][ T7451] flags: 0x200000000000012d(locked|referenced|uptodate|lru)
[  446.181420][ T7451] raw: 200000000000012d ffffea0000afebc8 ffffea0000a870c8 8
[  446.181839][ T7451] raw: 0000000000000101 0000000000000000 00000002ffffffff 0
[  446.182261][ T7451] page dumped because: VM_BUG_ON_FOLIO(folio_test_uptodate)
[  446.182639][ T7451] page_owner tracks the page as allocated
[  446.182943][ T7451] page last allocated via order 0, migratetype Movable, gf6
[  446.183886][ T7451]  post_alloc_hook+0x2d0/0x350
[  446.184126][ T7451]  get_page_from_freelist+0xb39/0x22a0
[  446.184395][ T7451]  __alloc_pages_noprof+0x1bc/0x480
[  446.184651][ T7451]  __folio_alloc_noprof+0x11/0x90
[  446.184898][ T7451]  __filemap_get_folio+0x55a/0xb00
[  446.185150][ T7451]  squashfs_copy_cache+0x35d/0x540
[  446.185402][ T7451]  squashfs_read_folio+0xa13/0xc00
[  446.185695][ T7451]  filemap_read_folio+0xc0/0x2a0
[  446.185941][ T7451]  filemap_get_pages+0x155c/0x1bd0
[  446.186195][ T7451]  filemap_read+0x3b6/0xd50
[  446.186428][ T7451]  generic_file_read_iter+0x344/0x450
[  446.186690][ T7451]  __kernel_read+0x3b5/0xb10
[  446.186928][ T7451]  integrity_kernel_read+0x7f/0xb0
[  446.187328][ T7451]  ima_calc_file_hash_tfm+0x2bc/0x3d0
[  446.187716][ T7451]  ima_calc_file_hash+0x1ba/0x490
[  446.188078][ T7451]  ima_collect_measurement+0x848/0x9d0
[  446.188364][ T7451] page last free pid 7451 tgid 7451 stack trace:
[  446.188711][ T7451]  free_unref_folios+0x8ea/0x13e0
[  446.189002][ T7451]  shrink_folio_list+0xcff/0x4070
[  446.189295][ T7451]  shrink_lruvec+0xe94/0x29f0
[  446.189586][ T7451]  shrink_node+0xa74/0x2730
[  446.189823][ T7451]  do_try_to_free_pages+0x363/0x1860
[  446.190101][ T7451]  try_to_free_pages+0x2bf/0x790
[  446.190428][ T7451]  __alloc_pages_slowpath.constprop.0+0x7bc/0x2650
[  446.190810][ T7451]  __alloc_pages_noprof+0x3e6/0x480
[  446.191134][ T7451]  squashfs_bio_read+0x4af/0xf50
[  446.191408][ T7451]  squashfs_read_data+0x208/0xf10
[  446.191723][ T7451]  squashfs_readahead+0x1502/0x21e0
[  446.192034][ T7451]  read_pages+0x19f/0xdb0
[  446.192298][ T7451]  page_cache_ra_unbounded+0x3e0/0x750
[  446.192646][ T7451]  page_cache_ra_order+0x8ef/0xc30
[  446.192958][ T7451]  page_cache_async_ra+0x5cb/0x820
[  446.193269][ T7451]  filemap_get_pages+0x105b/0x1bd0
[  446.193563][ T7451] ------------[ cut here ]------------
[  446.193848][ T7451] kernel BUG at mm/filemap.c:1535!
[  446.194121][ T7451] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
[  446.194503][ T7451] CPU: 4 UID: 0 PID: 7451 Comm: cat Not tainted 6.13.0-rc20
[  446.195006][ T7451] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS4
[  446.195523][ T7451] RIP: 0010:folio_end_read+0x17b/0x1a0
[  446.195812][ T7451] Code: e8 1a 62 ca ff 48 c7 c6 a0 17 d8 8a 48 89 ef e8 2b8
[  446.196797][ T7451] RSP: 0018:ffffc900002e7710 EFLAGS: 00010293
[  446.197114][ T7451] RAX: 0000000000000000 RBX: 0000000000000001 RCX: ffffc908
[  446.197522][ T7451] RDX: ffff88801befd880 RSI: ffffffff81cd65bb RDI: ffff8884
[  446.197930][ T7451] RBP: ffffea0000a93200 R08: 0000000000000000 R09: fffffbfa
[  446.198353][ T7451] R10: ffffffff8f7d79d7 R11: 0000000000000001 R12: 00000000
[  446.198765][ T7451] R13: 0000000000000000 R14: 0000000000000001 R15: ffffea00
[  446.199174][ T7451] FS:  00007f92f7c65700(0000) GS:ffff88802da00000(0000) kn0
[  446.199633][ T7451] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  446.199975][ T7451] CR2: 00007ff2432219d8 CR3: 0000000011268000 CR4: 00000000
[  446.200385][ T7451] Call Trace:
[  446.200559][ T7451]  <TASK>
[  446.200715][ T7451]  ? die+0x31/0x80
[  446.200918][ T7451]  ? do_trap+0x232/0x430
[  446.201142][ T7451]  ? folio_end_read+0x17b/0x1a0
[  446.201401][ T7451]  ? folio_end_read+0x17b/0x1a0
[  446.201658][ T7451]  ? do_error_trap+0xf4/0x230
[  446.201905][ T7451]  ? folio_end_read+0x17b/0x1a0
[  446.202167][ T7451]  ? handle_invalid_op+0x34/0x40
[  446.202434][ T7451]  ? folio_end_read+0x17b/0x1a0
[  446.202694][ T7451]  ? exc_invalid_op+0x2d/0x40
[  446.202947][ T7451]  ? asm_exc_invalid_op+0x1a/0x20
[  446.203216][ T7451]  ? folio_end_read+0x17b/0x1a0
[  446.203475][ T7451]  ? folio_end_read+0x17b/0x1a0
[  446.203735][ T7451]  squashfs_copy_cache+0x1d4/0x540
[  446.204005][ T7451]  squashfs_read_folio+0xa13/0xc00
[  446.204274][ T7451]  ? __pfx_squashfs_read_folio+0x10/0x10
[  446.204571][ T7451]  ? __pfx_squashfs_read_folio+0x10/0x10
[  446.204867][ T7451]  filemap_read_folio+0xc0/0x2a0
[  446.205128][ T7451]  ? __pfx_filemap_read_folio+0x10/0x10
[  446.205419][ T7451]  ? page_cache_async_ra+0x5cb/0x820
[  446.205703][ T7451]  filemap_get_pages+0x155c/0x1bd0
[  446.205972][ T7451]  ? current_time+0x79/0x380
[  446.206219][ T7451]  ? __pfx_filemap_get_pages+0x10/0x10
[  446.206520][ T7451]  filemap_read+0x3b6/0xd50
[  446.206771][ T7451]  ? find_held_lock+0x2d/0x110
[  446.207032][ T7451]  ? vfs_write+0x57e/0x1190
[  446.207279][ T7451]  ? __pfx_filemap_read+0x10/0x10
[  446.207554][ T7451]  ? pipe_write+0xf9f/0x1ae0
[  446.207828][ T7451]  ? __pfx_pipe_write+0x10/0x10
[  446.208135][ T7451]  ? lock_acquire+0x1b1/0x550
[  446.208435][ T7451]  ? __pfx_autoremove_wake_function+0x10/0x10
[  446.208810][ T7451]  generic_file_read_iter+0x344/0x450
[  446.209151][ T7451]  ? rw_verify_area+0xd0/0x700
[  446.209440][ T7451]  vfs_read+0x83e/0xba0
[  446.209672][ T7451]  ? __pfx_vfs_read+0x10/0x10
[  446.209920][ T7451]  ? __pfx_generic_fadvise+0x10/0x10
[  446.210203][ T7451]  ksys_read+0x122/0x240
[  446.210435][ T7451]  ? __pfx_ksys_read+0x10/0x10
[  446.210691][ T7451]  do_syscall_64+0x74/0x1c0
[  446.210932][ T7451]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  446.211243][ T7451] RIP: 0033:0x7f92f76dbba0
[  446.211480][ T7451] Code: 0b 31 c0 48 83 c4 08 e9 be fe ff ff 48 8d 3d 3f f04
[  446.212458][ T7451] RSP: 002b:00007fff1ae88768 EFLAGS: 00000246 ORIG_RAX: 000
[  446.212887][ T7451] RAX: ffffffffffffffda RBX: 0000000000008000 RCX: 00007f90
[  446.213292][ T7451] RDX: 0000000000008000 RSI: 000000003e0a0000 RDI: 00000003
[  446.213700][ T7451] RBP: 0000000000008000 R08: 0000000000000003 R09: 00007f90
[  446.214107][ T7451] R10: 0000000000000002 R11: 0000000000000246 R12: 00000000
[  446.214518][ T7451] R13: 0000000000000003 R14: 0000000000000000 R15: 00000000
[  446.214925][ T7451]  </TASK>
[  446.215086][ T7451] Modules linked in:
[  446.215317][ T7451] ---[ end trace 0000000000000000 ]---
[  446.215609][ T7451] RIP: 0010:folio_end_read+0x17b/0x1a0
[  446.215906][ T7451] Code: e8 1a 62 ca ff 48 c7 c6 a0 17 d8 8a 48 89 ef e8 2b8
[  446.216889][ T7451] RSP: 0018:ffffc900002e7710 EFLAGS: 00010293
[  446.217204][ T7451] RAX: 0000000000000000 RBX: 0000000000000001 RCX: ffffc908
[  446.217609][ T7451] RDX: ffff88801befd880 RSI: ffffffff81cd65bb RDI: ffff8884
[  446.218015][ T7451] RBP: ffffea0000a93200 R08: 0000000000000000 R09: fffffbfa
[  446.218459][ T7451] R10: ffffffff8f7d79d7 R11: 0000000000000001 R12: 00000000
[  446.218878][ T7451] R13: 0000000000000000 R14: 0000000000000001 R15: ffffea00
[  446.219287][ T7451] FS:  00007f92f7c65700(0000) GS:ffff88802da00000(0000) kn0
[  446.219746][ T7451] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  446.220089][ T7451] CR2: 00007ff2432219d8 CR3: 0000000011268000 CR4: 00000000
[  446.220498][ T7451] Kernel panic - not syncing: Fatal exception
[  446.221221][ T7451] Kernel Offset: disabled
[  446.221468][ T7451] Rebooting in 86400 seconds..

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

[Matthew Wilcox]

On Fri, Dec 20, 2024 at 06:19:35PM +0000, Phillip Lougher wrote:
> > @@ -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 filled = false;

ahh, this should have been filled = true (if the folio is already
uptodate, then it has been filled).  Or maybe it'd be less confusing if
we named the bool 'uptodate'.

Would you like me to submit a fresh set of patches, or will you fix
these two bugs up?

Thanks for testing!

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

[Phillip Lougher]

On 20/12/2024 18:24, Matthew Wilcox wrote:
> On Fri, Dec 20, 2024 at 06:19:35PM +0000, Phillip Lougher wrote:
>>> @@ -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 filled = false;
> 
> ahh, this should have been filled = true (if the folio is already
> uptodate, then it has been filled).  Or maybe it'd be less confusing if
> we named the bool 'uptodate'.
> 
> Would you like me to submit a fresh set of patches, or will you fix
> these two bugs up?
> 
> Thanks for testing!

Np.

Andrew Morton is kindly handling Squashfs patch submission to Linus for me,
and so I can't easily fix them up.

Andrew what you like done here please?

Thanks

Phillip

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

[Andrew Morton]

On Fri, 20 Dec 2024 18:42:57 +0000 Phillip Lougher <phillip@squashfs.org.uk> wrote:

> 
> 
> On 20/12/2024 18:24, Matthew Wilcox wrote:
> > On Fri, Dec 20, 2024 at 06:19:35PM +0000, Phillip Lougher wrote:
> >>> @@ -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 filled = false;
> > 
> > ahh, this should have been filled = true (if the folio is already
> > uptodate, then it has been filled).  Or maybe it'd be less confusing if
> > we named the bool 'uptodate'.
> > 
> > Would you like me to submit a fresh set of patches, or will you fix
> > these two bugs up?
> > 
> > Thanks for testing!
> 
> Np.
> 
> Andrew Morton is kindly handling Squashfs patch submission to Linus for me,
> and so I can't easily fix them up.
> 
> Andrew what you like done here please?

I think a v2 series would be best, please.

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

[Phillip Lougher]

On 12/16/24 16:26, 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 2/5] squashfs: Pass a folio to squashfs_readpage_fragment()

[Phillip Lougher]

On 12/16/24 16:26, 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 3/5] squashfs: Convert squashfs_readpage_block() to take a folio

[Phillip Lougher]

On 12/16/24 16:26, 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 4/5] squashfs; Convert squashfs_copy_cache() to take a folio

[Phillip Lougher]

On 12/16/24 16:26, 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 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Phillip Lougher]

On 12/16/24 16:26, 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..d363fb26c2c8 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 filled = false;
>   
>   		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);
> +		filled = squashfs_fill_page(push_folio, buffer, offset, avail);
>   skip_folio:
> -		folio_unlock(push_folio);
> +		folio_end_read(folio, filled);
>   		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);
>   

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

[Phillip Lougher]

On 1/14/25 21:07, Phillip Lougher wrote:
> 
> 
> On 12/16/24 16:26, 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>

Oops, wrong patch-series.

> 
>> ---
>>   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;
>>   }