Re: [PATCH 1/2] NFSD: Fix duplicate fsnotify access events for iterator READs

Christoph Hellwig <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-nfs
Message-ID <[email protected]>
On Sat, Aug 22, 2026 at 12:21:12PM +0200, Amir Goldstein wrote:
> (1) In this series
> https://lore.kernel.org/linux-fsdevel/[email protected]/
> 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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.