public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Bernd Schubert <bschubert@ddn.com>
Cc: Joanne Koong <joannelkoong@gmail.com>,
	Horst Birthelmer <horst@birthelmer.de>,
	Horst Birthelmer <horst@birthelmer.com>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Luis Henriques <luis@igalia.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Horst Birthelmer <hbirthelmer@ddn.com>
Subject: Re: [PATCH v6 3/3] fuse: add an implementation of open+getattr
Date: Mon, 2 Mar 2026 21:06:14 -0800	[thread overview]
Message-ID: <20260303050614.GO13829@frogsfrogsfrogs> (raw)
In-Reply-To: <62edc506-2b0c-4470-8bdd-ee2d7fcc1cf1@ddn.com>

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
> 
> 
> 

  reply	other threads:[~2026-03-03  5:06 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-26 16:43 [PATCH v6 0/3] fuse: compound commands Horst Birthelmer
2026-02-26 16:43 ` [PATCH v6 1/3] fuse: add compound command to combine multiple requests Horst Birthelmer
2026-02-26 23:05   ` Joanne Koong
2026-02-27  9:45   ` Miklos Szeredi
2026-02-27 10:48     ` Horst Birthelmer
2026-02-27 11:29       ` Miklos Szeredi
2026-02-27 11:37         ` Horst Birthelmer
2026-02-27 11:58           ` Miklos Szeredi
2026-03-02  9:56     ` Horst Birthelmer
2026-03-02 11:03       ` Miklos Szeredi
2026-03-02 13:19         ` Horst Birthelmer
2026-03-02 13:30           ` Miklos Szeredi
2026-03-06 14:27     ` Horst Birthelmer
2026-02-26 16:43 ` [PATCH v6 2/3] fuse: create helper functions for filling in fuse args for open and getattr Horst Birthelmer
2026-02-26 16:43 ` [PATCH v6 3/3] fuse: add an implementation of open+getattr Horst Birthelmer
2026-02-26 19:12   ` Joanne Koong
2026-02-27  7:48     ` Horst Birthelmer
2026-02-27 17:51       ` Joanne Koong
2026-02-27 18:07         ` Joanne Koong
2026-02-28  8:14           ` Horst Birthelmer
2026-03-02 18:56             ` Joanne Koong
2026-03-02 20:03               ` Bernd Schubert
2026-03-03  5:06                 ` Darrick J. Wong [this message]
2026-03-03  7:26                   ` Horst Birthelmer
2026-03-03 10:03                   ` Miklos Szeredi
2026-03-03 10:38                     ` Horst Birthelmer
2026-03-03 21:19                       ` Joanne Koong
2026-03-04  9:11                         ` Horst Birthelmer
2026-03-04 21:42                           ` Joanne Koong
2026-03-03 23:13                     ` Joanne Koong
2026-03-04  9:37                       ` Miklos Szeredi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260303050614.GO13829@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=bschubert@ddn.com \
    --cc=hbirthelmer@ddn.com \
    --cc=horst@birthelmer.com \
    --cc=horst@birthelmer.de \
    --cc=joannelkoong@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luis@igalia.com \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox