On Fri, 7 Jul 2023 at 15:28, Bernd Schubert <bschubert@xxxxxxx> wrote:
>
> From: Dharmendra Singh <dsingh@xxxxxxx>
>
> This adds full atomic open support, to avoid lookup before open/create.
> If the implementation (fuse server/daemon) does not support atomic open
> it falls back to non-atomic open.
>
> Signed-off-by: Dharmendra Singh <dsingh@xxxxxxx>
> Signed-off-by: Horst Birthelmer <hbirthelmer@xxxxxxx>
> Signed-off-by: Bernd Schubert <bschubert@xxxxxxx>
> ---
> fs/fuse/dir.c | 170 +++++++++++++++++++++++++++++++++++++-
> fs/fuse/fuse_i.h | 3 +
> include/uapi/linux/fuse.h | 3 +
> 3 files changed, 175 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 6ffc573de470..8145bbfc7a40 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -724,7 +724,7 @@ static int _fuse_create_open(struct inode *dir, struct dentry *entry,
>
> static int fuse_mknod(struct mnt_idmap *, struct inode *, struct dentry *,
> umode_t, dev_t);
> -static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
> +static int fuse_create_open(struct inode *dir, struct dentry *entry,
> struct file *file, unsigned flags,
> umode_t mode)
> {
> @@ -770,6 +770,174 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
> return finish_no_open(file, res);
> }
>
> +static int _fuse_atomic_open(struct inode *dir, struct dentry *entry,
> + struct file *file, unsigned flags,
> + umode_t mode)
> +{
> +
> + int err;
> + struct inode *inode;
> + struct fuse_mount *fm = get_fuse_mount(dir);
> + struct fuse_conn *fc = fm->fc;
> + FUSE_ARGS(args);
> + struct fuse_forget_link *forget;
> + struct fuse_create_in inarg;
> + struct fuse_open_out outopen;
> + struct fuse_entry_out outentry;
> + struct fuse_inode *fi;
> + struct fuse_file *ff;
> + struct dentry *res = NULL;
> +
> + /* Userspace expects S_IFREG in create mode */
> + if ((flags & O_CREAT) && (mode & S_IFMT) != S_IFREG) {
> + WARN_ON(1);
> + err = -EINVAL;
> + goto out_err;
> + }
> + forget = fuse_alloc_forget();
> + err = -ENOMEM;
> + if (!forget)
> + goto out_err;
> +
> + err = -ENOMEM;
> + ff = fuse_file_alloc(fm);
> + if (!ff)
> + goto out_put_forget_req;
> +
> + if (!fc->dont_mask)
> + mode &= ~current_umask();
> +
> + flags &= ~O_NOCTTY;
> + memset(&inarg, 0, sizeof(inarg));
> + memset(&outentry, 0, sizeof(outentry));
> + inarg.flags = flags;
> + inarg.mode = mode;
> + inarg.umask = current_umask();
> +
> + if (fc->handle_killpriv_v2 && (flags & O_TRUNC) &&
> + !(flags & O_EXCL) && !capable(CAP_FSETID)) {
> + inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID;
> + }
> +
> + args.opcode = FUSE_OPEN_ATOMIC;
> + args.nodeid = get_node_id(dir);
> + args.in_numargs = 2;
> + args.in_args[0].size = sizeof(inarg);
> + args.in_args[0].value = &inarg;
> + args.in_args[1].size = entry->d_name.len + 1;
> + args.in_args[1].value = entry->d_name.name;
> + args.out_numargs = 2;
> + args.out_args[0].size = sizeof(outentry);
> + args.out_args[0].value = &outentry;
> + args.out_args[1].size = sizeof(outopen);
> + args.out_args[1].value = &outopen;
> +
> + if (flags & O_CREAT) {
> + err = get_create_ext(&args, dir, entry, mode);
> + if (err)
> + goto out_free_ff;
> + }
> +
> + err = fuse_simple_request(fm, &args);
free_ext_value() missing.
Which also begs the question: can't _fuse_create_open() and
_fuse_atomic_open() be consolidated into a common helper? There's
just too much duplication between them to warrant completely separate
implementations.
> + if (err == -ENOSYS) {
> + fc->no_open_atomic = 1;
> + fuse_file_free(ff);
> + kfree(forget);
> + goto fallback;
> + }
> + if (err) {
> + if (err == -ENOENT)
> + fuse_invalidate_entry_cache(entry);
> + goto out_free_ff;
> + }
> +
> + err = -EIO;
> + if (invalid_nodeid(outentry.nodeid) || fuse_invalid_attr(&outentry.attr))
> + goto out_free_ff;
> +
> + ff->fh = outopen.fh;
> + ff->nodeid = outentry.nodeid;
> + ff->open_flags = outopen.open_flags;
> + inode = fuse_iget(dir->i_sb, outentry.nodeid, outentry.generation,
> + &outentry.attr, entry_attr_timeout(&outentry), 0);
> + if (!inode) {
> + flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
> + fuse_sync_release(NULL, ff, flags);
> + fuse_queue_forget(fm->fc, forget, outentry.nodeid, 1);
> + err = -ENOMEM;
> + goto out_err;
> + }
> + if (d_in_lookup(entry)) {
> + res = d_splice_alias(inode, entry);
> + if (res) {
> + if (IS_ERR(res)) {
> + /*
> + * Close the file in user space, but do not unlink it,
> + * if it was created - with network file systems other
> + * clients might have already accessed it.
> + */
> + fi = get_fuse_inode(inode);
> + fuse_sync_release(fi, ff, flags);
> + fuse_queue_forget(fm->fc, forget, outentry.nodeid, 1);
> + err = PTR_ERR(res);
> + goto out_err;
> + }
> + entry = res;
> + }
> + } else
> + d_instantiate(entry, inode);
> + fuse_change_entry_timeout(entry, &outentry);
> +
> + if (outopen.open_flags & FOPEN_FILE_CREATED) {
> + if (!(flags & O_CREAT)) {
> + pr_debug("Server side bug, FOPEN_FILE_CREATED set "
> + "without O_CREAT, ignoring.");
> + } else {
> + /* This should be always set when the file is created */
> + fuse_dir_changed(dir);
> + file->f_mode |= FMODE_CREATED;
> + }
> + }
> +
> + if (!(flags & O_CREAT))
> + fuse_advise_use_readdirplus(dir);
We advise to use readdirplus from lookup, because readdirplus can
substitute for a lookup. But readdirplus cannot substitute for the
atomic open, so it's not a good idea to advise using readdirpuls in
this case. At least that's how I see this.
Thanks,
Miklos
