diff mbox series

[v3,5/9] fuse: move initialization of fuse_file to fuse_writepages() instead of in callback

Message ID 20240823162730.521499-6-joannelkoong@gmail.com (mailing list archive)
State New
Headers show
Series fuse: writeback clean up / refactoring | expand

Commit Message

Joanne Koong Aug. 23, 2024, 4:27 p.m. UTC
Prior to this change, data->ff is checked and if not initialized then
initialized in the fuse_writepages_fill() callback, which gets called
for every dirty page in the address space mapping.

This logic is better placed in the main fuse_writepages() caller where
data.ff is initialized before walking the dirty pages.

No functional changes added.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 fs/fuse/file.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

Comments

Josef Bacik Aug. 23, 2024, 6:59 p.m. UTC | #1
On Fri, Aug 23, 2024 at 09:27:26AM -0700, Joanne Koong wrote:
> Prior to this change, data->ff is checked and if not initialized then
> initialized in the fuse_writepages_fill() callback, which gets called
> for every dirty page in the address space mapping.
> 
> This logic is better placed in the main fuse_writepages() caller where
> data.ff is initialized before walking the dirty pages.
> 
> No functional changes added.
> 
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>

You remove the out label in the previous patch, and then add it back here, you
can probably merge the previous patch and this patch into one patch and it would
look fine, and reduce the churn a bit.  Thanks,

Josef
Joanne Koong Aug. 23, 2024, 9:21 p.m. UTC | #2
On Fri, Aug 23, 2024 at 11:59 AM Josef Bacik <josef@toxicpanda.com> wrote:
>
> On Fri, Aug 23, 2024 at 09:27:26AM -0700, Joanne Koong wrote:
> > Prior to this change, data->ff is checked and if not initialized then
> > initialized in the fuse_writepages_fill() callback, which gets called
> > for every dirty page in the address space mapping.
> >
> > This logic is better placed in the main fuse_writepages() caller where
> > data.ff is initialized before walking the dirty pages.
> >
> > No functional changes added.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
>
> You remove the out label in the previous patch, and then add it back here, you
> can probably merge the previous patch and this patch into one patch and it would
> look fine, and reduce the churn a bit.  Thanks,

Gotcha. I'll merge the previous patch (4/9) with this one together in
the next version.

Thanks,
Joanne

>
> Josef
diff mbox series

Patch

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 8a9b6e8dbd1b..a51b0b085616 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2263,13 +2263,6 @@  static int fuse_writepages_fill(struct folio *folio,
 	struct page *tmp_page;
 	int err;
 
-	if (!data->ff) {
-		err = -EIO;
-		data->ff = fuse_write_file_get(fi);
-		if (!data->ff)
-			goto out_unlock;
-	}
-
 	if (wpa && fuse_writepage_need_send(fc, &folio->page, ap, data)) {
 		fuse_writepages_send(data);
 		data->wpa = NULL;
@@ -2348,6 +2341,7 @@  static int fuse_writepages(struct address_space *mapping,
 			   struct writeback_control *wbc)
 {
 	struct inode *inode = mapping->host;
+	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_fill_wb_data data;
 	int err;
@@ -2361,23 +2355,26 @@  static int fuse_writepages(struct address_space *mapping,
 
 	data.inode = inode;
 	data.wpa = NULL;
-	data.ff = NULL;
+	data.ff = fuse_write_file_get(fi);
+	if (!data.ff)
+		return -EIO;
 
 	data.orig_pages = kcalloc(fc->max_pages,
 				  sizeof(struct page *),
 				  GFP_NOFS);
+	err = -ENOMEM;
 	if (!data.orig_pages)
-		return -ENOMEM;
+		goto out;
 
 	err = write_cache_pages(mapping, wbc, fuse_writepages_fill, &data);
 	if (data.wpa) {
 		WARN_ON(!data.wpa->ia.ap.num_pages);
 		fuse_writepages_send(&data);
 	}
-	if (data.ff)
-		fuse_file_put(data.ff, false);
 
 	kfree(data.orig_pages);
+out:
+	fuse_file_put(data.ff, false);
 	return err;
 }