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

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

[Ameer Hamza]

A READ served without splice emits two fsnotify access events; a
local read of the same file emits one. vfs_iocb_iter_read() emits
an access event when the read succeeds, and nfsd_finish_read()
emits a second one for the same READ. An inotify watch on an
exported file sees each of these READs twice, so anything counting
accesses counts double. READs take this path whenever nfsd does
not use splice, for example with sec=krb5i or sec=krb5p, or when
nfsd_disable_splice_read is set, as the NFSD_IO_DONTCACHE and
NFSD_IO_DIRECT modes do. Spliced READs are not affected, since
splice_direct_to_actor() emits no event and nfsd_finish_read()
emits their only one.

Move the fsnotify_access() call from nfsd_finish_read() into
nfsd_splice_read(), the one path whose VFS helper does not emit
it. Each READ now emits exactly one access event whichever path
serves it. Measured with an inotify watch: the iterator path drops
from two events per READ to one, and the splice path is unchanged
at one.

Suggested-by: Chuck Lever <cel@kernel.org>
Link: https://lore.kernel.org/linux-nfs/2ed0ee16-c20e-44bd-9e7a-f5b71764f14f@app.fastmail.com/
Fixes: 507df40ebf316 ("NFSD: Hoist rq_vec preparation into nfsd_read()")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Ameer Hamza <ameer.hamza@truenas.com>
---
 fs/nfsd/vfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f9131827d391e..f45d4ad70b964 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1046,7 +1046,6 @@ static __be32 nfsd_finish_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		nfsd_stats_io_read_add(nn, fhp->fh_export, host_err);
 		*eof = nfsd_eof_on_read(file, offset, host_err, *count);
 		*count = host_err;
-		fsnotify_access(file);
 		trace_nfsd_read_io_done(rqstp, fhp, offset, *count);
 		return 0;
 	} else {
@@ -1084,6 +1083,9 @@ __be32 nfsd_splice_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	if (!host_err)
 		host_err = splice_direct_to_actor(file, &sd,
 						  nfsd_direct_splice_actor);
+	/* splice_direct_to_actor() does not emit an fsnotify event */
+	if (host_err >= 0)
+		fsnotify_access(file);
 	return nfsd_finish_read(rqstp, fhp, file, offset, count, eof, host_err);
 }
 

base-commit: c5f58d03c50196301ac2ce7da81e8be33eba57c6
-- 
2.53.0

[PATCH 2/2] NFSD: Update the atime for iterator READs

[Ameer Hamza]

A READ served by nfsd_iter_read() or nfsd_direct_read() leaves the
atime update to the filesystem's ->read_iter, and not every
implementation does it: fuse_dax_read_iter() carries a TODO for
it, and kernfs_fop_read_iter() does not touch the atime at all. On
a fuse DAX export, READs never advance the atime, no matter how
often clients read the file. Spliced READs are not affected, since
splice_direct_to_actor() ends with file_accessed(). nfsd serves
whatever filesystem is exported, so it cannot rely on every
->read_iter keeping the convention.

Call file_accessed() after each successful vfs_iocb_iter_read().

Reported-by: Chuck Lever <cel@kernel.org>
Closes: https://lore.kernel.org/linux-nfs/2ed0ee16-c20e-44bd-9e7a-f5b71764f14f@app.fastmail.com/
Fixes: 507df40ebf316 ("NFSD: Hoist rq_vec preparation into nfsd_read()")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Ameer Hamza <ameer.hamza@truenas.com>
---
 fs/nfsd/vfs.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f45d4ad70b964..f017729cc1be0 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1144,6 +1144,9 @@ nfsd_direct_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	if (host_err >= 0) {
 		unsigned int pad = offset - dio_start;
 
+		/* Not every ->read_iter implementation updates the atime */
+		file_accessed(nf->nf_file);
+
 		/* The returned payload starts after the pad */
 		rqstp->rq_res.page_base = pad;
 
@@ -1229,6 +1232,9 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	trace_nfsd_read_vector(rqstp, fhp, offset, *count - total);
 	iov_iter_bvec(&iter, ITER_DEST, rqstp->rq_bvec, v, *count - total);
 	host_err = vfs_iocb_iter_read(file, &kiocb, &iter);
+	/* Not every ->read_iter implementation updates the atime */
+	if (host_err >= 0)
+		file_accessed(file);
 	return nfsd_finish_read(rqstp, fhp, file, offset, count, eof, host_err);
 }
 
