From: Christoph Hellwig <hch@infradead.org>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Chuck Lever <cel@kernel.org>,
Christoph Hellwig <hch@infradead.org>,
Ameer Hamza <ameer.hamza@truenas.com>,
Jeff Layton <jlayton@kernel.org>, NeilBrown <neil@brown.name>,
Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org,
Jan Kara <jack@suse.cz>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Christian Brauner <brauner@kernel.org>
Subject: Re: [PATCH 1/2] NFSD: Fix duplicate fsnotify access events for iterator READs
Date: Sun, 23 Aug 2026 21:57:44 -0700 [thread overview]
Message-ID: <aovPSDE_IGKggzyN@infradead.org> (raw)
In-Reply-To: <CAOQ4uxgUOqFv6pVmdT_+4DjA0KQoQDCzN2Xz=xm1FyumewHZXg@mail.gmail.com>
On Sat, Aug 22, 2026 at 12:21:12PM +0200, Amir Goldstein wrote:
> (1) In this series
> https://lore.kernel.org/linux-fsdevel/20231122122715.2561213-1-amir73il@gmail.com/
> we intentionally moved the permission hook outside
> of the splice iterators because we wanted to avoid calling them
> with freeze protection held and also there were some duplicate calls
> for this work.
This got me into a little rathole of looking into the other
do_splice_direct_actor callers. And I still don't understand why
taking file_start_write outside the main splice machinery is fine
for splice_file_range callers, but not for do_splice_direct callers,
and what consideration exists for potential new callers.
(and yes, the naming does not help)
> (2) emitting many READ events from an iterator instead of one event for
> the user's READ request is more noisy and serves no purpose to users.
> In most cases (but not always) those events could be merged, but at the
> cost of futile CPU cycles.
Yes.
> >From a quick inspection of the code, it looks like:
> - fsnotify_access() is missing in vfs_splice_read()
Yes. Then again I don't really understand vfs_splice_read, it basically
just forward ->splice_read. I guess for backing_file this is
expected and matches what do_backing_file_read_iter does. for
code it looks weird as the context doesn't change at all.
> - the naming convention for splice_ do_splice_ vfs_splice_ is a horror
The entire cascade of do_*, *actor* and the whole structure of the
splіce code is horrible unfortunately. Part of that is due to the
mess of inflicting a fake pipe for the fastpath callers that don't
need it, but paet of it is just self-inflicted bad naming.
> - we could make the low level splice_direct_to_actor() static and possibly
> rename it to splice_direct_to_actor_sd() or something
> - we could export vfs_splice_direct_to_actor() for nfsd which wraps
> splice_direct_to_actor() with permission hook and fsnotify_access
The latter is the right thing to do.
next prev parent reply other threads:[~2026-08-24 4:57 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-18 22:57 [PATCH 1/2] NFSD: Fix duplicate fsnotify access events for iterator READs Ameer Hamza
2026-08-18 22:57 ` [PATCH 2/2] NFSD: Update the atime " Ameer Hamza
2026-08-19 5:58 ` Christoph Hellwig
2026-08-19 14:27 ` Chuck Lever
2026-08-19 6:02 ` [PATCH 1/2] NFSD: Fix duplicate fsnotify access events " Christoph Hellwig
2026-08-19 14:03 ` Chuck Lever
2026-08-22 10:21 ` Amir Goldstein
2026-08-23 16:21 ` Chuck Lever
2026-08-24 8:36 ` Amir Goldstein
2026-08-24 4:57 ` Christoph Hellwig [this message]
2026-08-24 9:26 ` Amir Goldstein
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=aovPSDE_IGKggzyN@infradead.org \
--to=hch@infradead.org \
--cc=Dai.Ngo@oracle.com \
--cc=ameer.hamza@truenas.com \
--cc=amir73il@gmail.com \
--cc=brauner@kernel.org \
--cc=cel@kernel.org \
--cc=jack@suse.cz \
--cc=jlayton@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neil@brown.name \
--cc=okorniev@redhat.com \
--cc=tom@talpey.com \
/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.