From: Horst Birthelmer <hbirthelmer@ddn.com>

The discussion about compound commands in fuse was
started over an argument to add a new operation that
will open a file and return its attributes in the same operation.

Here is a demonstration of that use case with compound commands.

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
---
 fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
 fs/fuse/fuse_i.h |   4 +-
 fs/fuse/ioctl.c  |   2 +-
 3 files changed, 99 insertions(+), 18 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -136,8 +136,71 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
 	}
 }
 
+static int fuse_compound_open_getattr(struct fuse_mount *fm, u64 nodeid,
+				      struct inode *inode, int flags, int opcode,
+				      struct fuse_file *ff,
+				      struct fuse_attr_out *outattrp,
+				      struct fuse_open_out *outopenp)
+{
+	struct fuse_conn *fc = fm->fc;
+	struct fuse_compound_req *compound;
+	struct fuse_args open_args = {};
+	struct fuse_args getattr_args = {};
+	struct fuse_open_in open_in = {};
+	struct fuse_getattr_in getattr_in = {};
+	int err;
+
+	compound = fuse_compound_alloc(fm, 2, FUSE_COMPOUND_SEPARABLE);
+	if (!compound)
+		return -ENOMEM;
+
+	open_in.flags = flags & ~(O_CREAT | O_EXCL | O_NOCTTY);
+	if (!fm->fc->atomic_o_trunc)
+		open_in.flags &= ~O_TRUNC;
+
+	if (fm->fc->handle_killpriv_v2 &&
+	    (open_in.flags & O_TRUNC) && !capable(CAP_FSETID))
+		open_in.open_flags |= FUSE_OPEN_KILL_SUIDGID;
+
+	fuse_open_args_fill(&open_args, nodeid, opcode, &open_in, outopenp);
+
+	err = fuse_compound_add(compound, &open_args, NULL);
+	if (err)
+		goto out;
+
+	fuse_getattr_args_fill(&getattr_args, nodeid, &getattr_in, outattrp);
+
+	err = fuse_compound_add(compound, &getattr_args, NULL);
+	if (err)
+		goto out;
+
+	err = fuse_compound_send(compound);
+	if (err)
+		goto out;
+
+	err = fuse_compound_get_error(compound, 0);
+	if (err)
+		goto out;
+
+	ff->fh = outopenp->fh;
+	ff->open_flags = outopenp->open_flags;
+
+	err = fuse_compound_get_error(compound, 1);
+	if (err)
+		goto out;
+
+	fuse_change_attributes(inode, &outattrp->attr, NULL,
+			       ATTR_TIMEOUT(outattrp),
+			       fuse_get_attr_version(fc));
+
+out:
+	fuse_compound_free(compound);
+	return err;
+}
+
 struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
-				 unsigned int open_flags, bool isdir)
+				struct inode *inode,
+				unsigned int open_flags, bool isdir)
 {
 	struct fuse_conn *fc = fm->fc;
 	struct fuse_file *ff;
@@ -163,23 +226,40 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
 	if (open) {
 		/* Store outarg for fuse_finish_open() */
 		struct fuse_open_out *outargp = &ff->args->open_outarg;
-		int err;
+		int err = -ENOSYS;
 
-		err = fuse_send_open(fm, nodeid, open_flags, opcode, outargp);
-		if (!err) {
-			ff->fh = outargp->fh;
-			ff->open_flags = outargp->open_flags;
-		} else if (err != -ENOSYS) {
-			fuse_file_free(ff);
-			return ERR_PTR(err);
-		} else {
-			if (isdir) {
+		if (inode) {
+			struct fuse_attr_out attr_outarg;
+
+			err = fuse_compound_open_getattr(fm, nodeid, inode,
+							 open_flags, opcode, ff,
+							 &attr_outarg, outargp);
+		}
+
+		if (err == -ENOSYS) {
+			err = fuse_send_open(fm, nodeid, open_flags, opcode,
+					     outargp);
+			if (!err) {
+				ff->fh = outargp->fh;
+				ff->open_flags = outargp->open_flags;
+			}
+		}
+
+		if (err) {
+			if (err != -ENOSYS) {
+				/* err is not ENOSYS */
+				fuse_file_free(ff);
+				return ERR_PTR(err);
+			} else {
 				/* No release needed */
 				kfree(ff->args);
 				ff->args = NULL;
-				fc->no_opendir = 1;
-			} else {
-				fc->no_open = 1;
+
+				/* we don't have open */
+				if (isdir)
+					fc->no_opendir = 1;
+				else
+					fc->no_open = 1;
 			}
 		}
 	}
@@ -195,11 +275,10 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
 int fuse_do_open(struct fuse_mount *fm, u64 nodeid, struct file *file,
 		 bool isdir)
 {
-	struct fuse_file *ff = fuse_file_open(fm, nodeid, file->f_flags, isdir);
+	struct fuse_file *ff = fuse_file_open(fm, nodeid, file_inode(file), file->f_flags, isdir);
 
 	if (!IS_ERR(ff))
 		file->private_data = ff;
-
 	return PTR_ERR_OR_ZERO(ff);
 }
 EXPORT_SYMBOL_GPL(fuse_do_open);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index ff8222b66c4f7b04c0671a980237a43871affd0a..40409a4ab016a061eea20afee76c8a7fe9c15adb 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1588,7 +1588,9 @@ void fuse_file_io_release(struct fuse_file *ff, struct inode *inode);
 
 /* file.c */
 struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
-				 unsigned int open_flags, bool isdir);
+								struct inode *inode,
+								unsigned int open_flags,
+								bool isdir);
 void fuse_file_release(struct inode *inode, struct fuse_file *ff,
 		       unsigned int open_flags, fl_owner_t id, bool isdir);
 
diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
index fdc175e93f74743eb4d2e5a4bc688df1c62e64c4..07a02e47b2c3a68633d213675a8cc380a0cf31d8 100644
--- a/fs/fuse/ioctl.c
+++ b/fs/fuse/ioctl.c
@@ -494,7 +494,7 @@ static struct fuse_file *fuse_priv_ioctl_prepare(struct inode *inode)
 	if (!S_ISREG(inode->i_mode) && !isdir)
 		return ERR_PTR(-ENOTTY);
 
-	return fuse_file_open(fm, get_node_id(inode), O_RDONLY, isdir);
+	return fuse_file_open(fm, get_node_id(inode), NULL, O_RDONLY, isdir);
 }
 
 static void fuse_priv_ioctl_cleanup(struct inode *inode, struct fuse_file *ff)

-- 
2.53.0

