[PATCH 1/2] fuse: Remove fuse_writepage

[PATCH 1/2] fuse: Remove fuse_writepage

[Matthew Wilcox (Oracle)]

The writepage operation is deprecated as it leads to worse performance
under high memory pressure due to folios being written out in LRU order
rather than sequentially within a file.  Use filemap_migrate_folio() to
support dirty folio migration instead of writepage.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/fuse/file.c | 30 +-----------------------------
 1 file changed, 1 insertion(+), 29 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 7f5d857c5692..340ccaafb3f7 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2104,34 +2104,6 @@ static int fuse_writepage_locked(struct page *page)
 	return error;
 }
 
-static int fuse_writepage(struct page *page, struct writeback_control *wbc)
-{
-	struct fuse_conn *fc = get_fuse_conn(page->mapping->host);
-	int err;
-
-	if (fuse_page_is_writeback(page->mapping->host, page->index)) {
-		/*
-		 * ->writepages() should be called for sync() and friends.  We
-		 * should only get here on direct reclaim and then we are
-		 * allowed to skip a page which is already in flight
-		 */
-		WARN_ON(wbc->sync_mode == WB_SYNC_ALL);
-
-		redirty_page_for_writepage(wbc, page);
-		unlock_page(page);
-		return 0;
-	}
-
-	if (wbc->sync_mode == WB_SYNC_NONE &&
-	    fc->num_background >= fc->congestion_threshold)
-		return AOP_WRITEPAGE_ACTIVATE;
-
-	err = fuse_writepage_locked(page);
-	unlock_page(page);
-
-	return err;
-}
-
 struct fuse_fill_wb_data {
 	struct fuse_writepage_args *wpa;
 	struct fuse_file *ff;
@@ -3347,10 +3319,10 @@ static const struct file_operations fuse_file_operations = {
 static const struct address_space_operations fuse_file_aops  = {
 	.read_folio	= fuse_read_folio,
 	.readahead	= fuse_readahead,
-	.writepage	= fuse_writepage,
 	.writepages	= fuse_writepages,
 	.launder_folio	= fuse_launder_folio,
 	.dirty_folio	= filemap_dirty_folio,
+	.migrate_folio	= filemap_migrate_folio,
 	.bmap		= fuse_bmap,
 	.direct_IO	= fuse_direct_IO,
 	.write_begin	= fuse_write_begin,
-- 
2.43.0

[PATCH 2/2] fuse: Convert fuse_writepage_locked to take a folio

[Matthew Wilcox (Oracle)]

The one remaining caller of fuse_writepage_locked() already has a folio,
so convert this function entirely.  Saves a few calls to compound_head()
but no attempt is made to support large folios in this patch.

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

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 340ccaafb3f7..f173cbce1d31 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2040,26 +2040,26 @@ static void fuse_writepage_add_to_bucket(struct fuse_conn *fc,
 	rcu_read_unlock();
 }
 
-static int fuse_writepage_locked(struct page *page)
+static int fuse_writepage_locked(struct folio *folio)
 {
-	struct address_space *mapping = page->mapping;
+	struct address_space *mapping = folio->mapping;
 	struct inode *inode = mapping->host;
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_writepage_args *wpa;
 	struct fuse_args_pages *ap;
-	struct page *tmp_page;
+	struct folio *tmp_folio;
 	int error = -ENOMEM;
 
-	set_page_writeback(page);
+	folio_start_writeback(folio);
 
 	wpa = fuse_writepage_args_alloc();
 	if (!wpa)
 		goto err;
 	ap = &wpa->ia.ap;
 
-	tmp_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
-	if (!tmp_page)
+	tmp_folio = folio_alloc(GFP_NOFS | __GFP_HIGHMEM, 0);
+	if (!tmp_folio)
 		goto err_free;
 
 	error = -EIO;
@@ -2068,21 +2068,21 @@ static int fuse_writepage_locked(struct page *page)
 		goto err_nofile;
 
 	fuse_writepage_add_to_bucket(fc, wpa);
-	fuse_write_args_fill(&wpa->ia, wpa->ia.ff, page_offset(page), 0);
+	fuse_write_args_fill(&wpa->ia, wpa->ia.ff, folio_pos(folio), 0);
 
-	copy_highpage(tmp_page, page);
+	folio_copy(tmp_folio, folio);
 	wpa->ia.write.in.write_flags |= FUSE_WRITE_CACHE;
 	wpa->next = NULL;
 	ap->args.in_pages = true;
 	ap->num_pages = 1;
-	ap->pages[0] = tmp_page;
+	ap->pages[0] = &tmp_folio->page;
 	ap->descs[0].offset = 0;
 	ap->descs[0].length = PAGE_SIZE;
 	ap->args.end = fuse_writepage_end;
 	wpa->inode = inode;
 
 	inc_wb_stat(&inode_to_bdi(inode)->wb, WB_WRITEBACK);
-	inc_node_page_state(tmp_page, NR_WRITEBACK_TEMP);
+	node_stat_add_folio(tmp_folio, NR_WRITEBACK_TEMP);
 
 	spin_lock(&fi->lock);
 	tree_insert(&fi->writepages, wpa);
@@ -2090,17 +2090,17 @@ static int fuse_writepage_locked(struct page *page)
 	fuse_flush_writepages(inode);
 	spin_unlock(&fi->lock);
 
-	end_page_writeback(page);
+	folio_end_writeback(folio);
 
 	return 0;
 
 err_nofile:
-	__free_page(tmp_page);
+	folio_put(tmp_folio);
 err_free:
 	kfree(wpa);
 err:
-	mapping_set_error(page->mapping, error);
-	end_page_writeback(page);
+	mapping_set_error(folio->mapping, error);
+	folio_end_writeback(folio);
 	return error;
 }
 
@@ -2466,7 +2466,7 @@ static int fuse_launder_folio(struct folio *folio)
 
 		/* Serialize with pending writeback for the same page */
 		fuse_wait_on_page_writeback(inode, folio->index);
-		err = fuse_writepage_locked(&folio->page);
+		err = fuse_writepage_locked(folio);
 		if (!err)
 			fuse_wait_on_page_writeback(inode, folio->index);
 	}
-- 
2.43.0

Re: [PATCH 2/2] fuse: Convert fuse_writepage_locked to take a folio

[Bernd Schubert]

On 2/28/24 19:29, Matthew Wilcox (Oracle) wrote:
> The one remaining caller of fuse_writepage_locked() already has a folio,
> so convert this function entirely.  Saves a few calls to compound_head()
> but no attempt is made to support large folios in this patch.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/fuse/file.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 340ccaafb3f7..f173cbce1d31 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -2040,26 +2040,26 @@ static void fuse_writepage_add_to_bucket(struct fuse_conn *fc,
>  	rcu_read_unlock();
>  }
>  
> -static int fuse_writepage_locked(struct page *page)
> +static int fuse_writepage_locked(struct folio *folio)
>  {
> -	struct address_space *mapping = page->mapping;
> +	struct address_space *mapping = folio->mapping;
>  	struct inode *inode = mapping->host;
>  	struct fuse_conn *fc = get_fuse_conn(inode);
>  	struct fuse_inode *fi = get_fuse_inode(inode);
>  	struct fuse_writepage_args *wpa;
>  	struct fuse_args_pages *ap;
> -	struct page *tmp_page;
> +	struct folio *tmp_folio;
>  	int error = -ENOMEM;
>  
> -	set_page_writeback(page);
> +	folio_start_writeback(folio);
>  
>  	wpa = fuse_writepage_args_alloc();
>  	if (!wpa)
>  		goto err;
>  	ap = &wpa->ia.ap;
>  
> -	tmp_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
> -	if (!tmp_page)
> +	tmp_folio = folio_alloc(GFP_NOFS | __GFP_HIGHMEM, 0);
> +	if (!tmp_folio)
>  		goto err_free;
>  
>  	error = -EIO;
> @@ -2068,21 +2068,21 @@ static int fuse_writepage_locked(struct page *page)
>  		goto err_nofile;
>  
>  	fuse_writepage_add_to_bucket(fc, wpa);
> -	fuse_write_args_fill(&wpa->ia, wpa->ia.ff, page_offset(page), 0);
> +	fuse_write_args_fill(&wpa->ia, wpa->ia.ff, folio_pos(folio), 0);
>  
> -	copy_highpage(tmp_page, page);
> +	folio_copy(tmp_folio, folio);
>  	wpa->ia.write.in.write_flags |= FUSE_WRITE_CACHE;
>  	wpa->next = NULL;
>  	ap->args.in_pages = true;
>  	ap->num_pages = 1;
> -	ap->pages[0] = tmp_page;
> +	ap->pages[0] = &tmp_folio->page;

Hi Matthew,

sorry for late review. The part I'm totally confused with (already
without this patch), why is this handling a single page only and not the
entire folio? Is it guaranteed that the folio has a single page only?



Thanks,
Bernd

Re: [PATCH 2/2] fuse: Convert fuse_writepage_locked to take a folio

[Matthew Wilcox]

On Sat, Apr 13, 2024 at 01:28:31PM +0200, Bernd Schubert wrote:
> On 2/28/24 19:29, Matthew Wilcox (Oracle) wrote:
> > The one remaining caller of fuse_writepage_locked() already has a folio,
> > so convert this function entirely.  Saves a few calls to compound_head()
> > but no attempt is made to support large folios in this patch.
>
> sorry for late review. The part I'm totally confused with (already
> without this patch), why is this handling a single page only and not the
> entire folio? Is it guaranteed that the folio has a single page only?

Hi Bernd,

, filesystems have to tell the VFS that they support large folios before
they'll see a large folio.  That's a call to mapping_set_large_folios()
today, although there's proposals to make that more granular.

If there's interest in supporting large folios in FUSE, I'm happy to
help, but my primary motivation is sorting out struct page, not fixing
individual filesystems.

Re: [PATCH 2/2] fuse: Convert fuse_writepage_locked to take a folio

[Bernd Schubert]

On 4/13/24 17:59, Matthew Wilcox wrote:
> On Sat, Apr 13, 2024 at 01:28:31PM +0200, Bernd Schubert wrote:
>> On 2/28/24 19:29, Matthew Wilcox (Oracle) wrote:
>>> The one remaining caller of fuse_writepage_locked() already has a folio,
>>> so convert this function entirely.  Saves a few calls to compound_head()
>>> but no attempt is made to support large folios in this patch.
>>
>> sorry for late review. The part I'm totally confused with (already
>> without this patch), why is this handling a single page only and not the
>> entire folio? Is it guaranteed that the folio has a single page only?
> 
> Hi Bernd,
> 
> , filesystems have to tell the VFS that they support large folios before
> they'll see a large folio.  That's a call to mapping_set_large_folios()
> today, although there's proposals to make that more granular.
> 
> If there's interest in supporting large folios in FUSE, I'm happy to
> help, but my primary motivation is sorting out struct page, not fixing
> individual filesystems.

Thank you Matthew! Especially for your quick reply on a Saturday!

I had totally missed mapping_set_large_folios. I will look into
converting fuse to large folios once I get to performance optimizations
for our file system. For now I was just going through all recent commits
and had already wondered about the single page folio before.


Thanks again,
Bernd

Re: [PATCH 1/2] fuse: Remove fuse_writepage

[Miklos Szeredi]

On Wed, 28 Feb 2024 at 19:29, Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> The writepage operation is deprecated as it leads to worse performance
> under high memory pressure due to folios being written out in LRU order
> rather than sequentially within a file.  Use filemap_migrate_folio() to
> support dirty folio migration instead of writepage.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Applied series, thanks.

Miklos