On Thu, 22 Aug 2024 at 01:25, Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
>
> This change refactors the shared logic in fuse_writepages_fill() and
> fuse_writepages_locked() into two separate helper functions,
> fuse_writepage_args_page_fill() and fuse_writepage_args_setup().
>
> No functional changes added.
>
> Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> ---
> fs/fuse/file.c | 102 +++++++++++++++++++++++++++----------------------
> 1 file changed, 57 insertions(+), 45 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 812b3d043b26..fe8ae19587fb 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1936,7 +1936,6 @@ static void fuse_writepage_end(struct fuse_mount *fm, struct fuse_args *args,
>
> wpa->next = next->next;
> next->next = NULL;
> - next->ia.ff = fuse_file_get(wpa->ia.ff);
> tree_insert(&fi->writepages, next);
>
> /*
> @@ -2049,50 +2048,78 @@ static void fuse_writepage_add_to_bucket(struct fuse_conn *fc,
> rcu_read_unlock();
> }
>
> +static void fuse_writepage_args_page_fill(struct fuse_writepage_args *wpa, struct folio *folio,
> + struct folio *tmp_folio, uint32_t page_index)
> +{
> + struct inode *inode = folio->mapping->host;
> + struct fuse_args_pages *ap = &wpa->ia.ap;
> +
> + folio_copy(tmp_folio, folio);
> +
> + ap->pages[page_index] = &tmp_folio->page;
> + ap->descs[page_index].offset = 0;
> + ap->descs[page_index].length = PAGE_SIZE;
> +
> + inc_wb_stat(&inode_to_bdi(inode)->wb, WB_WRITEBACK);
> + inc_node_page_state(&tmp_folio->page, NR_WRITEBACK_TEMP);
> +}
> +
> +static struct fuse_writepage_args *fuse_writepage_args_setup(struct folio *folio,
> + struct fuse_file *ff)
> +{
> + struct inode *inode = folio->mapping->host;
> + struct fuse_conn *fc = get_fuse_conn(inode);
> + struct fuse_writepage_args *wpa;
> + struct fuse_args_pages *ap;
> +
> + wpa = fuse_writepage_args_alloc();
> + if (!wpa)
> + return NULL;
> +
> + fuse_writepage_add_to_bucket(fc, wpa);
> + fuse_write_args_fill(&wpa->ia, ff, folio_pos(folio), 0);
> + wpa->ia.write.in.write_flags |= FUSE_WRITE_CACHE;
> + wpa->inode = inode;
> + wpa->ia.ff = ff;
> +
> + ap = &wpa->ia.ap;
> + ap->args.in_pages = true;
> + ap->args.end = fuse_writepage_end;
> +
> + return wpa;
> +}
> +
> static int fuse_writepage_locked(struct folio *folio)
> {
> 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 folio *tmp_folio;
> + struct fuse_file *ff;
> int error = -ENOMEM;
>
> - wpa = fuse_writepage_args_alloc();
> - if (!wpa)
> - goto err;
> -
> tmp_folio = folio_alloc(GFP_NOFS | __GFP_HIGHMEM, 0);
> if (!tmp_folio)
> - goto err_free;
> + goto err;
>
> error = -EIO;
> - wpa->ia.ff = fuse_write_file_get(fi);
> - if (!wpa->ia.ff)
> + ff = fuse_write_file_get(fi);
> + if (!ff)
> goto err_nofile;
>
> - fuse_writepage_add_to_bucket(fc, wpa);
> - fuse_write_args_fill(&wpa->ia, wpa->ia.ff, folio_pos(folio), 0);
> -
> - wpa->ia.write.in.write_flags |= FUSE_WRITE_CACHE;
> - wpa->next = NULL;
> - wpa->inode = inode;
> + wpa = fuse_writepage_args_setup(folio, ff);
> + if (!wpa) {
> + error = -ENOMEM;
> + goto err_writepage_args;
> + }
Please use the following pattern, unless there's a good reason not to:
+ error = -ENOMEM;
+ if (!wpa)
+ goto err_writepage_args;