-- 
2.53.0

Re: [PATCH 2/2] NFSD: Update the atime for iterator READs

[Christoph Hellwig]

On Wed, Aug 19, 2026 at 03:57:15AM +0500, Ameer Hamza wrote:
> A READ served by nfsd_iter_read() or nfsd_direct_read() leaves the
> atime update to the filesystem's ->read_iter, and not every
> implementation does it: fuse_dax_read_iter() carries a TODO for
> it, and kernfs_fop_read_iter() does not touch the atime at all. On
> a fuse DAX export, READs never advance the atime, no matter how
> often clients read the file. Spliced READs are not affected, since
> splice_direct_to_actor() ends with file_accessed(). nfsd serves
> whatever filesystem is exported, so it cannot rely on every
> ->read_iter keeping the convention.

Code outside of file systems and library code has absolutely no
business ever calling file_accessed.

And please stop this LLM garbage.  Your patches seem to have a pattern
of finding some unusual corner case that is broken in a file system
and than work around it in core code.  That's not how it works.

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

[Christoph Hellwig]

On Wed, Aug 19, 2026 at 03:57:14AM +0500, Ameer Hamza wrote:
> NFSD_IO_DIRECT modes do. Spliced READs are not affected, since
> splice_direct_to_actor() emits no event and nfsd_finish_read()
> emits their only one.

Well, that is the underlying bug here.  ->splice_read should not
skip fsnotify events and nfsd should not work around this as
fsnotify is not the business of the users of VFS APIs.

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

[Chuck Lever]

On Wed, Aug 19, 2026, at 2:02 AM, Christoph Hellwig wrote:
> Well, that is the underlying bug here.  ->splice_read should not                                                                       
> skip fsnotify events and nfsd should not work around this as
> fsnotify is not the business of the users of VFS APIs.

I asked for the duplicate event to be split out of Ameer's larger
series as a backportable fix because there is clearly a bug here.
But NFSD might not be the correct place to address it.

Today the fsnotify event comes from the system call implementations,
not from the splice helpers. do_sendfile() calls fsnotify_access()
once do_splice_direct() returns, and do_splice() does the same for
splice(2), while vfs_splice_read() and splice_direct_to_actor()
emit nothing. vfs_iocb_iter_read() is the outlier, emitting from
inside the helper.

NFSD calls splice_direct_to_actor() directly, so on that path it
acts like do_sendfile() and emits the event itself. That is why
the fsnotify event counts differ between NFSD's two read paths.
Moving the fsnotify call site down into ->splice_read would double
up sendfile events unless the system call implementations stop
emitting it at the same time.

Jan, Amir, what are your thoughts?

-- 
Chuck Lever

Re: [PATCH 2/2] NFSD: Update the atime for iterator READs

[Chuck Lever]

On Wed, Aug 19, 2026, at 1:58 AM, Christoph Hellwig wrote:
> Code outside of file systems and library code has absolutely no
> business ever calling file_accessed.

Every call site in the kernel is a filesystem, so indeed, NFSD would
have been the first call site outside a filesystem.

I misread the FUSE code: I see now that FUSE does not maintain atime
in the kernel. file_accessed() from NFSD would have stamped a kernel-
side atime on an inode whose atime the user space filesystem manages.

I assumed FamFS would hit this very issue, but maybe it does not.

The TODO in fuse_dax_read_iter() is a bit confusing. Implementing
that should be a one-liner, or it should be replaced with a comment
that explains why that callback is missing an explicit atime update.

So we can drop 2/2. Sorry for the noise.


-- 
Chuck Lever

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

[Amir Goldstein]

On Wed, Aug 19, 2026 at 4:04 PM Chuck Lever <cel@kernel.org> wrote:
>
>
>
> On Wed, Aug 19, 2026, at 2:02 AM, Christoph Hellwig wrote:
> > Well, that is the underlying bug here.  ->splice_read should not
> > skip fsnotify events and nfsd should not work around this as
> > fsnotify is not the business of the users of VFS APIs.
>
> I asked for the duplicate event to be split out of Ameer's larger
> series as a backportable fix because there is clearly a bug here.
> But NFSD might not be the correct place to address it.
>
> Today the fsnotify event comes from the system call implementations,
> not from the splice helpers. do_sendfile() calls fsnotify_access()
> once do_splice_direct() returns, and do_splice() does the same for
> splice(2), while vfs_splice_read() and splice_direct_to_actor()
> emit nothing. vfs_iocb_iter_read() is the outlier, emitting from
> inside the helper.
>
> NFSD calls splice_direct_to_actor() directly, so on that path it
> acts like do_sendfile() and emits the event itself. That is why
> the fsnotify event counts differ between NFSD's two read paths.
> Moving the fsnotify call site down into ->splice_read would double
> up sendfile events unless the system call implementations stop
> emitting it at the same time.
>
> Jan, Amir, what are your thoughts?
>

