mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] NFSD: Fix duplicate fsnotify access events for iterator READs
@ 2026-08-18 22:57 Ameer Hamza
  2026-08-18 22:57 ` [PATCH 2/2] NFSD: Update the atime " Ameer Hamza
  2026-08-19  6:02 ` [PATCH 1/2] NFSD: Fix duplicate fsnotify access events " Christoph Hellwig
  0 siblings, 2 replies; 6+ messages in thread
From: Ameer Hamza @ 2026-08-18 22:57 UTC (permalink / raw)
  To: cel, jlayton, neil, okorniev, Dai.Ngo, tom
  Cc: linux-nfs, linux-kernel, alexander.motin, caleb.stjohn,
	ameer.hamza, stable

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] NFSD: Update the atime for iterator READs
  2026-08-18 22:57 [PATCH 1/2] NFSD: Fix duplicate fsnotify access events for iterator READs Ameer Hamza
@ 2026-08-18 22:57 ` Ameer Hamza
  2026-08-19  5:58   ` Christoph Hellwig
  2026-08-19  6:02 ` [PATCH 1/2] NFSD: Fix duplicate fsnotify access events " Christoph Hellwig
  1 sibling, 1 reply; 6+ messages in thread
From: Ameer Hamza @ 2026-08-18 22:57 UTC (permalink / raw)
  To: cel, jlayton, neil, okorniev, Dai.Ngo, tom
  Cc: linux-nfs, linux-kernel, alexander.motin, caleb.stjohn,
	ameer.hamza, stable

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] NFSD: Update the atime for iterator READs
  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
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2026-08-19  5:58 UTC (permalink / raw)
  To: Ameer Hamza
  Cc: cel, jlayton, neil, okorniev, Dai.Ngo, tom, linux-nfs,
	linux-kernel, alexander.motin, caleb.stjohn, stable

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] NFSD: Fix duplicate fsnotify access events for iterator READs
  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  6:02 ` Christoph Hellwig
  2026-08-19 14:03   ` Chuck Lever
  1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2026-08-19  6:02 UTC (permalink / raw)
  To: Ameer Hamza
  Cc: cel, jlayton, neil, okorniev, Dai.Ngo, tom, linux-nfs,
	linux-kernel, Jan Kara, Amir Goldstein

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.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] NFSD: Fix duplicate fsnotify access events for iterator READs
  2026-08-19  6:02 ` [PATCH 1/2] NFSD: Fix duplicate fsnotify access events " Christoph Hellwig
@ 2026-08-19 14:03   ` Chuck Lever
  0 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2026-08-19 14:03 UTC (permalink / raw)
  To: Christoph Hellwig, Ameer Hamza
  Cc: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	linux-nfs, linux-kernel, Jan Kara, Amir Goldstein



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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] NFSD: Update the atime for iterator READs
  2026-08-19  5:58   ` Christoph Hellwig
@ 2026-08-19 14:27     ` Chuck Lever
  0 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2026-08-19 14:27 UTC (permalink / raw)
  To: Christoph Hellwig, Ameer Hamza
  Cc: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	linux-nfs, linux-kernel, alexander.motin, caleb.stjohn, stable


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-19 14:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®