On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>
> From: Horst Birthelmer <hbirthelmer@ddn.com>
>
> The discussion about compound commands in fuse was
> started over an argument to add a new operation that
> will open a file and return its attributes in the same operation.
>
> Here is a demonstration of that use case with compound commands.
>
> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> ---
>  fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
>  fs/fuse/fuse_i.h |   4 +-
>  fs/fuse/ioctl.c  |   2 +-
>  3 files changed, 99 insertions(+), 18 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -136,8 +136,71 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
>         }
>  }
>
> +static int fuse_compound_open_getattr(struct fuse_mount *fm, u64 nodeid,
> +                                     struct inode *inode, int flags, int opcode,
> +                                     struct fuse_file *ff,
> +                                     struct fuse_attr_out *outattrp,
> +                                     struct fuse_open_out *outopenp)
> +{
> +       struct fuse_conn *fc = fm->fc;
> +       struct fuse_compound_req *compound;
> +       struct fuse_args open_args = {};
> +       struct fuse_args getattr_args = {};
> +       struct fuse_open_in open_in = {};
> +       struct fuse_getattr_in getattr_in = {};
> +       int err;
> +
> +       compound = fuse_compound_alloc(fm, 2, FUSE_COMPOUND_SEPARABLE);
> +       if (!compound)
> +               return -ENOMEM;
> +
> +       open_in.flags = flags & ~(O_CREAT | O_EXCL | O_NOCTTY);
> +       if (!fm->fc->atomic_o_trunc)
> +               open_in.flags &= ~O_TRUNC;
> +
> +       if (fm->fc->handle_killpriv_v2 &&
> +           (open_in.flags & O_TRUNC) && !capable(CAP_FSETID))
> +               open_in.open_flags |= FUSE_OPEN_KILL_SUIDGID;

Do you think it makes sense to move this chunk of logic into
fuse_open_args_fill() since this logic has to be done in
fuse_send_open() as well?

> +
> +       fuse_open_args_fill(&open_args, nodeid, opcode, &open_in, outopenp);
> +
> +       err = fuse_compound_add(compound, &open_args, NULL);
> +       if (err)
> +               goto out;
> +
> +       fuse_getattr_args_fill(&getattr_args, nodeid, &getattr_in, outattrp);
> +
> +       err = fuse_compound_add(compound, &getattr_args, NULL);
> +       if (err)
> +               goto out;
> +
> +       err = fuse_compound_send(compound);
> +       if (err)
> +               goto out;
> +
> +       err = fuse_compound_get_error(compound, 0);
> +       if (err)
> +               goto out;
> +
> +       ff->fh = outopenp->fh;
> +       ff->open_flags = outopenp->open_flags;

It looks like this logic is shared between here and the non-compound
open path, maybe a bit better to just do this in fuse_file_open()
instead? That way we also don't need to pass the struct fuse_file *ff
as an arg either.

> +
> +       err = fuse_compound_get_error(compound, 1);
> +       if (err)
> +               goto out;

For this open+getattr case, if getattr fails but the open succeeds,
should this still succeed the open since they're separable requests? I
think we had a conversation about it in v4, but imo this case should.

> +
> +       fuse_change_attributes(inode, &outattrp->attr, NULL,
> +                              ATTR_TIMEOUT(outattrp),
> +                              fuse_get_attr_version(fc));
> +
> +out:
> +       fuse_compound_free(compound);
> +       return err;
> +}
> +
>  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> -                                unsigned int open_flags, bool isdir)
> +                               struct inode *inode,

As I understand it, now every open() is a opengetattr() (except for
the ioctl path) but is this the desired behavior? for example if there
was a previous FUSE_LOOKUP that was just done, doesn't this mean
there's no getattr that's needed since the lookup refreshed the attrs?
or if the server has reasonable entry_valid and attr_valid timeouts,
multiple opens() of the same file would only need to send FUSE_OPEN
and not the FUSE_GETATTR, no?


> +                               unsigned int open_flags, bool isdir)
>  {
>         struct fuse_conn *fc = fm->fc;
>         struct fuse_file *ff;
> @@ -163,23 +226,40 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
>         if (open) {
>                 /* Store outarg for fuse_finish_open() */
>                 struct fuse_open_out *outargp = &ff->args->open_outarg;
> -               int err;
> +               int err = -ENOSYS;
>
> -               err = fuse_send_open(fm, nodeid, open_flags, opcode, outargp);
> -               if (!err) {
> -                       ff->fh = outargp->fh;
> -                       ff->open_flags = outargp->open_flags;
> -               } else if (err != -ENOSYS) {
> -                       fuse_file_free(ff);
> -                       return ERR_PTR(err);
> -               } else {
> -                       if (isdir) {
> +               if (inode) {
> +                       struct fuse_attr_out attr_outarg;
> +
> +                       err = fuse_compound_open_getattr(fm, nodeid, inode,
> +                                                        open_flags, opcode, ff,
> +                                                        &attr_outarg, outargp);

instead of passing in &attr_outarg, what about just having that moved
to fuse_compound_open_getattr()?

> +               }
> +
> +               if (err == -ENOSYS) {
> +                       err = fuse_send_open(fm, nodeid, open_flags, opcode,
> +                                            outargp);
> +                       if (!err) {
> +                               ff->fh = outargp->fh;
> +                               ff->open_flags = outargp->open_flags;
> +                       }
> +               }
> +
> +               if (err) {
> +                       if (err != -ENOSYS) {
> +                               /* err is not ENOSYS */
> +                               fuse_file_free(ff);
> +                               return ERR_PTR(err);
> +                       } else {
>                                 /* No release needed */
>                                 kfree(ff->args);
>                                 ff->args = NULL;
> -                               fc->no_opendir = 1;
> -                       } else {
> -                               fc->no_open = 1;
> +
> +                               /* we don't have open */
> +                               if (isdir)
> +                                       fc->no_opendir = 1;
> +                               else
> +                                       fc->no_open = 1;

kfree(ff->args) and ff->args = NULL should not be called for the
!isdir case or it leads to the deadlock that was fixed in
https://lore.kernel.org/linux-fsdevel/20251010220738.3674538-2-joannelkoong@gmail.com/

I think if you have the "ff->fh = outargp..." and "ff->open_flags =
..." logic shared between fuse_compound_open_getattr() and
fuse_send_open() then the original errorr handling for this could just
be left as-is.

Thanks,
Joanne

>                         }
>                 }
>         }
> @@ -195,11 +275,10 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
>  int fuse_do_open(struct fuse_mount *fm, u64 nodeid, struct file *file,
>                  bool isdir)
>  {
> -       struct fuse_file *ff = fuse_file_open(fm, nodeid, file->f_flags, isdir);
> +       struct fuse_file *ff = fuse_file_open(fm, nodeid, file_inode(file), file->f_flags, isdir);
>
>         if (!IS_ERR(ff))
>                 file->private_data = ff;
> -
>         return PTR_ERR_OR_ZERO(ff);
>  }
>  EXPORT_SYMBOL_GPL(fuse_do_open);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index ff8222b66c4f7b04c0671a980237a43871affd0a..40409a4ab016a061eea20afee76c8a7fe9c15adb 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1588,7 +1588,9 @@ void fuse_file_io_release(struct fuse_file *ff, struct inode *inode);
>
>  /* file.c */
>  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> -                                unsigned int open_flags, bool isdir);
> +                                                               struct inode *inode,
> +                                                               unsigned int open_flags,
> +                                                               bool isdir);
>  void fuse_file_release(struct inode *inode, struct fuse_file *ff,
>                        unsigned int open_flags, fl_owner_t id, bool isdir);
>
> diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
> index fdc175e93f74743eb4d2e5a4bc688df1c62e64c4..07a02e47b2c3a68633d213675a8cc380a0cf31d8 100644
> --- a/fs/fuse/ioctl.c
> +++ b/fs/fuse/ioctl.c
> @@ -494,7 +494,7 @@ static struct fuse_file *fuse_priv_ioctl_prepare(struct inode *inode)
>         if (!S_ISREG(inode->i_mode) && !isdir)
>                 return ERR_PTR(-ENOTTY);
>
> -       return fuse_file_open(fm, get_node_id(inode), O_RDONLY, isdir);
> +       return fuse_file_open(fm, get_node_id(inode), NULL, O_RDONLY, isdir);
>  }
>
>  static void fuse_priv_ioctl_cleanup(struct inode *inode, struct fuse_file *ff)
>
> --
> 2.53.0
>

