On Thu, Feb 1, 2024 at 6:46 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Thu, Feb 1, 2024 at 12:51 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > > > On Thu, 1 Feb 2024 at 11:41, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > > I was considering splitting fuse_finish_open() to the first part that > > > can fail and the "finally" part that deals with attributes, but seeing > > > that this entire chunk of atomic_o_trunc code in fuse_finish_open() > > > is never relevant to atomic_open(), I'd rather just move it out > > > into fuse_open_common() which has loads of other code related to > > > atomic_o_trunc anyway? > > > > Yep. > > FWIW, I pushed some cleanups to: > > https://github.com/amir73il/linux/commits/fuse_io_mode-wip/ > > * e71b0c0356c8 - (github/fuse_io_mode-wip) fuse: introduce inode io modes > * 081ddd63a9ff - fuse: prepare for failing open response > * 437b84a47a8a - fuse: allocate ff->release_args only if release is needed > * e2df18f9a3d6 - fuse: factor out helper fuse_truncate_update_attr() > > e2df18f9a3d6 is the O_TRUNC change discussed above. > 437b84a47a8a gets rid of the isdir argument to fuse_file_put(), so this > one liner that you disliked is gone. > > I will see if I can also get the opendir separation cleanup done. > This is what I have in WIP branch - not sure if that is what you meant: * 285a83f439d8 - (github/fuse_io_mode-wip) fuse: introduce inode io modes * cf7e1707a319 - fuse: break up fuse_open_common() * d8fcee9252ca - fuse: prepare for failing open response * 5e4786da5d6e - fuse: allocate ff->release_args only if release is needed * 64a6a415239c - fuse: factor out helper fuse_truncate_update_attr() Thanks, Amir. commit cf7e1707a31990ed5df1294047909fce60cc3ec1 Author: Amir Goldstein <amir73il@xxxxxxxxx> Date: Fri Feb 2 13:30:30 2024 +0200 fuse: break up fuse_open_common() fuse_open_common() has a lot of code relevant only for regular files and O_TRUNC in particular. Copy the little bit of remaining code into fuse_dir_open() and stop using this common helper for directory open. Suggested-by: Miklos Szeredi <miklos@xxxxxxxxxx> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 27daf0bf84ad..3498255402fe 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1632,7 +1632,27 @@ static const char *fuse_get_link(struct dentry *dentry, struct inode *inode, static int fuse_dir_open(struct inode *inode, struct file *file) { - return fuse_open_common(inode, file, true); + struct fuse_mount *fm = get_fuse_mount(inode); + struct fuse_inode *fi = get_fuse_inode(inode); + int err; + + if (fuse_is_bad(inode)) + return -EIO; + + err = generic_file_open(inode, file); + if (err) + return err; + + err = fuse_do_open(fm, get_node_id(inode), file, true); + if (!err) { + struct fuse_file *ff = file->private_data; + + err = fuse_finish_open(inode, file); + if (err) + fuse_sync_release(fi, ff, file->f_flags); + } + + return err; } static int fuse_dir_release(struct inode *inode, struct file *file) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 891bfa8a6724..1d6b3499c069 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -229,7 +229,7 @@ static void fuse_truncate_update_attr(struct inode *inode, struct file *file) fuse_invalidate_attr_mask(inode, FUSE_STATX_MODSIZE); } -int fuse_open_common(struct inode *inode, struct file *file, bool isdir) +int fuse_open(struct inode *inode, struct file *file) { struct fuse_mount *fm = get_fuse_mount(inode); struct fuse_inode *fi = get_fuse_inode(inode); @@ -260,7 +260,7 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir) if (is_wb_truncate || dax_truncate) fuse_set_nowrite(inode); - err = fuse_do_open(fm, get_node_id(inode), file, isdir); + err = fuse_do_open(fm, get_node_id(inode), file, false); if (!err) { ff = file->private_data; err = fuse_finish_open(inode, file); @@ -359,11 +359,6 @@ void fuse_release_common(struct file *file, bool isdir) (fl_owner_t) file, isdir); } -static int fuse_open(struct inode *inode, struct file *file) -{ - return fuse_open_common(inode, file, false); -} - static int fuse_release(struct inode *inode, struct file *file) { struct fuse_conn *fc = get_fuse_conn(inode); diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 536b4515c2c8..9ad5f882bd0a 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -1034,8 +1034,6 @@ void fuse_read_args_fill(struct fuse_io_args *ia, struct file *file, loff_t pos, /** * Send OPEN or OPENDIR request */ -int fuse_open_common(struct inode *inode, struct file *file, bool isdir); - struct fuse_file *fuse_file_alloc(struct fuse_mount *fm, bool release); void fuse_file_free(struct fuse_file *ff); int fuse_finish_open(struct inode *inode, struct file *file); --
