* [PATCH v1 01/13] fuse: fix missing forget requests in error paths
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 8:18 ` Amir Goldstein
2026-04-24 1:16 ` [PATCH v1 02/13] fuse: clean up fuse_lookup_name() Joanne Koong
` (12 subsequent siblings)
13 siblings, 1 reply; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
If the server returned a valid nodeid but invalid attributes, the kernel
needs to send a forget request to tell the server to release its
reference, else this leads to server-side leaks. These paths only
trigger with a misbehaving server returning invalid attributes, so the
real-world impact is minimal, but forgets should be sent.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 52 ++++++++++++++++++++++++++++++---------------------
1 file changed, 31 insertions(+), 21 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index be41c14ef329..6ab05d7d297d 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -583,7 +583,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
err = -EIO;
if (fuse_invalid_attr(&outarg->attr))
- goto out_put_forget;
+ goto out_queue_forget;
if (outarg->nodeid == FUSE_ROOT_ID && outarg->generation != 0) {
pr_warn_once("root generation should be zero\n");
outarg->generation = 0;
@@ -593,16 +593,18 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
&outarg->attr, ATTR_TIMEOUT(outarg),
attr_version, evict_ctr);
err = -ENOMEM;
- if (!*inode) {
- fuse_chan_queue_forget(fm->fc->chan, forget, outarg->nodeid, 1);
- goto out;
- }
+ if (!*inode)
+ goto out_queue_forget;
err = 0;
out_put_forget:
kfree(forget);
out:
return err;
+
+out_queue_forget:
+ fuse_chan_queue_forget(fm->fc->chan, forget, outarg->nodeid, 1);
+ return err;
}
static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
@@ -882,22 +884,21 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
goto out_free_ff;
err = -EIO;
- if (!S_ISREG(outentry.attr.mode) || invalid_nodeid(outentry.nodeid) ||
- fuse_invalid_attr(&outentry.attr))
+ if (invalid_nodeid(outentry.nodeid))
goto out_free_ff;
ff->fh = outopenp->fh;
ff->nodeid = outentry.nodeid;
ff->open_flags = outopenp->open_flags;
+
+ if (!S_ISREG(outentry.attr.mode) || fuse_invalid_attr(&outentry.attr))
+ goto out_forget;
+
inode = fuse_iget(dir->i_sb, outentry.nodeid, outentry.generation,
&outentry.attr, ATTR_TIMEOUT(&outentry), 0, 0);
- if (!inode) {
- flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
- fuse_sync_release(NULL, ff, flags);
- fuse_chan_queue_forget(fm->fc->chan, forget, outentry.nodeid, 1);
- err = -ENOMEM;
- goto out_err;
- }
+ err = -ENOMEM;
+ if (!inode)
+ goto out_forget;
kfree(forget);
d_instantiate(entry, inode);
entry->d_time = epoch;
@@ -925,6 +926,11 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
kfree(forget);
out_err:
return err;
+out_forget:
+ flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
+ fuse_sync_release(NULL, ff, flags);
+ fuse_chan_queue_forget(fm->fc->chan, forget, outentry.nodeid, 1);
+ return err;
}
static int fuse_mknod(struct mnt_idmap *, struct inode *, struct dentry *,
@@ -1010,18 +1016,18 @@ static struct dentry *create_new_entry(struct mnt_idmap *idmap, struct fuse_moun
goto out_put_forget_req;
err = -EIO;
- if (invalid_nodeid(outarg.nodeid) || fuse_invalid_attr(&outarg.attr))
+ if (invalid_nodeid(outarg.nodeid))
goto out_put_forget_req;
- if ((outarg.attr.mode ^ mode) & S_IFMT)
- goto out_put_forget_req;
+ if (fuse_invalid_attr(&outarg.attr) || (outarg.attr.mode ^ mode) & S_IFMT)
+ goto out_forget;
inode = fuse_iget(dir->i_sb, outarg.nodeid, outarg.generation,
&outarg.attr, ATTR_TIMEOUT(&outarg), 0, 0);
- if (!inode) {
- fuse_chan_queue_forget(fm->fc->chan, forget, outarg.nodeid, 1);
- return ERR_PTR(-ENOMEM);
- }
+ err = -ENOMEM;
+ if (!inode)
+ goto out_forget;
+
kfree(forget);
d_drop(entry);
@@ -1044,6 +1050,10 @@ static struct dentry *create_new_entry(struct mnt_idmap *idmap, struct fuse_moun
fuse_invalidate_entry(entry);
kfree(forget);
return ERR_PTR(err);
+
+out_forget:
+ fuse_chan_queue_forget(fm->fc->chan, forget, outarg.nodeid, 1);
+ return ERR_PTR(err);
}
static int create_new_nondir(struct mnt_idmap *idmap, struct fuse_mount *fm,
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 01/13] fuse: fix missing forget requests in error paths
2026-04-24 1:16 ` [PATCH v1 01/13] fuse: fix missing forget requests in error paths Joanne Koong
@ 2026-04-24 8:18 ` Amir Goldstein
2026-04-24 17:43 ` Joanne Koong
0 siblings, 1 reply; 19+ messages in thread
From: Amir Goldstein @ 2026-04-24 8:18 UTC (permalink / raw)
To: Joanne Koong; +Cc: miklos, fuse-devel
On Thu, Apr 23, 2026 at 06:16:34PM -0700, Joanne Koong wrote:
> If the server returned a valid nodeid but invalid attributes, the kernel
> needs to send a forget request to tell the server to release its
> reference, else this leads to server-side leaks. These paths only
> trigger with a misbehaving server returning invalid attributes, so the
> real-world impact is minimal, but forgets should be sent.
>
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> ---
> fs/fuse/dir.c | 52 ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 31 insertions(+), 21 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index be41c14ef329..6ab05d7d297d 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -583,7 +583,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
>
> err = -EIO;
> if (fuse_invalid_attr(&outarg->attr))
> - goto out_put_forget;
> + goto out_queue_forget;
> if (outarg->nodeid == FUSE_ROOT_ID && outarg->generation != 0) {
> pr_warn_once("root generation should be zero\n");
> outarg->generation = 0;
> @@ -593,16 +593,18 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
> &outarg->attr, ATTR_TIMEOUT(outarg),
> attr_version, evict_ctr);
> err = -ENOMEM;
> - if (!*inode) {
> - fuse_chan_queue_forget(fm->fc->chan, forget, outarg->nodeid, 1);
> - goto out;
> - }
> + if (!*inode)
> + goto out_queue_forget;
> err = 0;
>
> out_put_forget:
> kfree(forget);
> out:
> return err;
> +
> +out_queue_forget:
> + fuse_chan_queue_forget(fm->fc->chan, forget, outarg->nodeid, 1);
> + return err;
> }
>
> static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
> @@ -882,22 +884,21 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
> goto out_free_ff;
>
> err = -EIO;
> - if (!S_ISREG(outentry.attr.mode) || invalid_nodeid(outentry.nodeid) ||
> - fuse_invalid_attr(&outentry.attr))
> + if (invalid_nodeid(outentry.nodeid))
> goto out_free_ff;
>
> ff->fh = outopenp->fh;
> ff->nodeid = outentry.nodeid;
> ff->open_flags = outopenp->open_flags;
> +
> + if (!S_ISREG(outentry.attr.mode) || fuse_invalid_attr(&outentry.attr))
> + goto out_forget;
> +
> inode = fuse_iget(dir->i_sb, outentry.nodeid, outentry.generation,
> &outentry.attr, ATTR_TIMEOUT(&outentry), 0, 0);
> - if (!inode) {
> - flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
> - fuse_sync_release(NULL, ff, flags);
> - fuse_chan_queue_forget(fm->fc->chan, forget, outentry.nodeid, 1);
> - err = -ENOMEM;
> - goto out_err;
> - }
> + err = -ENOMEM;
> + if (!inode)
> + goto out_forget;
> kfree(forget);
> d_instantiate(entry, inode);
> entry->d_time = epoch;
> @@ -925,6 +926,11 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
> kfree(forget);
> out_err:
> return err;
> +out_forget:
> + flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
> + fuse_sync_release(NULL, ff, flags);
> + fuse_chan_queue_forget(fm->fc->chan, forget, outentry.nodeid, 1);
> + return err;
> }
>
> static int fuse_mknod(struct mnt_idmap *, struct inode *, struct dentry *,
> @@ -1010,18 +1016,18 @@ static struct dentry *create_new_entry(struct mnt_idmap *idmap, struct fuse_moun
> goto out_put_forget_req;
>
> err = -EIO;
> - if (invalid_nodeid(outarg.nodeid) || fuse_invalid_attr(&outarg.attr))
> + if (invalid_nodeid(outarg.nodeid))
> goto out_put_forget_req;
>
> - if ((outarg.attr.mode ^ mode) & S_IFMT)
> - goto out_put_forget_req;
> + if (fuse_invalid_attr(&outarg.attr) || (outarg.attr.mode ^ mode) & S_IFMT)
This seems like a repeating pattern.
Would you consider hiding this behind a helper that validates we got
an expected set of attr:
if (fuse_invalid_attr_ftype(&outarg.attr, mode & S_IFMT))
Thanks,
Amir.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v1 01/13] fuse: fix missing forget requests in error paths
2026-04-24 8:18 ` Amir Goldstein
@ 2026-04-24 17:43 ` Joanne Koong
0 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 17:43 UTC (permalink / raw)
To: Amir Goldstein; +Cc: miklos, fuse-devel
On Fri, Apr 24, 2026 at 1:18 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Thu, Apr 23, 2026 at 06:16:34PM -0700, Joanne Koong wrote:
> > If the server returned a valid nodeid but invalid attributes, the kernel
> > needs to send a forget request to tell the server to release its
> > reference, else this leads to server-side leaks. These paths only
> > trigger with a misbehaving server returning invalid attributes, so the
> > real-world impact is minimal, but forgets should be sent.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > ---
> > fs/fuse/dir.c | 52 ++++++++++++++++++++++++++++++---------------------
> > 1 file changed, 31 insertions(+), 21 deletions(-)
> >
> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index be41c14ef329..6ab05d7d297d 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -583,7 +583,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
> >
> > err = -EIO;
> > if (fuse_invalid_attr(&outarg->attr))
> > - goto out_put_forget;
> > + goto out_queue_forget;
> > if (outarg->nodeid == FUSE_ROOT_ID && outarg->generation != 0) {
> > pr_warn_once("root generation should be zero\n");
> > outarg->generation = 0;
> > @@ -593,16 +593,18 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
> > &outarg->attr, ATTR_TIMEOUT(outarg),
> > attr_version, evict_ctr);
> > err = -ENOMEM;
> > - if (!*inode) {
> > - fuse_chan_queue_forget(fm->fc->chan, forget, outarg->nodeid, 1);
> > - goto out;
> > - }
> > + if (!*inode)
> > + goto out_queue_forget;
> > err = 0;
> >
> > out_put_forget:
> > kfree(forget);
> > out:
> > return err;
> > +
> > +out_queue_forget:
> > + fuse_chan_queue_forget(fm->fc->chan, forget, outarg->nodeid, 1);
> > + return err;
> > }
> >
> > static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
> > @@ -882,22 +884,21 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
> > goto out_free_ff;
> >
> > err = -EIO;
> > - if (!S_ISREG(outentry.attr.mode) || invalid_nodeid(outentry.nodeid) ||
> > - fuse_invalid_attr(&outentry.attr))
> > + if (invalid_nodeid(outentry.nodeid))
> > goto out_free_ff;
> >
> > ff->fh = outopenp->fh;
> > ff->nodeid = outentry.nodeid;
> > ff->open_flags = outopenp->open_flags;
> > +
> > + if (!S_ISREG(outentry.attr.mode) || fuse_invalid_attr(&outentry.attr))
> > + goto out_forget;
> > +
> > inode = fuse_iget(dir->i_sb, outentry.nodeid, outentry.generation,
> > &outentry.attr, ATTR_TIMEOUT(&outentry), 0, 0);
> > - if (!inode) {
> > - flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
> > - fuse_sync_release(NULL, ff, flags);
> > - fuse_chan_queue_forget(fm->fc->chan, forget, outentry.nodeid, 1);
> > - err = -ENOMEM;
> > - goto out_err;
> > - }
> > + err = -ENOMEM;
> > + if (!inode)
> > + goto out_forget;
> > kfree(forget);
> > d_instantiate(entry, inode);
> > entry->d_time = epoch;
> > @@ -925,6 +926,11 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
> > kfree(forget);
> > out_err:
> > return err;
> > +out_forget:
> > + flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
> > + fuse_sync_release(NULL, ff, flags);
> > + fuse_chan_queue_forget(fm->fc->chan, forget, outentry.nodeid, 1);
> > + return err;
> > }
> >
> > static int fuse_mknod(struct mnt_idmap *, struct inode *, struct dentry *,
> > @@ -1010,18 +1016,18 @@ static struct dentry *create_new_entry(struct mnt_idmap *idmap, struct fuse_moun
> > goto out_put_forget_req;
> >
> > err = -EIO;
> > - if (invalid_nodeid(outarg.nodeid) || fuse_invalid_attr(&outarg.attr))
> > + if (invalid_nodeid(outarg.nodeid))
> > goto out_put_forget_req;
> >
> > - if ((outarg.attr.mode ^ mode) & S_IFMT)
> > - goto out_put_forget_req;
> > + if (fuse_invalid_attr(&outarg.attr) || (outarg.attr.mode ^ mode) & S_IFMT)
>
> This seems like a repeating pattern.
> Would you consider hiding this behind a helper that validates we got
> an expected set of attr:
>
> if (fuse_invalid_attr_ftype(&outarg.attr, mode & S_IFMT))
I will add this helper.
Thanks,
Joanne
>
> Thanks,
> Amir.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 02/13] fuse: clean up fuse_lookup_name()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
2026-04-24 1:16 ` [PATCH v1 01/13] fuse: fix missing forget requests in error paths Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 1:16 ` [PATCH v1 03/13] fuse: clean up fuse_lookup() Joanne Koong
` (11 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Remove goto with direct returns. Assign fm->fc to a local variable and
use that. No functional changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 6ab05d7d297d..31dc87cfa1af 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -558,22 +558,20 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
struct fuse_mount *fm = get_fuse_mount_super(sb);
FUSE_ARGS(args);
struct fuse_forget_link *forget;
+ struct fuse_conn *fc = fm->fc;
u64 attr_version, evict_ctr;
int err;
*inode = NULL;
- err = -ENAMETOOLONG;
- if (name->len > fm->fc->name_max)
- goto out;
-
+ if (name->len > fc->name_max)
+ return -ENAMETOOLONG;
forget = fuse_alloc_forget();
- err = -ENOMEM;
if (!forget)
- goto out;
+ return -ENOMEM;
- attr_version = fuse_get_attr_version(fm->fc);
- evict_ctr = fuse_get_evict_ctr(fm->fc);
+ attr_version = fuse_get_attr_version(fc);
+ evict_ctr = fuse_get_evict_ctr(fc);
fuse_lookup_init(&args, nodeid, name, outarg);
err = fuse_simple_request(fm, &args);
@@ -597,13 +595,12 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
goto out_queue_forget;
err = 0;
- out_put_forget:
+out_put_forget:
kfree(forget);
- out:
return err;
out_queue_forget:
- fuse_chan_queue_forget(fm->fc->chan, forget, outarg->nodeid, 1);
+ fuse_chan_queue_forget(fc->chan, forget, outarg->nodeid, 1);
return err;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v1 03/13] fuse: clean up fuse_lookup()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
2026-04-24 1:16 ` [PATCH v1 01/13] fuse: fix missing forget requests in error paths Joanne Koong
2026-04-24 1:16 ` [PATCH v1 02/13] fuse: clean up fuse_lookup_name() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 1:16 ` [PATCH v1 04/13] fuse: clean up fuse_dentry_revalidate() Joanne Koong
` (10 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Remove gotos with direct returns and simplify the d_splice_alias error
path. No functional changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 31dc87cfa1af..112d979c49c5 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -625,21 +625,19 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
err = fuse_lookup_name(dir->i_sb, get_node_id(dir), &entry->d_name,
&outarg, &inode);
fuse_unlock_inode(dir, locked);
- if (err == -ENOENT) {
+ if (err == -ENOENT)
outarg_valid = false;
- err = 0;
- }
- if (err)
- goto out_err;
+ else if (err)
+ return ERR_PTR(err);
- err = -EIO;
- if (inode && get_node_id(inode) == FUSE_ROOT_ID)
- goto out_iput;
+ if (inode && get_node_id(inode) == FUSE_ROOT_ID) {
+ iput(inode);
+ return ERR_PTR(-EIO);
+ }
newent = d_splice_alias(inode, entry);
- err = PTR_ERR(newent);
if (IS_ERR(newent))
- goto out_err;
+ return newent;
entry = newent ? newent : entry;
entry->d_time = epoch;
@@ -651,11 +649,6 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
if (inode)
fuse_advise_use_readdirplus(dir);
return newent;
-
- out_iput:
- iput(inode);
- out_err:
- return ERR_PTR(err);
}
static int get_security_context(struct dentry *entry, umode_t mode,
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v1 04/13] fuse: clean up fuse_dentry_revalidate()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (2 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 03/13] fuse: clean up fuse_lookup() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 8:20 ` Amir Goldstein
2026-04-24 1:16 ` [PATCH v1 05/13] fuse: clean up fuse_create_open() Joanne Koong
` (9 subsequent siblings)
13 siblings, 1 reply; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel; +Cc: Amir Goldstein
Remove gotos with direct returns and flatten nested if/else structure.
No functional changes.
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
Cc: Amir Goldstein <amir73il@gmail.com>
---
fs/fuse/dir.c | 114 ++++++++++++++++++++++++--------------------------
1 file changed, 54 insertions(+), 60 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 112d979c49c5..c5b3cb001147 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -386,72 +386,32 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
struct dentry *entry, unsigned int flags)
{
struct inode *inode;
- struct fuse_mount *fm;
struct fuse_conn *fc;
struct fuse_inode *fi;
+ struct fuse_entry_out outarg;
+ struct fuse_forget_link *forget;
+ FUSE_ARGS(args);
+ bool automount_changed;
+ u64 attr_version;
+ bool need_reval;
int ret;
fc = get_fuse_conn_super(dir->i_sb);
if (entry->d_time < atomic_read(&fc->epoch))
- goto invalid;
+ return 0;
inode = d_inode_rcu(entry);
if (inode && fuse_is_bad(inode))
- goto invalid;
- else if (time_before64(fuse_dentry_time(entry), get_jiffies_64()) ||
- (flags & (LOOKUP_EXCL | LOOKUP_REVAL | LOOKUP_RENAME_TARGET))) {
- struct fuse_entry_out outarg;
- FUSE_ARGS(args);
- struct fuse_forget_link *forget;
- u64 attr_version;
-
- /* For negative dentries, always do a fresh lookup */
- if (!inode)
- goto invalid;
-
- ret = -ECHILD;
- if (flags & LOOKUP_RCU)
- goto out;
+ return 0;
- fm = get_fuse_mount(inode);
+ need_reval = time_before64(fuse_dentry_time(entry), get_jiffies_64()) ||
+ (flags & (LOOKUP_EXCL | LOOKUP_REVAL | LOOKUP_RENAME_TARGET));
- forget = fuse_alloc_forget();
- ret = -ENOMEM;
- if (!forget)
- goto out;
-
- attr_version = fuse_get_attr_version(fm->fc);
-
- fuse_lookup_init(&args, get_node_id(dir), name, &outarg);
- ret = fuse_simple_request(fm, &args);
- /* Zero nodeid is same as -ENOENT */
- if (!ret && !outarg.nodeid)
- ret = -ENOENT;
- if (!ret) {
- fi = get_fuse_inode(inode);
- if (outarg.nodeid != get_node_id(inode) ||
- (bool) IS_AUTOMOUNT(inode) != (bool) (outarg.attr.flags & FUSE_ATTR_SUBMOUNT)) {
- fuse_chan_queue_forget(fm->fc->chan, forget,
- outarg.nodeid, 1);
- goto invalid;
- }
- spin_lock(&fi->lock);
- fi->nlookup++;
- spin_unlock(&fi->lock);
- }
- kfree(forget);
- if (ret == -ENOMEM || ret == -EINTR)
- goto out;
- if (ret || fuse_invalid_attr(&outarg.attr) ||
- fuse_stale_inode(inode, outarg.generation, &outarg.attr))
- goto invalid;
+ /* For negative dentries that need revalidation, always do a fresh lookup */
+ if (!inode)
+ return need_reval ? 0 : 1;
- forget_all_cached_acls(inode);
- fuse_change_attributes(inode, &outarg.attr, NULL,
- ATTR_TIMEOUT(&outarg),
- attr_version);
- fuse_change_entry_timeout(entry, &outarg);
- } else if (inode) {
+ if (!need_reval) {
fi = get_fuse_inode(inode);
if (flags & LOOKUP_RCU) {
if (test_bit(FUSE_I_INIT_RDPLUS, &fi->state))
@@ -459,14 +419,48 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
} else if (test_and_clear_bit(FUSE_I_INIT_RDPLUS, &fi->state)) {
fuse_advise_use_readdirplus(dir);
}
+
+ return 1;
}
- ret = 1;
-out:
- return ret;
-invalid:
- ret = 0;
- goto out;
+ if (flags & LOOKUP_RCU)
+ return -ECHILD;
+
+ forget = fuse_alloc_forget();
+ if (!forget)
+ return -ENOMEM;
+
+ attr_version = fuse_get_attr_version(fc);
+
+ fuse_lookup_init(&args, get_node_id(dir), name, &outarg);
+ ret = fuse_simple_request(get_fuse_mount(inode), &args);
+ if (ret || !outarg.nodeid) {
+ kfree(forget);
+ return (ret == -ENOMEM || ret == -EINTR) ? ret : 0;
+ }
+
+ automount_changed =
+ !!IS_AUTOMOUNT(inode) != !!(outarg.attr.flags & FUSE_ATTR_SUBMOUNT);
+
+ if (outarg.nodeid != get_node_id(inode) || automount_changed) {
+ fuse_chan_queue_forget(fc->chan, forget, outarg.nodeid, 1);
+ return 0;
+ }
+ fi = get_fuse_inode(inode);
+ spin_lock(&fi->lock);
+ fi->nlookup++;
+ spin_unlock(&fi->lock);
+
+ kfree(forget);
+ if (fuse_invalid_attr(&outarg.attr) ||
+ fuse_stale_inode(inode, outarg.generation, &outarg.attr))
+ return 0;
+
+ forget_all_cached_acls(inode);
+ fuse_change_attributes(inode, &outarg.attr, NULL, ATTR_TIMEOUT(&outarg),
+ attr_version);
+ fuse_change_entry_timeout(entry, &outarg);
+ return 1;
}
static int fuse_dentry_init(struct dentry *dentry)
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 04/13] fuse: clean up fuse_dentry_revalidate()
2026-04-24 1:16 ` [PATCH v1 04/13] fuse: clean up fuse_dentry_revalidate() Joanne Koong
@ 2026-04-24 8:20 ` Amir Goldstein
0 siblings, 0 replies; 19+ messages in thread
From: Amir Goldstein @ 2026-04-24 8:20 UTC (permalink / raw)
To: Joanne Koong; +Cc: miklos, fuse-devel
On Fri, Apr 24, 2026 at 3:18 AM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> Remove gotos with direct returns and flatten nested if/else structure.
> No functional changes.
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> Cc: Amir Goldstein <amir73il@gmail.com>
> ---
> fs/fuse/dir.c | 114 ++++++++++++++++++++++++--------------------------
> 1 file changed, 54 insertions(+), 60 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 112d979c49c5..c5b3cb001147 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -386,72 +386,32 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
> struct dentry *entry, unsigned int flags)
> {
> struct inode *inode;
> - struct fuse_mount *fm;
> struct fuse_conn *fc;
> struct fuse_inode *fi;
> + struct fuse_entry_out outarg;
> + struct fuse_forget_link *forget;
> + FUSE_ARGS(args);
> + bool automount_changed;
> + u64 attr_version;
> + bool need_reval;
> int ret;
>
> fc = get_fuse_conn_super(dir->i_sb);
> if (entry->d_time < atomic_read(&fc->epoch))
> - goto invalid;
> + return 0;
>
> inode = d_inode_rcu(entry);
> if (inode && fuse_is_bad(inode))
> - goto invalid;
> - else if (time_before64(fuse_dentry_time(entry), get_jiffies_64()) ||
> - (flags & (LOOKUP_EXCL | LOOKUP_REVAL | LOOKUP_RENAME_TARGET))) {
> - struct fuse_entry_out outarg;
> - FUSE_ARGS(args);
> - struct fuse_forget_link *forget;
> - u64 attr_version;
> -
> - /* For negative dentries, always do a fresh lookup */
> - if (!inode)
> - goto invalid;
> -
> - ret = -ECHILD;
> - if (flags & LOOKUP_RCU)
> - goto out;
> + return 0;
>
> - fm = get_fuse_mount(inode);
> + need_reval = time_before64(fuse_dentry_time(entry), get_jiffies_64()) ||
> + (flags & (LOOKUP_EXCL | LOOKUP_REVAL | LOOKUP_RENAME_TARGET));
>
> - forget = fuse_alloc_forget();
> - ret = -ENOMEM;
> - if (!forget)
> - goto out;
> -
> - attr_version = fuse_get_attr_version(fm->fc);
> -
> - fuse_lookup_init(&args, get_node_id(dir), name, &outarg);
> - ret = fuse_simple_request(fm, &args);
> - /* Zero nodeid is same as -ENOENT */
> - if (!ret && !outarg.nodeid)
> - ret = -ENOENT;
> - if (!ret) {
> - fi = get_fuse_inode(inode);
> - if (outarg.nodeid != get_node_id(inode) ||
> - (bool) IS_AUTOMOUNT(inode) != (bool) (outarg.attr.flags & FUSE_ATTR_SUBMOUNT)) {
> - fuse_chan_queue_forget(fm->fc->chan, forget,
> - outarg.nodeid, 1);
> - goto invalid;
> - }
> - spin_lock(&fi->lock);
> - fi->nlookup++;
> - spin_unlock(&fi->lock);
> - }
> - kfree(forget);
> - if (ret == -ENOMEM || ret == -EINTR)
> - goto out;
> - if (ret || fuse_invalid_attr(&outarg.attr) ||
> - fuse_stale_inode(inode, outarg.generation, &outarg.attr))
> - goto invalid;
> + /* For negative dentries that need revalidation, always do a fresh lookup */
> + if (!inode)
> + return need_reval ? 0 : 1;
>
> - forget_all_cached_acls(inode);
> - fuse_change_attributes(inode, &outarg.attr, NULL,
> - ATTR_TIMEOUT(&outarg),
> - attr_version);
> - fuse_change_entry_timeout(entry, &outarg);
> - } else if (inode) {
> + if (!need_reval) {
> fi = get_fuse_inode(inode);
> if (flags & LOOKUP_RCU) {
> if (test_bit(FUSE_I_INIT_RDPLUS, &fi->state))
> @@ -459,14 +419,48 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
> } else if (test_and_clear_bit(FUSE_I_INIT_RDPLUS, &fi->state)) {
> fuse_advise_use_readdirplus(dir);
> }
> +
> + return 1;
> }
> - ret = 1;
> -out:
> - return ret;
>
> -invalid:
> - ret = 0;
> - goto out;
> + if (flags & LOOKUP_RCU)
> + return -ECHILD;
> +
> + forget = fuse_alloc_forget();
> + if (!forget)
> + return -ENOMEM;
> +
> + attr_version = fuse_get_attr_version(fc);
> +
> + fuse_lookup_init(&args, get_node_id(dir), name, &outarg);
> + ret = fuse_simple_request(get_fuse_mount(inode), &args);
> + if (ret || !outarg.nodeid) {
> + kfree(forget);
> + return (ret == -ENOMEM || ret == -EINTR) ? ret : 0;
> + }
> +
> + automount_changed =
> + !!IS_AUTOMOUNT(inode) != !!(outarg.attr.flags & FUSE_ATTR_SUBMOUNT);
I am not sure that this help var is really useful?
It is used just once in the next line and its name does not clarify
something that needs to be clarified.
> +
> + if (outarg.nodeid != get_node_id(inode) || automount_changed) {
> + fuse_chan_queue_forget(fc->chan, forget, outarg.nodeid, 1);
> + return 0;
> + }
> + fi = get_fuse_inode(inode);
> + spin_lock(&fi->lock);
> + fi->nlookup++;
> + spin_unlock(&fi->lock);
> +
> + kfree(forget);
Nit: line spacing is weird.
I would do this kfree before the nlookup++ block.
> + if (fuse_invalid_attr(&outarg.attr) ||
> + fuse_stale_inode(inode, outarg.generation, &outarg.attr))
> + return 0;
> +
> + forget_all_cached_acls(inode);
> + fuse_change_attributes(inode, &outarg.attr, NULL, ATTR_TIMEOUT(&outarg),
> + attr_version);
> + fuse_change_entry_timeout(entry, &outarg);
> + return 1;
> }
>
Apart from the nits, feel free to add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Thanks,
Amir.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v1 05/13] fuse: clean up fuse_create_open()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (3 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 04/13] fuse: clean up fuse_dentry_revalidate() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 1:16 ` [PATCH v1 06/13] fuse: clean up fuse_rename2() Joanne Koong
` (8 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Remove goto with direct return. No functional changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index c5b3cb001147..4a6c9e2340bb 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -820,9 +820,8 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
epoch = atomic_read(&fm->fc->epoch);
forget = fuse_alloc_forget();
- err = -ENOMEM;
if (!forget)
- goto out_err;
+ return -ENOMEM;
ff = fuse_file_alloc(fm, true);
if (!ff)
@@ -908,7 +907,6 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
fuse_file_free(ff);
out_put_forget_req:
kfree(forget);
-out_err:
return err;
out_forget:
flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v1 06/13] fuse: clean up fuse_rename2()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (4 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 05/13] fuse: clean up fuse_create_open() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 1:16 ` [PATCH v1 07/13] fuse: clean up fuse_time_to_jiffies() Joanne Koong
` (7 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Move flag validation into the flags branch and return early to eliminate
the else block. No functional changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 4a6c9e2340bb..b87236af4bf7 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1293,10 +1293,10 @@ static int fuse_rename2(struct mnt_idmap *idmap, struct inode *olddir,
if (fuse_is_bad(olddir))
return -EIO;
- if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))
- return -EINVAL;
-
if (flags) {
+ if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))
+ return -EINVAL;
+
if (fc->no_rename2 || fc->minor < 23)
return -EINVAL;
@@ -1308,13 +1308,12 @@ static int fuse_rename2(struct mnt_idmap *idmap, struct inode *olddir,
fc->no_rename2 = 1;
err = -EINVAL;
}
- } else {
- err = fuse_rename_common(&invalid_mnt_idmap, olddir, oldent, newdir, newent, 0,
- FUSE_RENAME,
- sizeof(struct fuse_rename_in));
+ return err;
}
- return err;
+ return fuse_rename_common(&invalid_mnt_idmap, olddir, oldent, newdir,
+ newent, 0, FUSE_RENAME,
+ sizeof(struct fuse_rename_in));
}
static int fuse_link(struct dentry *entry, struct inode *newdir,
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v1 07/13] fuse: clean up fuse_time_to_jiffies()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (5 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 06/13] fuse: clean up fuse_rename2() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 1:16 ` [PATCH v1 08/13] fuse: clean up fuse_link() Joanne Koong
` (6 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Remove unnecessary else statement. No functional changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index b87236af4bf7..45bbbac8477d 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -288,8 +288,8 @@ u64 fuse_time_to_jiffies(u64 sec, u32 nsec)
};
return get_jiffies_64() + timespec64_to_jiffies(&ts);
- } else
- return 0;
+ }
+ return 0;
}
/*
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v1 08/13] fuse: clean up fuse_link()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (6 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 07/13] fuse: clean up fuse_time_to_jiffies() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 1:16 ` [PATCH v1 09/13] fuse: clean up fuse_do_getattr() Joanne Koong
` (5 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Remove goto with direct return and have -ENOSYS check as an else if. No
functional changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 45bbbac8477d..e4d80069c3d8 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1326,7 +1326,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
FUSE_ARGS(args);
if (fm->fc->no_link)
- goto out;
+ return -EPERM;
memset(&inarg, 0, sizeof(inarg));
inarg.oldnodeid = get_node_id(inode);
@@ -1341,10 +1341,9 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
fuse_update_ctime_in_cache(inode);
else if (err == -EINTR)
fuse_invalidate_attr(inode);
-
- if (err == -ENOSYS)
+ else if (err == -ENOSYS)
fm->fc->no_link = 1;
-out:
+
if (fm->fc->no_link)
return -EPERM;
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v1 09/13] fuse: clean up fuse_do_getattr()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (7 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 08/13] fuse: clean up fuse_link() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 1:16 ` [PATCH v1 10/13] fuse: clean up fuse_update_get_attr() Joanne Koong
` (4 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Restructure logic to reduce if-else nesting. No functional changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index e4d80069c3d8..cb34138d083b 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1495,20 +1495,20 @@ static int fuse_do_getattr(struct mnt_idmap *idmap, struct inode *inode,
args.out_args[0].size = sizeof(outarg);
args.out_args[0].value = &outarg;
err = fuse_simple_request(fm, &args);
- if (!err) {
- if (fuse_invalid_attr(&outarg.attr) ||
- inode_wrong_type(inode, outarg.attr.mode)) {
- fuse_make_bad(inode);
- err = -EIO;
- } else {
- fuse_change_attributes(inode, &outarg.attr, NULL,
- ATTR_TIMEOUT(&outarg),
- attr_version);
- if (stat)
- fuse_fillattr(idmap, inode, &outarg.attr, stat);
- }
+ if (err)
+ return err;
+
+ if (fuse_invalid_attr(&outarg.attr) ||
+ inode_wrong_type(inode, outarg.attr.mode)) {
+ fuse_make_bad(inode);
+ return -EIO;
}
- return err;
+ fuse_change_attributes(inode, &outarg.attr, NULL, ATTR_TIMEOUT(&outarg),
+ attr_version);
+ if (stat)
+ fuse_fillattr(idmap, inode, &outarg.attr, stat);
+
+ return 0;
}
static int fuse_update_get_attr(struct mnt_idmap *idmap, struct inode *inode,
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v1 10/13] fuse: clean up fuse_update_get_attr()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (8 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 09/13] fuse: clean up fuse_do_getattr() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 1:16 ` [PATCH v1 11/13] fuse: clean up fuse_get_link() Joanne Koong
` (3 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Consolidate logic for determining sync and return early from the sync
path to reduce if-else nesting. No functional changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index cb34138d083b..4f58adb0fdae 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1529,13 +1529,10 @@ static int fuse_update_get_attr(struct mnt_idmap *idmap, struct inode *inode,
if (fc->no_statx)
request_mask &= STATX_BASIC_STATS;
- if (!request_mask)
+ if (!request_mask || flags & AT_STATX_DONT_SYNC)
sync = false;
- else if (flags & AT_STATX_FORCE_SYNC)
- sync = true;
- else if (flags & AT_STATX_DONT_SYNC)
- sync = false;
- else if (request_mask & inval_mask & ~cache_mask)
+ else if ((flags & AT_STATX_FORCE_SYNC) ||
+ (request_mask & inval_mask & ~cache_mask))
sync = true;
else
sync = time_before64(fi->i_time, get_jiffies_64());
@@ -1550,10 +1547,12 @@ static int fuse_update_get_attr(struct mnt_idmap *idmap, struct inode *inode,
err = 0;
goto retry;
}
- } else {
- err = fuse_do_getattr(idmap, inode, stat, file);
+ return err;
}
- } else if (stat) {
+ return fuse_do_getattr(idmap, inode, stat, file);
+ }
+
+ if (stat) {
generic_fillattr(idmap, request_mask, inode, stat);
stat->mode = fi->orig_i_mode;
stat->ino = fi->orig_ino;
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v1 11/13] fuse: clean up fuse_get_link()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (9 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 10/13] fuse: clean up fuse_update_get_attr() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 1:16 ` [PATCH v1 12/13] fuse: clean up fuse_dir_open() Joanne Koong
` (2 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Replace goto with direct return. No functional changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 4f58adb0fdae..f186bf15c6f0 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1847,34 +1847,28 @@ static const char *fuse_get_link(struct dentry *dentry, struct inode *inode,
struct folio *folio;
int err;
- err = -EIO;
if (fuse_is_bad(inode))
- goto out_err;
+ return ERR_PTR(-EIO);
if (fc->cache_symlinks)
return page_get_link_raw(dentry, inode, callback);
- err = -ECHILD;
if (!dentry)
- goto out_err;
+ return ERR_PTR(-ECHILD);
folio = folio_alloc(GFP_KERNEL, 0);
- err = -ENOMEM;
if (!folio)
- goto out_err;
+ return ERR_PTR(-ENOMEM);
err = fuse_readlink_folio(inode, folio);
if (err) {
folio_put(folio);
- goto out_err;
+ return ERR_PTR(err);
}
set_delayed_call(callback, page_put_link, folio);
return folio_address(folio);
-
-out_err:
- return ERR_PTR(err);
}
static int fuse_dir_open(struct inode *inode, struct file *file)
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v1 12/13] fuse: clean up fuse_dir_open()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (10 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 11/13] fuse: clean up fuse_get_link() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 1:16 ` [PATCH v1 13/13] fuse: clean up setattr() Joanne Koong
2026-04-24 7:51 ` [PATCH v1 00/13] fuse: dir.c cleanups Amir Goldstein
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Restructure logic to reduce if-else nesting. No functional changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index f186bf15c6f0..bdeea7065c5c 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1873,6 +1873,7 @@ static const char *fuse_get_link(struct dentry *dentry, struct inode *inode,
static int fuse_dir_open(struct inode *inode, struct file *file)
{
+ struct fuse_file *ff;
struct fuse_mount *fm = get_fuse_mount(inode);
int err;
@@ -1884,21 +1885,21 @@ static int fuse_dir_open(struct inode *inode, struct file *file)
return err;
err = fuse_do_open(fm, get_node_id(inode), file, true);
- if (!err) {
- struct fuse_file *ff = file->private_data;
+ if (err)
+ return err;
- /*
- * Keep handling FOPEN_STREAM and FOPEN_NONSEEKABLE for
- * directories for backward compatibility, though it's unlikely
- * to be useful.
- */
- if (ff->open_flags & (FOPEN_STREAM | FOPEN_NONSEEKABLE))
- nonseekable_open(inode, file);
- if (!(ff->open_flags & FOPEN_KEEP_CACHE))
- invalidate_inode_pages2(inode->i_mapping);
- }
+ ff = file->private_data;
- return err;
+ /*
+ * Keep handling FOPEN_STREAM and FOPEN_NONSEEKABLE for directories for
+ * backward compatibility, though it's unlikely to be useful.
+ */
+ if (ff->open_flags & (FOPEN_STREAM | FOPEN_NONSEEKABLE))
+ nonseekable_open(inode, file);
+ if (!(ff->open_flags & FOPEN_KEEP_CACHE))
+ invalidate_inode_pages2(inode->i_mapping);
+
+ return 0;
}
static int fuse_dir_release(struct inode *inode, struct file *file)
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v1 13/13] fuse: clean up setattr()
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (11 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 12/13] fuse: clean up fuse_dir_open() Joanne Koong
@ 2026-04-24 1:16 ` Joanne Koong
2026-04-24 7:51 ` [PATCH v1 00/13] fuse: dir.c cleanups Amir Goldstein
13 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 1:16 UTC (permalink / raw)
To: miklos, fuse-devel
Use cached fc in fuse_do_setattr() and fuse_setattr(). No functional
changes.
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
fs/fuse/dir.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index bdeea7065c5c..e97b216f685c 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -2220,7 +2220,7 @@ int fuse_do_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
inarg.valid |= FATTR_KILL_SUIDGID;
}
- attr_version = fuse_get_attr_version(fm->fc);
+ attr_version = fuse_get_attr_version(fc);
fuse_setattr_fill(fc, &args, inode, &inarg, &outarg);
err = fuse_simple_request(fm, &args);
if (err) {
@@ -2307,7 +2307,7 @@ static int fuse_setattr(struct mnt_idmap *idmap, struct dentry *entry,
if (fuse_is_bad(inode))
return -EIO;
- if (!fuse_allow_current_process(get_fuse_conn(inode)))
+ if (!fuse_allow_current_process(fc))
return -EACCES;
if (attr->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID)) {
--
2.52.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v1 00/13] fuse: dir.c cleanups
2026-04-24 1:16 [PATCH v1 00/13] fuse: dir.c cleanups Joanne Koong
` (12 preceding siblings ...)
2026-04-24 1:16 ` [PATCH v1 13/13] fuse: clean up setattr() Joanne Koong
@ 2026-04-24 7:51 ` Amir Goldstein
2026-04-24 17:27 ` Joanne Koong
13 siblings, 1 reply; 19+ messages in thread
From: Amir Goldstein @ 2026-04-24 7:51 UTC (permalink / raw)
To: Joanne Koong; +Cc: miklos, fuse-devel
On Thu, Apr 23, 2026 at 06:16:33PM -0700, Joanne Koong wrote:
> Amir expressed his unhappiness [1] with some of the code structure in dir.c,
> particularly fuse_dentry_revalidate().
>
> This patchset is a series of cleanups in dir.c. There are no functional
> changes intended (except for the first patch).
I think this is not a good idea.
These sort of cleanup (e.g. removing unneeded gotos) are a kin to
whitespace cleanups - we should not do them unless going to change
the function for another reason.
The churn this creates and backporting conflicts are not worth the
win of the cleanup itself.
Therefore, please drop all but the fuse_dentry_revalidate() patch
which should be part of your passthrough series.
I will review only this one patch.
Thanks,
Amir.
>
> Thanks,
> Joanne
>
> [1] https://lore.kernel.org/fuse-devel/CAOQ4uxgCezMh3jdp519Lp9OZeqAyZHmc4Mp3FthJpfXhVoHEfw@mail.gmail.com/
>
> Joanne Koong (13):
> fuse: fix missing forget requests in error paths
> fuse: clean up fuse_lookup_name()
> fuse: clean up fuse_lookup()
> fuse: clean up fuse_dentry_revalidate()
> fuse: clean up fuse_create_open()
> fuse: clean up fuse_rename2()
> fuse: clean up fuse_time_to_jiffies()
> fuse: clean up fuse_link()
> fuse: clean up fuse_do_getattr()
> fuse: clean up fuse_update_get_attr()
> fuse: clean up fuse_get_link()
> fuse: clean up fuse_dir_open()
> fuse: clean up setattr()
>
> fs/fuse/dir.c | 324 ++++++++++++++++++++++++--------------------------
> 1 file changed, 154 insertions(+), 170 deletions(-)
>
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v1 00/13] fuse: dir.c cleanups
2026-04-24 7:51 ` [PATCH v1 00/13] fuse: dir.c cleanups Amir Goldstein
@ 2026-04-24 17:27 ` Joanne Koong
0 siblings, 0 replies; 19+ messages in thread
From: Joanne Koong @ 2026-04-24 17:27 UTC (permalink / raw)
To: Amir Goldstein; +Cc: miklos, fuse-devel
On Fri, Apr 24, 2026 at 12:51 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Thu, Apr 23, 2026 at 06:16:33PM -0700, Joanne Koong wrote:
> > Amir expressed his unhappiness [1] with some of the code structure in dir.c,
> > particularly fuse_dentry_revalidate().
> >
> > This patchset is a series of cleanups in dir.c. There are no functional
> > changes intended (except for the first patch).
>
> I think this is not a good idea.
> These sort of cleanup (e.g. removing unneeded gotos) are a kin to
> whitespace cleanups - we should not do them unless going to change
> the function for another reason.
>
> The churn this creates and backporting conflicts are not worth the
> win of the cleanup itself.
>
> Therefore, please drop all but the fuse_dentry_revalidate() patch
> which should be part of your passthrough series.
Ah, I hadn't considered the backporting conflicts. That makes sense -
I'll keep that in mind for the future.
I'll drop this series and include only the fuse_dentry_revalidate()
cleanup and have that as part of the passthrough series.
Thanks,
Joanne
>
> I will review only this one patch.
>
> Thanks,
> Amir.
>
> >
> > Thanks,
> > Joanne
> >
> > [1] https://lore.kernel.org/fuse-devel/CAOQ4uxgCezMh3jdp519Lp9OZeqAyZHmc4Mp3FthJpfXhVoHEfw@mail.gmail.com/
> >
> > Joanne Koong (13):
> > fuse: fix missing forget requests in error paths
> > fuse: clean up fuse_lookup_name()
> > fuse: clean up fuse_lookup()
> > fuse: clean up fuse_dentry_revalidate()
> > fuse: clean up fuse_create_open()
> > fuse: clean up fuse_rename2()
> > fuse: clean up fuse_time_to_jiffies()
> > fuse: clean up fuse_link()
> > fuse: clean up fuse_do_getattr()
> > fuse: clean up fuse_update_get_attr()
> > fuse: clean up fuse_get_link()
> > fuse: clean up fuse_dir_open()
> > fuse: clean up setattr()
> >
> > fs/fuse/dir.c | 324 ++++++++++++++++++++++++--------------------------
> > 1 file changed, 154 insertions(+), 170 deletions(-)
> >
> > --
> > 2.52.0
> >
^ permalink raw reply [flat|nested] 19+ messages in thread