On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >
> > From: Horst Birthelmer <hbirthelmer@ddn.com>
> >
> > The discussion about compound commands in fuse was
> > started over an argument to add a new operation that
> > will open a file and return its attributes in the same operation.
> >
> > Here is a demonstration of that use case with compound commands.
> >
> > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > ---
> >  fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> >  fs/fuse/fuse_i.h |   4 +-
> >  fs/fuse/ioctl.c  |   2 +-
> >  3 files changed, 99 insertions(+), 18 deletions(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -136,8 +136,71 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
> >         }
> >  }
> >
> > +static int fuse_compound_open_getattr(struct fuse_mount *fm, u64 nodeid,
> > +                                     struct inode *inode, int flags, int opcode,
> > +                                     struct fuse_file *ff,
> > +                                     struct fuse_attr_out *outattrp,
> > +                                     struct fuse_open_out *outopenp)
> > +{
> > +       struct fuse_conn *fc = fm->fc;
> > +       struct fuse_compound_req *compound;
> > +       struct fuse_args open_args = {};
> > +       struct fuse_args getattr_args = {};
> > +       struct fuse_open_in open_in = {};
> > +       struct fuse_getattr_in getattr_in = {};
> > +       int err;
> > +
> > +       compound = fuse_compound_alloc(fm, 2, FUSE_COMPOUND_SEPARABLE);
> > +       if (!compound)
> > +               return -ENOMEM;
> > +
> > +       open_in.flags = flags & ~(O_CREAT | O_EXCL | O_NOCTTY);
> > +       if (!fm->fc->atomic_o_trunc)
> > +               open_in.flags &= ~O_TRUNC;
> > +
> > +       if (fm->fc->handle_killpriv_v2 &&
> > +           (open_in.flags & O_TRUNC) && !capable(CAP_FSETID))
> > +               open_in.open_flags |= FUSE_OPEN_KILL_SUIDGID;
> 
> Do you think it makes sense to move this chunk of logic into
> fuse_open_args_fill() since this logic has to be done in
> fuse_send_open() as well?
>

Yes, I think that makes sense and would be beneficial to other requests in 
other compounds that will be constructed with that function.

> > +
> > +       fuse_open_args_fill(&open_args, nodeid, opcode, &open_in, outopenp);
> > +
> > +       err = fuse_compound_add(compound, &open_args, NULL);
> > +       if (err)
> > +               goto out;
> > +
> > +       fuse_getattr_args_fill(&getattr_args, nodeid, &getattr_in, outattrp);
> > +
> > +       err = fuse_compound_add(compound, &getattr_args, NULL);
> > +       if (err)
> > +               goto out;
> > +
> > +       err = fuse_compound_send(compound);
> > +       if (err)
> > +               goto out;
> > +
> > +       err = fuse_compound_get_error(compound, 0);
> > +       if (err)
> > +               goto out;
> > +
> > +       ff->fh = outopenp->fh;
> > +       ff->open_flags = outopenp->open_flags;
> 
> It looks like this logic is shared between here and the non-compound
> open path, maybe a bit better to just do this in fuse_file_open()
> instead? That way we also don't need to pass the struct fuse_file *ff
> as an arg either.
> 

Will do that.

> > +
> > +       err = fuse_compound_get_error(compound, 1);
> > +       if (err)
> > +               goto out;
> 
> For this open+getattr case, if getattr fails but the open succeeds,
> should this still succeed the open since they're separable requests? I
> think we had a conversation about it in v4, but imo this case should.
> 
You are right, we had the conversation and other people joined, so I
changed this code but to something else. Sorry about that.

I think your idea will work, since the behavior then is exactly what happens
at the moment with exactly the same drawback.

> > +
> > +       fuse_change_attributes(inode, &outattrp->attr, NULL,
> > +                              ATTR_TIMEOUT(outattrp),
> > +                              fuse_get_attr_version(fc));
> > +
> > +out:
> > +       fuse_compound_free(compound);
> > +       return err;
> > +}
> > +
> >  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> > -                                unsigned int open_flags, bool isdir)
> > +                               struct inode *inode,
> 
> As I understand it, now every open() is a opengetattr() (except for
> the ioctl path) but is this the desired behavior? for example if there
> was a previous FUSE_LOOKUP that was just done, doesn't this mean
> there's no getattr that's needed since the lookup refreshed the attrs?
> or if the server has reasonable entry_valid and attr_valid timeouts,
> multiple opens() of the same file would only need to send FUSE_OPEN
> and not the FUSE_GETATTR, no?

So your concern is, that we send too many requests?
If the fuse server implwments the compound that is not the case.

