All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.