Here are some thoughts and points for consideration.

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

At this point in time, the fsnotify_{access,modify} post hooks are
usually called
from the same context as the matching permission/security hooks.
It doesn't have to be this way, but it's a good mental model IMO.

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

From a quick inspection of the code, it looks like:
- fsnotify_access() is missing in vfs_splice_read()
- the naming convention for splice_ do_splice_ vfs_splice_ is a horror
- 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

Whether or not this is prettier than nfsd adding the fsnotify hooks
is a matter of taste, but I can live with either.

Thanks,
Amir.

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

[Chuck Lever]

On Sat, Aug 22, 2026, at 6:21 AM, 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.
>
> At this point in time, the fsnotify_{access,modify} post hooks are
> usually called
> from the same context as the matching permission/security hooks.
> It doesn't have to be this way, but it's a good mental model IMO.
>
> (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.
>
> From a quick inspection of the code, it looks like:
> - fsnotify_access() is missing in vfs_splice_read()
> - the naming convention for splice_ do_splice_ vfs_splice_ is a horror
> - 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

IIUC this last bullet seems like clean layering to me. Do
you want to propose a patch or shall I?


-- 
Chuck Lever

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

[Christoph Hellwig]

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.

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

[Amir Goldstein]

On Sun, Aug 23, 2026 at 6:21 PM Chuck Lever <cel@kernel.org> wrote:
>
>
>
> On Sat, Aug 22, 2026, at 6:21 AM, 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.
> >
> > At this point in time, the fsnotify_{access,modify} post hooks are
> > usually called
> > from the same context as the matching permission/security hooks.
> > It doesn't have to be this way, but it's a good mental model IMO.
> >
> > (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.
> >
> > From a quick inspection of the code, it looks like:
> > - fsnotify_access() is missing in vfs_splice_read()
> > - the naming convention for splice_ do_splice_ vfs_splice_ is a horror
> > - 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
>
> IIUC this last bullet seems like clean layering to me. Do
> you want to propose a patch or shall I?

Be my guest.
Maybe Ameer will want to post it for v2.

Thanks,
Amir.

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

[Amir Goldstein]

On Mon, Aug 24, 2026 at 6:57 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> 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.

Tough question.
If I can retrace my steps:
The main consideration in the "Tidy up file permission hooks"
work was to move permission hooks outside of sb_start_write()
to avoid "first order deadlocks" from pre-content events (and LSMs),
but I think we also tried best effort to avoid holding
file_start_write(out) while
performing read on file in, to avoid "second order deadlocks" with
weird setups like:
https://lore.kernel.org/linux-fsdevel/5lz5jq7gzoejbywmh56ayfkdiuqsjd2s5pl5uvlflfxc5lq4rr@thr4hrkw67d2/

For the first order deadlocks, both flavors are fine:
1. permission + file_start_write() in ceph_copy_file_range() +
    splice_file_range()
2. permission + do_splice_direct() in vfs_copy_file_range()

->copy_file_range() are called from vfs_copy_file_range() with
file_start_write() held so ceph_copy_file_range() needs to use
the first flavor.

But at least it's holding file_start_write() on ceph fs while reading
a file from cephfs (maybe not the same sb though).

The use cases of do_splice_direct() from ovl copy up
nfsd/ksmbd copy_file_range (*) have more potential of hitting the
second order deadlocks, so they try to avoid them with more
granular file_start_write().

TBH, I don't think that we proved to what extent this helps avoid
the second order deadlocks because obviously, those deadlocks
are still possible.

(*) In the past copy_file_range(2) across different fs was also
a use case, but we stopped supporting that use case.

Anyway, there could definitely be other reasons for the
do_splice_direct/splice_file_range split which I do not remember,
but hey, at least the kerneldoc for these helpers is pretty clear...

Thanks,
Amir.