[PATCH 0/2] Finish converting jffs2 to folios

[PATCH 0/2] Finish converting jffs2 to folios

[Matthew Wilcox (Oracle)]

This patch series applies on top of fs-next.  I suggest it goes through
Christian's tree.  After applying these two patches, there are no more
references to 'struct page' in jffs2.  I obviously haven't tested it at
all beyond compilation.

Matthew Wilcox (Oracle) (2):
  jffs2: Convert jffs2_do_readpage_nolock to take a folio
  jffs2: Use a folio in jffs2_garbage_collect_dnode()

 fs/jffs2/file.c | 24 +++++++++++-------------
 fs/jffs2/gc.c   | 25 ++++++++++++-------------
 2 files changed, 23 insertions(+), 26 deletions(-)

-- 
2.43.0

[PATCH 1/2] jffs2: Convert jffs2_do_readpage_nolock to take a folio

[Matthew Wilcox (Oracle)]

Both callers now have a folio, so pass it in.  No effort is made
here to support large folios.  Removes several hidden calls to
compound_head(), two references to page->index and a use of kmap.

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

diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index ada572c466f8..13c18ccc13b0 100644
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -77,29 +77,27 @@ const struct address_space_operations jffs2_file_address_operations =
 	.write_end =	jffs2_write_end,
 };
 
-static int jffs2_do_readpage_nolock (struct inode *inode, struct page *pg)
+static int jffs2_do_readpage_nolock(struct inode *inode, struct folio *folio)
 {
 	struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
 	struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
-	unsigned char *pg_buf;
+	unsigned char *kaddr;
 	int ret;
 
 	jffs2_dbg(2, "%s(): ino #%lu, page at offset 0x%lx\n",
-		  __func__, inode->i_ino, pg->index << PAGE_SHIFT);
+		  __func__, inode->i_ino, folio->index << PAGE_SHIFT);
 
-	BUG_ON(!PageLocked(pg));
+	BUG_ON(!folio_test_locked(folio));
 