> 
> 
> > +                               unsigned int open_flags, bool isdir)
> >  {
> >         struct fuse_conn *fc = fm->fc;
> >         struct fuse_file *ff;
> > @@ -163,23 +226,40 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> >         if (open) {
> >                 /* Store outarg for fuse_finish_open() */
> >                 struct fuse_open_out *outargp = &ff->args->open_outarg;
> > -               int err;
> > +               int err = -ENOSYS;
> >
> > -               err = fuse_send_open(fm, nodeid, open_flags, opcode, outargp);
> > -               if (!err) {
> > -                       ff->fh = outargp->fh;
> > -                       ff->open_flags = outargp->open_flags;
> > -               } else if (err != -ENOSYS) {
> > -                       fuse_file_free(ff);
> > -                       return ERR_PTR(err);
> > -               } else {
> > -                       if (isdir) {
> > +               if (inode) {
> > +                       struct fuse_attr_out attr_outarg;
> > +
> > +                       err = fuse_compound_open_getattr(fm, nodeid, inode,
> > +                                                        open_flags, opcode, ff,
> > +                                                        &attr_outarg, outargp);
> 
> instead of passing in &attr_outarg, what about just having that moved
> to fuse_compound_open_getattr()?
> 

This is a victim of 'code move' already.
I had the code to handle the outarg here before and did not change the functions
signature, which now looks stupid.

> > +               }
> > +
> > +               if (err == -ENOSYS) {
> > +                       err = fuse_send_open(fm, nodeid, open_flags, opcode,
> > +                                            outargp);
> > +                       if (!err) {
> > +                               ff->fh = outargp->fh;
> > +                               ff->open_flags = outargp->open_flags;
> > +                       }
> > +               }
> > +
> > +               if (err) {
> > +                       if (err != -ENOSYS) {
> > +                               /* err is not ENOSYS */
> > +                               fuse_file_free(ff);
> > +                               return ERR_PTR(err);
> > +                       } else {
> >                                 /* No release needed */
> >                                 kfree(ff->args);
> >                                 ff->args = NULL;
> > -                               fc->no_opendir = 1;
> > -                       } else {
> > -                               fc->no_open = 1;
> > +
> > +                               /* we don't have open */
> > +                               if (isdir)
> > +                                       fc->no_opendir = 1;
> > +                               else
> > +                                       fc->no_open = 1;
> 
> kfree(ff->args) and ff->args = NULL should not be called for the
> !isdir case or it leads to the deadlock that was fixed in
> https://lore.kernel.org/linux-fsdevel/20251010220738.3674538-2-joannelkoong@gmail.com/
> 
> I think if you have the "ff->fh = outargp..." and "ff->open_flags =
> ..." logic shared between fuse_compound_open_getattr() and
> fuse_send_open() then the original errorr handling for this could just
> be left as-is.

Very good argument to share the error handling then ...

> 
> Thanks,
> Joanne
> 

Thanks for taking the time,
Horst

On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
>
> On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> > >
> > > From: Horst Birthelmer <hbirthelmer@ddn.com>
> > >
> > > The discussion about compound commands in fuse was
> > > started over an argument to add a new operation that
> > > will open a file and return its attributes in the same operation.
> > >
> > > Here is a demonstration of that use case with compound commands.
> > >
> > > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > > ---
> > >  fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > >  fs/fuse/fuse_i.h |   4 +-
> > >  fs/fuse/ioctl.c  |   2 +-
> > >  3 files changed, 99 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > > --- a/fs/fuse/file.c
> > > +++ b/fs/fuse/file.c
> > >  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> > > -                                unsigned int open_flags, bool isdir)
> > > +                               struct inode *inode,
> >
> > As I understand it, now every open() is a opengetattr() (except for
> > the ioctl path) but is this the desired behavior? for example if there
> > was a previous FUSE_LOOKUP that was just done, doesn't this mean
> > there's no getattr that's needed since the lookup refreshed the attrs?
> > or if the server has reasonable entry_valid and attr_valid timeouts,
> > multiple opens() of the same file would only need to send FUSE_OPEN
> > and not the FUSE_GETATTR, no?
>
> So your concern is, that we send too many requests?
> If the fuse server implwments the compound that is not the case.
>

My concern is that we're adding unnecessary overhead for every open
when in most cases, the attributes are already uptodate. I don't think
we can assume that the server always has attributes locally cached, so
imo the extra getattr is nontrivial (eg might require having to
stat()).

Thanks,
Joanne

On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
> >
> > On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > > On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> > > >
> > > > From: Horst Birthelmer <hbirthelmer@ddn.com>
> > > >
> > > > The discussion about compound commands in fuse was
> > > > started over an argument to add a new operation that
> > > > will open a file and return its attributes in the same operation.
> > > >
> > > > Here is a demonstration of that use case with compound commands.
> > > >
> > > > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > > > ---
> > > >  fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > > >  fs/fuse/fuse_i.h |   4 +-
> > > >  fs/fuse/ioctl.c  |   2 +-
> > > >  3 files changed, 99 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > > > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > > > --- a/fs/fuse/file.c
> > > > +++ b/fs/fuse/file.c
> > > >  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> > > > -                                unsigned int open_flags, bool isdir)
> > > > +                               struct inode *inode,
> > >
> > > As I understand it, now every open() is a opengetattr() (except for
> > > the ioctl path) but is this the desired behavior? for example if there
> > > was a previous FUSE_LOOKUP that was just done, doesn't this mean
> > > there's no getattr that's needed since the lookup refreshed the attrs?
> > > or if the server has reasonable entry_valid and attr_valid timeouts,
> > > multiple opens() of the same file would only need to send FUSE_OPEN
> > > and not the FUSE_GETATTR, no?
> >
> > So your concern is, that we send too many requests?
> > If the fuse server implwments the compound that is not the case.
> >
>
> My concern is that we're adding unnecessary overhead for every open
> when in most cases, the attributes are already uptodate. I don't think
> we can assume that the server always has attributes locally cached, so
> imo the extra getattr is nontrivial (eg might require having to
> stat()).

Looking at where the attribute valid time gets set... it looks like
this gets stored in fi->i_time (as per
fuse_change_attributes_common()), so maybe it's better to only send
the compound open+getattr if time_before64(fi->i_time,
get_jiffies_64()) is true, otherwise only the open is needed. This
doesn't solve the O_APPEND data corruption bug seen in [1] but imo
this would be a more preferable way of doing it.

Thanks,
Joanne

[1] https://lore.kernel.org/linux-fsdevel/20240813212149.1909627-1-joannelkoong@gmail.com/

>
> Thanks,
> Joanne

On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
> On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
> >
> > On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
> > >
> > > On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > > > On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> > > > >
> > > > > From: Horst Birthelmer <hbirthelmer@ddn.com>
> > > > >
> > > > > The discussion about compound commands in fuse was
> > > > > started over an argument to add a new operation that
> > > > > will open a file and return its attributes in the same operation.
> > > > >
> > > > > Here is a demonstration of that use case with compound commands.
> > > > >
> > > > > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > > > > ---
> > > > >  fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > > > >  fs/fuse/fuse_i.h |   4 +-
> > > > >  fs/fuse/ioctl.c  |   2 +-
> > > > >  3 files changed, 99 insertions(+), 18 deletions(-)
> > > > >
> > > > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > > > > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > > > > --- a/fs/fuse/file.c
> > > > > +++ b/fs/fuse/file.c
> > > > >  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> > > > > -                                unsigned int open_flags, bool isdir)
> > > > > +                               struct inode *inode,
> > > >
> > > > As I understand it, now every open() is a opengetattr() (except for
> > > > the ioctl path) but is this the desired behavior? for example if there
> > > > was a previous FUSE_LOOKUP that was just done, doesn't this mean
> > > > there's no getattr that's needed since the lookup refreshed the attrs?
> > > > or if the server has reasonable entry_valid and attr_valid timeouts,
> > > > multiple opens() of the same file would only need to send FUSE_OPEN
> > > > and not the FUSE_GETATTR, no?
> > >
> > > So your concern is, that we send too many requests?
> > > If the fuse server implwments the compound that is not the case.
> > >
> >
> > My concern is that we're adding unnecessary overhead for every open
> > when in most cases, the attributes are already uptodate. I don't think
> > we can assume that the server always has attributes locally cached, so
> > imo the extra getattr is nontrivial (eg might require having to
> > stat()).
> 
> Looking at where the attribute valid time gets set... it looks like
> this gets stored in fi->i_time (as per
> fuse_change_attributes_common()), so maybe it's better to only send
> the compound open+getattr if time_before64(fi->i_time,
> get_jiffies_64()) is true, otherwise only the open is needed. This
> doesn't solve the O_APPEND data corruption bug seen in [1] but imo
> this would be a more preferable way of doing it.
> 
Don't take this as an objection. I'm looking for arguments, since my defense
was always the line I used above (if the fuse server implements the compound,
it's one call).

What made you change your mind from avoiding the data corruption to worry 
about the number of stat calls done? Since you were the one reporting the
issue and even providing a fix.

ATM I would rather have data consistency than fewer requests during open.

> Thanks,
> Joanne
> 
> [1] https://lore.kernel.org/linux-fsdevel/20240813212149.1909627-1-joannelkoong@gmail.com/
> 
> >
> > Thanks,
> > Joanne
> 

On Sat, Feb 28, 2026 at 12:14 AM Horst Birthelmer <horst@birthelmer.de> wrote:
>
> On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
> > On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
> > >
> > > On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
> > > >
> > > > On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > > > > On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> > > > > >
> > > > > > From: Horst Birthelmer <hbirthelmer@ddn.com>
> > > > > >
> > > > > > The discussion about compound commands in fuse was
> > > > > > started over an argument to add a new operation that
> > > > > > will open a file and return its attributes in the same operation.
> > > > > >
> > > > > > Here is a demonstration of that use case with compound commands.
> > > > > >
> > > > > > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > > > > > ---
> > > > > >  fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > > > > >  fs/fuse/fuse_i.h |   4 +-
> > > > > >  fs/fuse/ioctl.c  |   2 +-
> > > > > >  3 files changed, 99 insertions(+), 18 deletions(-)
> > > > > >
> > > > > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > > > > > index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> > > > > > --- a/fs/fuse/file.c
> > > > > > +++ b/fs/fuse/file.c
> > > > > >  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> > > > > > -                                unsigned int open_flags, bool isdir)
> > > > > > +                               struct inode *inode,
> > > > >
> > > > > As I understand it, now every open() is a opengetattr() (except for
> > > > > the ioctl path) but is this the desired behavior? for example if there
> > > > > was a previous FUSE_LOOKUP that was just done, doesn't this mean
> > > > > there's no getattr that's needed since the lookup refreshed the attrs?
> > > > > or if the server has reasonable entry_valid and attr_valid timeouts,
> > > > > multiple opens() of the same file would only need to send FUSE_OPEN
> > > > > and not the FUSE_GETATTR, no?
> > > >
> > > > So your concern is, that we send too many requests?
> > > > If the fuse server implwments the compound that is not the case.
> > > >
> > >
> > > My concern is that we're adding unnecessary overhead for every open
> > > when in most cases, the attributes are already uptodate. I don't think
> > > we can assume that the server always has attributes locally cached, so
> > > imo the extra getattr is nontrivial (eg might require having to
> > > stat()).
> >
> > Looking at where the attribute valid time gets set... it looks like
> > this gets stored in fi->i_time (as per
> > fuse_change_attributes_common()), so maybe it's better to only send
> > the compound open+getattr if time_before64(fi->i_time,
> > get_jiffies_64()) is true, otherwise only the open is needed. This
> > doesn't solve the O_APPEND data corruption bug seen in [1] but imo
> > this would be a more preferable way of doing it.
> >
> Don't take this as an objection. I'm looking for arguments, since my defense
> was always the line I used above (if the fuse server implements the compound,
> it's one call).

The overhead for the server to fetch the attributes may be nontrivial
(eg may require stat()). I really don't think we can assume the data
is locally cached somewhere. Why always compound the getattr to the
open instead of only compounding the getattr when the attributes are
actually invalid?

But maybe I'm wrong here and this is the preferable way of doing it.
Miklos, could you provide your input on this?

>
> What made you change your mind from avoiding the data corruption to worry
> about the number of stat calls done? Since you were the one reporting the
> issue and even providing a fix.

imo I think the O_APPEND fix should be something like:

if ((open_flags & O_APPEND) || time_before64(fi->i_time, get_jiffies_64()))
   send FUSE_OPEN + FUSE_GETATTR compound
else
   send FUSE_OPEN only

since the O_APPEND case specifically needs accurate file size for the
append offset.

Thanks,
Joanne

>
> ATM I would rather have data consistency than fewer requests during open.
>
> > Thanks,
> > Joanne
> >
> > [1] https://lore.kernel.org/linux-fsdevel/20240813212149.1909627-1-joannelkoong@gmail.com/
> >
> > >
> > > Thanks,
> > > Joanne
> >

On 3/2/26 19:56, Joanne Koong wrote:
> On Sat, Feb 28, 2026 at 12:14 AM Horst Birthelmer <horst@birthelmer.de> wrote:
>>
>> On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
>>> On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
>>>>
>>>> On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
>>>>>
>>>>> On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
>>>>>> On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>>>>>>>
>>>>>>> From: Horst Birthelmer <hbirthelmer@ddn.com>
>>>>>>>
>>>>>>> The discussion about compound commands in fuse was
>>>>>>> started over an argument to add a new operation that
>>>>>>> will open a file and return its attributes in the same operation.
>>>>>>>
>>>>>>> Here is a demonstration of that use case with compound commands.
>>>>>>>
>>>>>>> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
>>>>>>> ---
>>>>>>>  fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
>>>>>>>  fs/fuse/fuse_i.h |   4 +-
>>>>>>>  fs/fuse/ioctl.c  |   2 +-
>>>>>>>  3 files changed, 99 insertions(+), 18 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>>>>>>> index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
>>>>>>> --- a/fs/fuse/file.c
>>>>>>> +++ b/fs/fuse/file.c
>>>>>>>  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
>>>>>>> -                                unsigned int open_flags, bool isdir)
>>>>>>> +                               struct inode *inode,
>>>>>>
>>>>>> As I understand it, now every open() is a opengetattr() (except for
>>>>>> the ioctl path) but is this the desired behavior? for example if there
>>>>>> was a previous FUSE_LOOKUP that was just done, doesn't this mean
>>>>>> there's no getattr that's needed since the lookup refreshed the attrs?
>>>>>> or if the server has reasonable entry_valid and attr_valid timeouts,
>>>>>> multiple opens() of the same file would only need to send FUSE_OPEN
>>>>>> and not the FUSE_GETATTR, no?
>>>>>
>>>>> So your concern is, that we send too many requests?
>>>>> If the fuse server implwments the compound that is not the case.
>>>>>
>>>>
>>>> My concern is that we're adding unnecessary overhead for every open
>>>> when in most cases, the attributes are already uptodate. I don't think
>>>> we can assume that the server always has attributes locally cached, so
>>>> imo the extra getattr is nontrivial (eg might require having to
>>>> stat()).
>>>
>>> Looking at where the attribute valid time gets set... it looks like
>>> this gets stored in fi->i_time (as per
>>> fuse_change_attributes_common()), so maybe it's better to only send
>>> the compound open+getattr if time_before64(fi->i_time,
>>> get_jiffies_64()) is true, otherwise only the open is needed. This
>>> doesn't solve the O_APPEND data corruption bug seen in [1] but imo
>>> this would be a more preferable way of doing it.
>>>
>> Don't take this as an objection. I'm looking for arguments, since my defense
>> was always the line I used above (if the fuse server implements the compound,
>> it's one call).
> 
> The overhead for the server to fetch the attributes may be nontrivial
> (eg may require stat()). I really don't think we can assume the data
> is locally cached somewhere. Why always compound the getattr to the
> open instead of only compounding the getattr when the attributes are
> actually invalid?
> 
> But maybe I'm wrong here and this is the preferable way of doing it.
> Miklos, could you provide your input on this?

Personally I would see it as change of behavior if out of the sudden
open is followed by getattr. In my opinion fuse server needs to make a
decision that it wants that. Let's take my favorite sshfs example with a
1s latency - it be very noticeable if open would get slowed down by
factor 2.

Thanks,
Bernd

On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> 
> 
> On 3/2/26 19:56, Joanne Koong wrote:
> > On Sat, Feb 28, 2026 at 12:14 AM Horst Birthelmer <horst@birthelmer.de> wrote:
> >>
> >> On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
> >>> On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
> >>>>
> >>>> On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
> >>>>>
> >>>>> On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> >>>>>> On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >>>>>>>
> >>>>>>> From: Horst Birthelmer <hbirthelmer@ddn.com>
> >>>>>>>
> >>>>>>> The discussion about compound commands in fuse was
> >>>>>>> started over an argument to add a new operation that
> >>>>>>> will open a file and return its attributes in the same operation.
> >>>>>>>
> >>>>>>> Here is a demonstration of that use case with compound commands.
> >>>>>>>
> >>>>>>> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> >>>>>>> ---
> >>>>>>>  fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> >>>>>>>  fs/fuse/fuse_i.h |   4 +-
> >>>>>>>  fs/fuse/ioctl.c  |   2 +-
> >>>>>>>  3 files changed, 99 insertions(+), 18 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> >>>>>>> index a408a9668abbb361e2c1e386ebab9dfcb0a7a573..daa95a640c311fc393241bdf727e00a2bc714f35 100644
> >>>>>>> --- a/fs/fuse/file.c
> >>>>>>> +++ b/fs/fuse/file.c
> >>>>>>>  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> >>>>>>> -                                unsigned int open_flags, bool isdir)
> >>>>>>> +                               struct inode *inode,
> >>>>>>
> >>>>>> As I understand it, now every open() is a opengetattr() (except for
> >>>>>> the ioctl path) but is this the desired behavior? for example if there
> >>>>>> was a previous FUSE_LOOKUP that was just done, doesn't this mean
> >>>>>> there's no getattr that's needed since the lookup refreshed the attrs?
> >>>>>> or if the server has reasonable entry_valid and attr_valid timeouts,
> >>>>>> multiple opens() of the same file would only need to send FUSE_OPEN
> >>>>>> and not the FUSE_GETATTR, no?
> >>>>>
> >>>>> So your concern is, that we send too many requests?
> >>>>> If the fuse server implwments the compound that is not the case.
> >>>>>
> >>>>
> >>>> My concern is that we're adding unnecessary overhead for every open
> >>>> when in most cases, the attributes are already uptodate. I don't think
> >>>> we can assume that the server always has attributes locally cached, so
> >>>> imo the extra getattr is nontrivial (eg might require having to
> >>>> stat()).
> >>>
> >>> Looking at where the attribute valid time gets set... it looks like
> >>> this gets stored in fi->i_time (as per
> >>> fuse_change_attributes_common()), so maybe it's better to only send
> >>> the compound open+getattr if time_before64(fi->i_time,
> >>> get_jiffies_64()) is true, otherwise only the open is needed. This
> >>> doesn't solve the O_APPEND data corruption bug seen in [1] but imo
> >>> this would be a more preferable way of doing it.

/me notes that NFS can corrupt O_APPEND writes if you're not careful to
synchronize writers at the application level...

> >> Don't take this as an objection. I'm looking for arguments, since my defense
> >> was always the line I used above (if the fuse server implements the compound,
> >> it's one call).
> > 
> > The overhead for the server to fetch the attributes may be nontrivial
> > (eg may require stat()). I really don't think we can assume the data
> > is locally cached somewhere. Why always compound the getattr to the
> > open instead of only compounding the getattr when the attributes are
> > actually invalid?
> > 
> > But maybe I'm wrong here and this is the preferable way of doing it.
> > Miklos, could you provide your input on this?
> 
> Personally I would see it as change of behavior if out of the sudden
> open is followed by getattr. In my opinion fuse server needs to make a
> decision that it wants that. Let's take my favorite sshfs example with a
> 1s latency - it be very noticeable if open would get slowed down by
> factor 2.

I wonder, since O_APPEND writes supposedly reposition the file position
to i_size before every write, can we enlarge the write reply so that the
fuse server could tell the client what i_size is supposed to be after
every write?  Or perhaps add a notification so a network filesystem
could try to keep the kernel uptodate after another node appends to a
file?

Just my unqualified opinion ;)

--D

> Thanks,
> Bernd
> 
> 
> 

On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> >
> > On 3/2/26 19:56, Joanne Koong wrote:

> > > The overhead for the server to fetch the attributes may be nontrivial
> > > (eg may require stat()). I really don't think we can assume the data
> > > is locally cached somewhere. Why always compound the getattr to the
> > > open instead of only compounding the getattr when the attributes are
> > > actually invalid?
> > >
> > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > Miklos, could you provide your input on this?

Yes, it makes sense to refresh attributes only when necessary.

> I wonder, since O_APPEND writes supposedly reposition the file position
> to i_size before every write, can we enlarge the write reply so that the
> fuse server could tell the client what i_size is supposed to be after
> every write?  Or perhaps add a notification so a network filesystem
> could try to keep the kernel uptodate after another node appends to a
> file?

This can be done with FUSE_NOTIFY_INVAL_INODE.

Still racy.  If need to have perfect O_APPEND semantics,
FOPEN_DIRECT_IO is currently the only option.

It would be nice to have some sort of delegation/lease mechanism in
the fuse protocol to allow caching when remote is not modifying (which
is the common case usually) but force uncached I/O when there's
concurrent modification.

Thanks,
Miklos

On Tue, Mar 3, 2026 at 2:03 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > >
> > > On 3/2/26 19:56, Joanne Koong wrote:
>
> > > > The overhead for the server to fetch the attributes may be nontrivial
> > > > (eg may require stat()). I really don't think we can assume the data
> > > > is locally cached somewhere. Why always compound the getattr to the
> > > > open instead of only compounding the getattr when the attributes are
> > > > actually invalid?
> > > >
> > > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > > Miklos, could you provide your input on this?
>
> Yes, it makes sense to refresh attributes only when necessary.
>
> > I wonder, since O_APPEND writes supposedly reposition the file position
> > to i_size before every write, can we enlarge the write reply so that the
> > fuse server could tell the client what i_size is supposed to be after
> > every write?  Or perhaps add a notification so a network filesystem
> > could try to keep the kernel uptodate after another node appends to a
> > file?
>
> This can be done with FUSE_NOTIFY_INVAL_INODE.
>
> Still racy.  If need to have perfect O_APPEND semantics,
> FOPEN_DIRECT_IO is currently the only option.

I think FOPEN_DIRECT_IO also uses the stale file size. eg
write_iter()
   fuse_file_write_iter()
      fuse_direct_write_iter()
         generic_write_checks()
            generic_write_checks_count()

which does

        if (iocb->ki_flags & IOCB_APPEND)
                iocb->ki_pos = i_size_read(inode);

and uses the locally cached (and stale) i_size.

I think right now the only option is for the fuse server to just
handle append semantics itself if it detects the O_APPEND flag in the
write request and just ignore the kernel-provided offset (assuming the
distributed backend synchronizes access amongst multiple clients). Or
return -ESTALE to open() if it detects the file size is stale, which
will trigger a lookup to fetch the latest attributes.

Thanks,
Joanne

>
> It would be nice to have some sort of delegation/lease mechanism in
> the fuse protocol to allow caching when remote is not modifying (which
> is the common case usually) but force uncached I/O when there's
> concurrent modification.
>
> Thanks,
> Miklos

On Wed, 4 Mar 2026 at 00:13, Joanne Koong <joannelkoong@gmail.com> wrote:

> I think right now the only option is for the fuse server to just
> handle append semantics itself if it detects the O_APPEND flag in the
> write request and just ignore the kernel-provided offset (assuming the
> distributed backend synchronizes access amongst multiple clients).

Right.  It can also send a NOTIFY_INVAL_INODE in case it detects that
the offset was wrong.  This will fix the file size, but not the file
position.

fuse_write_out could be extended with a file offset, which would fix
both issues.

Thanks,
Miklos

On Tue, Mar 03, 2026 at 11:03:14AM +0100, Miklos Szeredi wrote:
> On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > >
> > > On 3/2/26 19:56, Joanne Koong wrote:
> 
> > > > The overhead for the server to fetch the attributes may be nontrivial
> > > > (eg may require stat()). I really don't think we can assume the data
> > > > is locally cached somewhere. Why always compound the getattr to the
> > > > open instead of only compounding the getattr when the attributes are
> > > > actually invalid?
> > > >
> > > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > > Miklos, could you provide your input on this?
> 
> Yes, it makes sense to refresh attributes only when necessary.
> 

OK, I will add a 'compound flag' for optional operations and don't
execute those, when the fuse server does not support compounds.

This way we can always send the open+getattr, but if the fuse server
does not process this as a compound (aka. it would be costly to do it), 
we only resend the FUSE_OPEN. Thus the current behavior does not change
and we can profit from fuse servers that actually have the possibility to
give us the attributes.

We can take a look at when to fetch the attributes in another context for
the other cases.

> 
> Thanks,
> Miklos
> 

Thanks,
Horst

On Tue, Mar 3, 2026 at 2:39 AM Horst Birthelmer <horst@birthelmer.de> wrote:
>
> On Tue, Mar 03, 2026 at 11:03:14AM +0100, Miklos Szeredi wrote:
> > On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > > >
> > > > On 3/2/26 19:56, Joanne Koong wrote:
> >
> > > > > The overhead for the server to fetch the attributes may be nontrivial
> > > > > (eg may require stat()). I really don't think we can assume the data
> > > > > is locally cached somewhere. Why always compound the getattr to the
> > > > > open instead of only compounding the getattr when the attributes are
> > > > > actually invalid?
> > > > >
> > > > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > > > Miklos, could you provide your input on this?
> >
> > Yes, it makes sense to refresh attributes only when necessary.
> >
>
> OK, I will add a 'compound flag' for optional operations and don't
> execute those, when the fuse server does not support compounds.
>
> This way we can always send the open+getattr, but if the fuse server
> does not process this as a compound (aka. it would be costly to do it),
> we only resend the FUSE_OPEN. Thus the current behavior does not change
> and we can profit from fuse servers that actually have the possibility to
> give us the attributes.

in my opinion, this adds additional complexity for no real benefit.  I
think we'll rarely hit a case where it'll be useful to speculatively
prefetch attributes that are already valid that is not already
accounted for by the attr_timeout the server set.

Thanks,
Joanne

>
> We can take a look at when to fetch the attributes in another context for
> the other cases.
>
> >
> > Thanks,
> > Miklos
> >
>
> Thanks,
> Horst

On Tue, Mar 03, 2026 at 01:19:43PM -0800, Joanne Koong wrote:
> On Tue, Mar 3, 2026 at 2:39 AM Horst Birthelmer <horst@birthelmer.de> wrote:
> >
> > On Tue, Mar 03, 2026 at 11:03:14AM +0100, Miklos Szeredi wrote:
> > > On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
> > > >
> > > > On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > > > >
> > > > > On 3/2/26 19:56, Joanne Koong wrote:
> > >
> > > > > > The overhead for the server to fetch the attributes may be nontrivial
> > > > > > (eg may require stat()). I really don't think we can assume the data
> > > > > > is locally cached somewhere. Why always compound the getattr to the
> > > > > > open instead of only compounding the getattr when the attributes are
> > > > > > actually invalid?
> > > > > >
> > > > > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > > > > Miklos, could you provide your input on this?
> > >
> > > Yes, it makes sense to refresh attributes only when necessary.
> > >
> >
> > OK, I will add a 'compound flag' for optional operations and don't
> > execute those, when the fuse server does not support compounds.
> >
> > This way we can always send the open+getattr, but if the fuse server
> > does not process this as a compound (aka. it would be costly to do it),
> > we only resend the FUSE_OPEN. Thus the current behavior does not change
> > and we can profit from fuse servers that actually have the possibility to
> > give us the attributes.
> 
> in my opinion, this adds additional complexity for no real benefit.  I
> think we'll rarely hit a case where it'll be useful to speculatively
> prefetch attributes that are already valid that is not already
> accounted for by the attr_timeout the server set.
> 

So the current consensus would be to use the compound when we 
either don't have the data, or it has become invalid, 
and use the current behavior  
(just do an open and don't worry about the stale attributes) 
when we have unexpired attributes?

Thanks,
Horst

On Wed, Mar 4, 2026 at 1:11 AM Horst Birthelmer <horst@birthelmer.de> wrote:
>
> On Tue, Mar 03, 2026 at 01:19:43PM -0800, Joanne Koong wrote:
> > On Tue, Mar 3, 2026 at 2:39 AM Horst Birthelmer <horst@birthelmer.de> wrote:
> > >
> > > On Tue, Mar 03, 2026 at 11:03:14AM +0100, Miklos Szeredi wrote:
> > > > On Tue, 3 Mar 2026 at 06:06, Darrick J. Wong <djwong@kernel.org> wrote:
> > > > >
> > > > > On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > > > > >
> > > > > > On 3/2/26 19:56, Joanne Koong wrote:
> > > >
> > > > > > > The overhead for the server to fetch the attributes may be nontrivial
> > > > > > > (eg may require stat()). I really don't think we can assume the data
> > > > > > > is locally cached somewhere. Why always compound the getattr to the
> > > > > > > open instead of only compounding the getattr when the attributes are
> > > > > > > actually invalid?
> > > > > > >
> > > > > > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > > > > > Miklos, could you provide your input on this?
> > > >
> > > > Yes, it makes sense to refresh attributes only when necessary.
> > > >
> > >
> > > OK, I will add a 'compound flag' for optional operations and don't
> > > execute those, when the fuse server does not support compounds.
> > >
> > > This way we can always send the open+getattr, but if the fuse server
> > > does not process this as a compound (aka. it would be costly to do it),
> > > we only resend the FUSE_OPEN. Thus the current behavior does not change
> > > and we can profit from fuse servers that actually have the possibility to
> > > give us the attributes.
> >
> > in my opinion, this adds additional complexity for no real benefit.  I
> > think we'll rarely hit a case where it'll be useful to speculatively
> > prefetch attributes that are already valid that is not already
> > accounted for by the attr_timeout the server set.
> >
>
> So the current consensus would be to use the compound when we
> either don't have the data, or it has become invalid,
> and use the current behavior
> (just do an open and don't worry about the stale attributes)
> when we have unexpired attributes?

In my opinion yes that would be the best path forward. I don't think
we should be optimizing for the distributed backend stale attributes
case as it introduces additional complexity and overhead and the case
is rarely encountered. I think it's better handled by the lease idea
Miklos mentioned.

Thanks,
Joanne
>
> Thanks,
> Horst

On Mon, Mar 02, 2026 at 09:06:14PM -0800, Darrick J. Wong wrote:
> On Mon, Mar 02, 2026 at 09:03:26PM +0100, Bernd Schubert wrote:
> > On 3/2/26 19:56, Joanne Koong wrote:
> > > On Sat, Feb 28, 2026 at 12:14 AM Horst Birthelmer <horst@birthelmer.de> wrote:
> > >>
> > >> On Fri, Feb 27, 2026 at 10:07:20AM -0800, Joanne Koong wrote:
> > >>> On Fri, Feb 27, 2026 at 9:51 AM Joanne Koong <joannelkoong@gmail.com> wrote:
> > >>>>
> > >>>> On Thu, Feb 26, 2026 at 11:48 PM Horst Birthelmer <horst@birthelmer.de> wrote:
> > >>>>>
> > >>>>> On Thu, Feb 26, 2026 at 11:12:00AM -0800, Joanne Koong wrote:
> > >>>>>> On Thu, Feb 26, 2026 at 8:43 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> > >>>>>>>
> > >>>>>>> From: Horst Birthelmer <hbirthelmer@ddn.com>
> > >>>>>>>
> > >>>>>>> The discussion about compound commands in fuse was
> > >>>>>>> started over an argument to add a new operation that
> > >>>>>>> will open a file and return its attributes in the same operation.
> > >>>>>>>
> > >>>>>>> Here is a demonstration of that use case with compound commands.
> > >>>>>>>
> > >>>>>>> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > >>>>>>> ---
> > >>>>>>>  fs/fuse/file.c   | 111 +++++++++++++++++++++++++++++++++++++++++++++++--------
> > >>>>>>>  fs/fuse/fuse_i.h |   4 +-
> > >>>>>>>  fs/fuse/ioctl.c  |   2 +-
> > >>>>>>>  3 files changed, 99 insertions(+), 18 deletions(-)
> > >>>>>>>
...
> > > 
> > > The overhead for the server to fetch the attributes may be nontrivial
> > > (eg may require stat()). I really don't think we can assume the data
> > > is locally cached somewhere. Why always compound the getattr to the
> > > open instead of only compounding the getattr when the attributes are
> > > actually invalid?
> > > 
> > > But maybe I'm wrong here and this is the preferable way of doing it.
> > > Miklos, could you provide your input on this?
> > 
> > Personally I would see it as change of behavior if out of the sudden
> > open is followed by getattr. In my opinion fuse server needs to make a
> > decision that it wants that. Let's take my favorite sshfs example with a
> > 1s latency - it be very noticeable if open would get slowed down by
> > factor 2.
> 
> I wonder, since O_APPEND writes supposedly reposition the file position
> to i_size before every write, can we enlarge the write reply so that the
> fuse server could tell the client what i_size is supposed to be after
> every write?  Or perhaps add a notification so a network filesystem
> could try to keep the kernel uptodate after another node appends to a
> file?
> 

Bernd already had that idea, that we add an optional getattr to a write
in a compound.
If the fuse server supports it, you would get that data.
I'm really not happy about that idea, since we would have to support 'pages' 
then for the compound.

Regarding the notification. Isn't that doable with the current code already?

> Just my unqualified opinion ;)
> 
> --D
> 
> > Thanks,
> > Bernd

Thanks,
Horst