-	pg_buf = kmap(pg);
-	/* FIXME: Can kmap fail? */
-
-	ret = jffs2_read_inode_range(c, f, pg_buf, pg->index << PAGE_SHIFT,
+	kaddr = kmap_local_folio(folio, 0);
+	ret = jffs2_read_inode_range(c, f, kaddr, folio->index << PAGE_SHIFT,
 				     PAGE_SIZE);
+	kunmap_local(kaddr);
 
 	if (!ret)
-		SetPageUptodate(pg);
+		folio_mark_uptodate(folio);
 
-	flush_dcache_page(pg);
-	kunmap(pg);
+	flush_dcache_folio(folio);
 
 	jffs2_dbg(2, "readpage finished\n");
 	return ret;
@@ -107,7 +105,7 @@ static int jffs2_do_readpage_nolock (struct inode *inode, struct page *pg)
 
 int __jffs2_read_folio(struct file *file, struct folio *folio)
 {
-	int ret = jffs2_do_readpage_nolock(folio->mapping->host, &folio->page);
+	int ret = jffs2_do_readpage_nolock(folio->mapping->host, folio);
 	folio_unlock(folio);
 	return ret;
 }
@@ -221,7 +219,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
 	 */
 	if (!folio_test_uptodate(folio)) {
 		mutex_lock(&f->sem);
-		ret = jffs2_do_readpage_nolock(inode, &folio->page);
+		ret = jffs2_do_readpage_nolock(inode, folio);
 		mutex_unlock(&f->sem);
 		if (ret) {
 			folio_unlock(folio);
-- 
2.43.0

Re: [PATCH 1/2] jffs2: Convert jffs2_do_readpage_nolock to take a folio

[Zhihao Cheng]

在 2024/8/15 3:59, Matthew Wilcox (Oracle) 写道:
> Both callers now have a folio, so pass it in.  No effort is made
> here to support large folios.  Removes several hidden calls to
> compound_head(), two references to page->index and a use of kmap.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/jffs2/file.c | 24 +++++++++++-------------
>   1 file changed, 11 insertions(+), 13 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
> index ada572c466f8..13c18ccc13b0 100644
> --- a/fs/jffs2/file.c
> +++ b/fs/jffs2/file.c
> @@ -77,29 +77,27 @@ const struct address_space_operations jffs2_file_address_operations =
>   	.write_end =	jffs2_write_end,
>   };
>   
> -static int jffs2_do_readpage_nolock (struct inode *inode, struct page *pg)
> +static int jffs2_do_readpage_nolock(struct inode *inode, struct folio *folio)
>   {
>   	struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
>   	struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
> -	unsigned char *pg_buf;
> +	unsigned char *kaddr;
>   	int ret;
>   
>   	jffs2_dbg(2, "%s(): ino #%lu, page at offset 0x%lx\n",
> -		  __func__, inode->i_ino, pg->index << PAGE_SHIFT);
> +		  __func__, inode->i_ino, folio->index << PAGE_SHIFT);
>   
> -	BUG_ON(!PageLocked(pg));
> +	BUG_ON(!folio_test_locked(folio));
>   
> -	pg_buf = kmap(pg);
> -	/* FIXME: Can kmap fail? */
> -
> -	ret = jffs2_read_inode_range(c, f, pg_buf, pg->index << PAGE_SHIFT,
> +	kaddr = kmap_local_folio(folio, 0);
> +	ret = jffs2_read_inode_range(c, f, kaddr, folio->index << PAGE_SHIFT,
>   				     PAGE_SIZE);
> +	kunmap_local(kaddr);
>   
>   	if (!ret)
> -		SetPageUptodate(pg);
> +		folio_mark_uptodate(folio);
>   
> -	flush_dcache_page(pg);
> -	kunmap(pg);
> +	flush_dcache_folio(folio);
>   
>   	jffs2_dbg(2, "readpage finished\n");
>   	return ret;
> @@ -107,7 +105,7 @@ static int jffs2_do_readpage_nolock (struct inode *inode, struct page *pg)
>   
>   int __jffs2_read_folio(struct file *file, struct folio *folio)
>   {
> -	int ret = jffs2_do_readpage_nolock(folio->mapping->host, &folio->page);
> +	int ret = jffs2_do_readpage_nolock(folio->mapping->host, folio);
>   	folio_unlock(folio);
>   	return ret;
>   }
> @@ -221,7 +219,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
>   	 */
>   	if (!folio_test_uptodate(folio)) {
>   		mutex_lock(&f->sem);
> -		ret = jffs2_do_readpage_nolock(inode, &folio->page);
> +		ret = jffs2_do_readpage_nolock(inode, folio);
>   		mutex_unlock(&f->sem);
>   		if (ret) {
>   			folio_unlock(folio);
> 

[PATCH 2/2] jffs2: Use a folio in jffs2_garbage_collect_dnode()

[Matthew Wilcox (Oracle)]

Call read_cache_folio() instead of read_cache_page() to get the folio
containing the page.  No attempt is made here to support large folios
as I assume that will never be interesting for jffs2.  Includes a switch
from kmap to kmap_local which looks safe.

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

diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
index 5c6602f3c189..822949d0eb00 100644
--- a/fs/jffs2/gc.c
+++ b/fs/jffs2/gc.c
@@ -1171,7 +1171,7 @@ static int jffs2_garbage_collect_dnode(struct jffs2_sb_info *c, struct jffs2_era
 	uint32_t alloclen, offset, orig_end, orig_start;
 	int ret = 0;
 	unsigned char *comprbuf = NULL, *writebuf;
-	struct page *page;
+	struct folio *folio;
 	unsigned char *pg_ptr;
 
 	memset(&ri, 0, sizeof(ri));
@@ -1317,25 +1317,25 @@ static int jffs2_garbage_collect_dnode(struct jffs2_sb_info *c, struct jffs2_era
 		BUG_ON(start > orig_start);
 	}
 
-	/* The rules state that we must obtain the page lock *before* f->sem, so
+	/* The rules state that we must obtain the folio lock *before* f->sem, so
 	 * drop f->sem temporarily. Since we also hold c->alloc_sem, nothing's
 	 * actually going to *change* so we're safe; we only allow reading.
 	 *
 	 * It is important to note that jffs2_write_begin() will ensure that its
-	 * page is marked Uptodate before allocating space. That means that if we
-	 * end up here trying to GC the *same* page that jffs2_write_begin() is
-	 * trying to write out, read_cache_page() will not deadlock. */
+	 * folio is marked uptodate before allocating space. That means that if we
+	 * end up here trying to GC the *same* folio that jffs2_write_begin() is
+	 * trying to write out, read_cache_folio() will not deadlock. */
 	mutex_unlock(&f->sem);
-	page = read_cache_page(inode->i_mapping, start >> PAGE_SHIFT,
+	folio = read_cache_folio(inode->i_mapping, start >> PAGE_SHIFT,
 			       __jffs2_read_folio, NULL);
-	if (IS_ERR(page)) {
-		pr_warn("read_cache_page() returned error: %ld\n",
-			PTR_ERR(page));
+	if (IS_ERR(folio)) {
+		pr_warn("read_cache_folio() returned error: %ld\n",
+			PTR_ERR(folio));
 		mutex_lock(&f->sem);
-		return PTR_ERR(page);
+		return PTR_ERR(folio);
 	}
 
-	pg_ptr = kmap(page);
+	pg_ptr = kmap_local_folio(folio, 0);
 	mutex_lock(&f->sem);
 
 	offset = start;
@@ -1400,7 +1400,6 @@ static int jffs2_garbage_collect_dnode(struct jffs2_sb_info *c, struct jffs2_era
 		}
 	}
 
-	kunmap(page);
-	put_page(page);
+	folio_release_kmap(folio, pg_ptr);
 	return ret;
 }
-- 
2.43.0

Re: [PATCH 2/2] jffs2: Use a folio in jffs2_garbage_collect_dnode()

[Zhihao Cheng]

在 2024/8/15 3:59, Matthew Wilcox (Oracle) 写道:
> Call read_cache_folio() instead of read_cache_page() to get the folio
> containing the page.  No attempt is made here to support large folios
> as I assume that will never be interesting for jffs2.  Includes a switch
> from kmap to kmap_local which looks safe.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/jffs2/gc.c | 25 ++++++++++++-------------
>   1 file changed, 12 insertions(+), 13 deletions(-)

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> 
> diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
> index 5c6602f3c189..822949d0eb00 100644
> --- a/fs/jffs2/gc.c
> +++ b/fs/jffs2/gc.c
> @@ -1171,7 +1171,7 @@ static int jffs2_garbage_collect_dnode(struct jffs2_sb_info *c, struct jffs2_era
>   	uint32_t alloclen, offset, orig_end, orig_start;
>   	int ret = 0;
>   	unsigned char *comprbuf = NULL, *writebuf;
> -	struct page *page;
> +	struct folio *folio;
>   	unsigned char *pg_ptr;
>   
>   	memset(&ri, 0, sizeof(ri));
> @@ -1317,25 +1317,25 @@ static int jffs2_garbage_collect_dnode(struct jffs2_sb_info *c, struct jffs2_era
>   		BUG_ON(start > orig_start);
>   	}
>   
> -	/* The rules state that we must obtain the page lock *before* f->sem, so
> +	/* The rules state that we must obtain the folio lock *before* f->sem, so
>   	 * drop f->sem temporarily. Since we also hold c->alloc_sem, nothing's
>   	 * actually going to *change* so we're safe; we only allow reading.
>   	 *
>   	 * It is important to note that jffs2_write_begin() will ensure that its
> -	 * page is marked Uptodate before allocating space. That means that if we
> -	 * end up here trying to GC the *same* page that jffs2_write_begin() is
> -	 * trying to write out, read_cache_page() will not deadlock. */
> +	 * folio is marked uptodate before allocating space. That means that if we
> +	 * end up here trying to GC the *same* folio that jffs2_write_begin() is
> +	 * trying to write out, read_cache_folio() will not deadlock. */
>   	mutex_unlock(&f->sem);
> -	page = read_cache_page(inode->i_mapping, start >> PAGE_SHIFT,
> +	folio = read_cache_folio(inode->i_mapping, start >> PAGE_SHIFT,
>   			       __jffs2_read_folio, NULL);
> -	if (IS_ERR(page)) {
> -		pr_warn("read_cache_page() returned error: %ld\n",
> -			PTR_ERR(page));
> +	if (IS_ERR(folio)) {
> +		pr_warn("read_cache_folio() returned error: %ld\n",
> +			PTR_ERR(folio));
>   		mutex_lock(&f->sem);
> -		return PTR_ERR(page);
> +		return PTR_ERR(folio);
>   	}
>   
> -	pg_ptr = kmap(page);
> +	pg_ptr = kmap_local_folio(folio, 0);
>   	mutex_lock(&f->sem);
>   
>   	offset = start;
> @@ -1400,7 +1400,6 @@ static int jffs2_garbage_collect_dnode(struct jffs2_sb_info *c, struct jffs2_era
>   		}
>   	}
>   
> -	kunmap(page);
> -	put_page(page);
> +	folio_release_kmap(folio, pg_ptr);
>   	return ret;
>   }
> 

Re: [PATCH 0/2] Finish converting jffs2 to folios

[Christian Brauner]

On Wed, 14 Aug 2024 20:59:11 +0100, Matthew Wilcox (Oracle) wrote:
> This patch series applies on top of fs-next.  I suggest it goes through
> Christian's tree.  After applying these two patches, there are no more
> references to 'struct page' in jffs2.  I obviously haven't tested it at
> all beyond compilation.
> 
> Matthew Wilcox (Oracle) (2):
>   jffs2: Convert jffs2_do_readpage_nolock to take a folio
>   jffs2: Use a folio in jffs2_garbage_collect_dnode()
> 
> [...]

Applied to the vfs.folio branch of the vfs/vfs.git tree.
Patches in the vfs.folio branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.folio

[1/2] jffs2: Convert jffs2_do_readpage_nolock to take a folio
      https://git.kernel.org/vfs/vfs/c/bcc7d11e6c09
[2/2] jffs2: Use a folio in jffs2_garbage_collect_dnode()
      https://git.kernel.org/vfs/vfs/c/2da4c51a66cd