[PATCH v1] splice: emit a single fsnotify access event per NFSD READ

[PATCH v1] splice: emit a single fsnotify access event per NFSD READ

[Chuck Lever]

nfsd_finish_read() calls fsnotify_access() on both NFSD read paths,
because splice_direct_to_actor() does not emit the event itself. The
iterator path reaches nfsd_finish_read() from vfs_iocb_iter_read(),
which has already emitted one. A READ served without splice therefore
emits two access events.

NFSD does not use splice for the GSS integrity and privacy services, so
a READ on a sec=krb5i or sec=krb5p mount takes the iterator. An inotify
watch on that file sees the READ twice.

Reported-by: Ameer Hamza <ameer.hamza@truenas.com>
Closes: https://lore.kernel.org/linux-nfs/20260818225715.572140-1-ameer.hamza@truenas.com/
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Link: https://lore.kernel.org/linux-nfs/CAOQ4uxgUOqFv6pVmdT_+4DjA0KQoQDCzN2Xz=xm1FyumewHZXg@mail.gmail.com/
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/vfs.c          |  7 ++----
 fs/splice.c            | 56 +++++++++++++++++++++++++++++++-----------
 include/linux/splice.h |  4 +--
 3 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f9131827d391..c134064a1c79 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 {
@@ -1080,10 +1079,8 @@ __be32 nfsd_splice_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	ssize_t host_err;
 
 	trace_nfsd_read_splice(rqstp, fhp, offset, *count);
-	host_err = rw_verify_area(READ, file, &offset, *count);
-	if (!host_err)
-		host_err = splice_direct_to_actor(file, &sd,
-						  nfsd_direct_splice_actor);
+	host_err = vfs_splice_direct_to_actor(file, &sd,
+					      nfsd_direct_splice_actor);
 	return nfsd_finish_read(rqstp, fhp, file, offset, count, eof, host_err);
 }
 
diff --git a/fs/splice.c b/fs/splice.c
index 9d8f63e2fd1a..42ad712a0f4d 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1009,21 +1009,14 @@ ssize_t vfs_splice_read(struct file *in, loff_t *ppos,
 }
 EXPORT_SYMBOL_GPL(vfs_splice_read);
 
-/**
- * splice_direct_to_actor - splices data directly between two non-pipes
- * @in:		file to splice from
- * @sd:		actor information on where to splice to
- * @actor:	handles the data splicing
- *
- * Description:
- *    This is a special case helper to splice directly between two
- *    points, without requiring an explicit pipe. Internally an allocated
- *    pipe is cached in the process, and reused during the lifetime of
- *    that process.
- *
+/*
+ * This is a special case helper to splice directly between two
+ * points, without requiring an explicit pipe. Internally an allocated
+ * pipe is cached in the process, and reused during the lifetime of
+ * that process.
  */
-ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
-			       splice_direct_actor *actor)
+static ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
+				      splice_direct_actor *actor)
 {
 	struct pipe_inode_info *pipe;
 	ssize_t ret, bytes;
@@ -1147,7 +1140,40 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
 
 	goto done;
 }
-EXPORT_SYMBOL(splice_direct_to_actor);
+
+/**
+ * vfs_splice_direct_to_actor - splice from a file through a caller's actor
+ * @in:		file to splice from; must be seekable (FMODE_LSEEK)
+ * @sd:		splice parameters. The caller sets @sd->pos and
+ *		@sd->total_len. @sd->pos advances as data is consumed,
+ *		and @sd->total_len is overwritten with the length of
+ *		each read.
+ * @actor:	consumes each pipe-full and returns the number of
+ *		bytes taken
+ *
+ * Description:
+ *    Splice from @in through @actor, for a caller that consumes
+ *    the data itself rather than sending it to a second file.
+ *    This helper verifies the read and emits the fsnotify access
+ *    event. do_splice_direct() leaves both to its callers.
+ *
+ * Return: The number of bytes spliced, or a negative errno.
+ */
+ssize_t vfs_splice_direct_to_actor(struct file *in, struct splice_desc *sd,
+				   splice_direct_actor *actor)
+{
+	ssize_t ret;
+
+	ret = rw_verify_area(READ, in, &sd->pos, sd->total_len);
+	if (ret < 0)
+		return ret;
+
+	ret = splice_direct_to_actor(in, sd, actor);
+	if (ret >= 0)
+		fsnotify_access(in);
+	return ret;
+}
+EXPORT_SYMBOL(vfs_splice_direct_to_actor);
 
 static int direct_splice_actor(struct pipe_inode_info *pipe,
 			       struct splice_desc *sd)
diff --git a/include/linux/splice.h b/include/linux/splice.h
index 9dec4861d09f..4e91d03ed645 100644
--- a/include/linux/splice.h
+++ b/include/linux/splice.h
@@ -79,8 +79,8 @@ ssize_t add_to_pipe(struct pipe_inode_info *pipe, struct pipe_buffer *buf);
 ssize_t vfs_splice_read(struct file *in, loff_t *ppos,
 			struct pipe_inode_info *pipe, size_t len,
 			unsigned int flags);
-ssize_t splice_direct_to_actor(struct file *file, struct splice_desc *sd,
-			       splice_direct_actor *actor);
+ssize_t vfs_splice_direct_to_actor(struct file *file, struct splice_desc *sd,
+				   splice_direct_actor *actor);
 ssize_t do_splice(struct file *in, loff_t *off_in, struct file *out,
 		  loff_t *off_out, size_t len, unsigned int flags);
 ssize_t do_splice_direct(struct file *in, loff_t *ppos, struct file *out,
-- 
2.54.0

Re: [PATCH v1] splice: emit a single fsnotify access event per NFSD READ

[Amir Goldstein]

On Mon, Aug 24, 2026 at 6:10 PM Chuck Lever <cel@kernel.org> wrote:
>
> nfsd_finish_read() calls fsnotify_access() on both NFSD read paths,
> because splice_direct_to_actor() does not emit the event itself. The
> iterator path reaches nfsd_finish_read() from vfs_iocb_iter_read(),
> which has already emitted one. A READ served without splice therefore
> emits two access events.
>
> NFSD does not use splice for the GSS integrity and privacy services, so
> a READ on a sec=krb5i or sec=krb5p mount takes the iterator. An inotify
> watch on that file sees the READ twice.
>
> Reported-by: Ameer Hamza <ameer.hamza@truenas.com>
> Closes: https://lore.kernel.org/linux-nfs/20260818225715.572140-1-ameer.hamza@truenas.com/
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Link: https://lore.kernel.org/linux-nfs/CAOQ4uxgUOqFv6pVmdT_+4DjA0KQoQDCzN2Xz=xm1FyumewHZXg@mail.gmail.com/
> Signed-off-by: Chuck Lever <cel@kernel.org>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/nfsd/vfs.c          |  7 ++----
>  fs/splice.c            | 56 +++++++++++++++++++++++++++++++-----------
>  include/linux/splice.h |  4 +--
>  3 files changed, 45 insertions(+), 22 deletions(-)
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index f9131827d391..c134064a1c79 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 {
> @@ -1080,10 +1079,8 @@ __be32 nfsd_splice_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
>         ssize_t host_err;
>
>         trace_nfsd_read_splice(rqstp, fhp, offset, *count);
> -       host_err = rw_verify_area(READ, file, &offset, *count);
> -       if (!host_err)
> -               host_err = splice_direct_to_actor(file, &sd,
> -                                                 nfsd_direct_splice_actor);
> +       host_err = vfs_splice_direct_to_actor(file, &sd,
> +                                             nfsd_direct_splice_actor);
>         return nfsd_finish_read(rqstp, fhp, file, offset, count, eof, host_err);
>  }
>
> diff --git a/fs/splice.c b/fs/splice.c
> index 9d8f63e2fd1a..42ad712a0f4d 100644
> --- a/fs/splice.c
> +++ b/fs/splice.c
> @@ -1009,21 +1009,14 @@ ssize_t vfs_splice_read(struct file *in, loff_t *ppos,
>  }
>  EXPORT_SYMBOL_GPL(vfs_splice_read);
>
> -/**
> - * splice_direct_to_actor - splices data directly between two non-pipes
> - * @in:                file to splice from
> - * @sd:                actor information on where to splice to
> - * @actor:     handles the data splicing
> - *
> - * Description:
> - *    This is a special case helper to splice directly between two
> - *    points, without requiring an explicit pipe. Internally an allocated
> - *    pipe is cached in the process, and reused during the lifetime of
> - *    that process.
> - *
> +/*
> + * This is a special case helper to splice directly between two
> + * points, without requiring an explicit pipe. Internally an allocated
> + * pipe is cached in the process, and reused during the lifetime of
> + * that process.
>   */
> -ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
> -                              splice_direct_actor *actor)
> +static ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
> +                                     splice_direct_actor *actor)
>  {
>         struct pipe_inode_info *pipe;
>         ssize_t ret, bytes;
> @@ -1147,7 +1140,40 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
>
>         goto done;
>  }
> -EXPORT_SYMBOL(splice_direct_to_actor);
> +
> +/**
> + * vfs_splice_direct_to_actor - splice from a file through a caller's actor
> + * @in:                file to splice from; must be seekable (FMODE_LSEEK)
> + * @sd:                splice parameters. The caller sets @sd->pos and
> + *             @sd->total_len. @sd->pos advances as data is consumed,
> + *             and @sd->total_len is overwritten with the length of
> + *             each read.
> + * @actor:     consumes each pipe-full and returns the number of
> + *             bytes taken
> + *
> + * Description:
> + *    Splice from @in through @actor, for a caller that consumes
> + *    the data itself rather than sending it to a second file.
> + *    This helper verifies the read and emits the fsnotify access
> + *    event. do_splice_direct() leaves both to its callers.
> + *
> + * Return: The number of bytes spliced, or a negative errno.
> + */
> +ssize_t vfs_splice_direct_to_actor(struct file *in, struct splice_desc *sd,
> +                                  splice_direct_actor *actor)
> +{
> +       ssize_t ret;
> +
> +       ret = rw_verify_area(READ, in, &sd->pos, sd->total_len);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = splice_direct_to_actor(in, sd, actor);
> +       if (ret >= 0)
> +               fsnotify_access(in);
> +       return ret;
> +}
> +EXPORT_SYMBOL(vfs_splice_direct_to_actor);
>
>  static int direct_splice_actor(struct pipe_inode_info *pipe,
>                                struct splice_desc *sd)
> diff --git a/include/linux/splice.h b/include/linux/splice.h
> index 9dec4861d09f..4e91d03ed645 100644
> --- a/include/linux/splice.h
> +++ b/include/linux/splice.h
> @@ -79,8 +79,8 @@ ssize_t add_to_pipe(struct pipe_inode_info *pipe, struct pipe_buffer *buf);
>  ssize_t vfs_splice_read(struct file *in, loff_t *ppos,
>                         struct pipe_inode_info *pipe, size_t len,
>                         unsigned int flags);
> -ssize_t splice_direct_to_actor(struct file *file, struct splice_desc *sd,
> -                              splice_direct_actor *actor);
> +ssize_t vfs_splice_direct_to_actor(struct file *file, struct splice_desc *sd,
> +                                  splice_direct_actor *actor);
>  ssize_t do_splice(struct file *in, loff_t *off_in, struct file *out,
>                   loff_t *off_out, size_t len, unsigned int flags);
>  ssize_t do_splice_direct(struct file *in, loff_t *ppos, struct file *out,
> --
> 2.54.0
>

Re: [PATCH v1] splice: emit a single fsnotify access event per NFSD READ

[Christoph Hellwig]

On Mon, Aug 24, 2026 at 12:10:21PM -0400, Chuck Lever wrote:
> nfsd_finish_read() calls fsnotify_access() on both NFSD read paths,
> because splice_direct_to_actor() does not emit the event itself. The
> iterator path reaches nfsd_finish_read() from vfs_iocb_iter_read(),
> which has already emitted one. A READ served without splice therefore
> emits two access events.
> 
> NFSD does not use splice for the GSS integrity and privacy services, so
> a READ on a sec=krb5i or sec=krb5p mount takes the iterator. An inotify
> watch on that file sees the READ twice.
> 
> Reported-by: Ameer Hamza <ameer.hamza@truenas.com>
> Closes: https://lore.kernel.org/linux-nfs/20260818225715.572140-1-ameer.hamza@truenas.com/
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Link: https://lore.kernel.org/linux-nfs/CAOQ4uxgUOqFv6pVmdT_+4DjA0KQoQDCzN2Xz=xm1FyumewHZXg@mail.gmail.com/
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
>  fs/nfsd/vfs.c          |  7 ++----
>  fs/splice.c            | 56 +++++++++++++++++++++++++++++++-----------
>  include/linux/splice.h |  4 +--
>  3 files changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index f9131827d391..c134064a1c79 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 {
> @@ -1080,10 +1079,8 @@ __be32 nfsd_splice_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	ssize_t host_err;
>  
>  	trace_nfsd_read_splice(rqstp, fhp, offset, *count);
> -	host_err = rw_verify_area(READ, file, &offset, *count);
> -	if (!host_err)
> -		host_err = splice_direct_to_actor(file, &sd,
> -						  nfsd_direct_splice_actor);
> +	host_err = vfs_splice_direct_to_actor(file, &sd,
> +					      nfsd_direct_splice_actor);
>  	return nfsd_finish_read(rqstp, fhp, file, offset, count, eof, host_err);
>  }
>  
> diff --git a/fs/splice.c b/fs/splice.c
> index 9d8f63e2fd1a..42ad712a0f4d 100644
> --- a/fs/splice.c
> +++ b/fs/splice.c
> @@ -1009,21 +1009,14 @@ ssize_t vfs_splice_read(struct file *in, loff_t *ppos,
>  }
>  EXPORT_SYMBOL_GPL(vfs_splice_read);
>  
> -/**
> - * splice_direct_to_actor - splices data directly between two non-pipes
> - * @in:		file to splice from
> - * @sd:		actor information on where to splice to
> - * @actor:	handles the data splicing
> - *
> - * Description:
> - *    This is a special case helper to splice directly between two
> - *    points, without requiring an explicit pipe. Internally an allocated
> - *    pipe is cached in the process, and reused during the lifetime of
> - *    that process.
> - *
> +/*
> + * This is a special case helper to splice directly between two
> + * points, without requiring an explicit pipe. Internally an allocated
> + * pipe is cached in the process, and reused during the lifetime of
> + * that process.
>   */
> -ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
> -			       splice_direct_actor *actor)
> +static ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
> +				      splice_direct_actor *actor)
>  {
>  	struct pipe_inode_info *pipe;
>  	ssize_t ret, bytes;
> @@ -1147,7 +1140,40 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
>  
>  	goto done;
>  }
> -EXPORT_SYMBOL(splice_direct_to_actor);
> +
> +/**
> + * vfs_splice_direct_to_actor - splice from a file through a caller's actor

Can we use this to start retiring the really weird direct terminology
here that is confusing?  I.e. name this vfs_splice_to_actor?
With that we'll only need to fix up do_splice_direct (which is an event
worse name) to fix the public API.

> + * @in:		file to splice from; must be seekable (FMODE_LSEEK)
> + * @sd:		splice parameters. The caller sets @sd->pos and
> + *		@sd->total_len. @sd->pos advances as data is consumed,
> + *		and @sd->total_len is overwritten with the length of
> + *		each read.
> + * @actor:	consumes each pipe-full and returns the number of
> + *		bytes taken

The @ line should just have a short description of the parameter and
not explain all the details.

> + *
> + * Description:

And this Description is accepted by te parset, but redundant.

I'd rework the kerneldoc as:

/**
 * vfs_splice_to_actor - call an actor on data read from a file
 * @in:		file to read from
 * @sd:		splice parameter descriptor
 * @actor:	callback to process a pipes worth of data
 *
 * Read up to @sd->total_len worth of data from @in, and call @actor when the
 * hidden pipe used to buffer the data is full. The caller must set @sd->pos and
 * @sd->total_len. @sd->pos advances as data is consumed, and @sd->total_len is
 * overwritten with the length of data read.
 *
 * Ensures the read is allowed using rw_verify_area() and emits fsnotify access
 * events. @in must be seekable (FMODE_LSEEK).
 *
 * Return: The number of bytes spliced, or a negative errno.
 */

And, looking at the weird semantics of the splice_desc, maybe an even
better idea is to build that inside of vfs_splice_direct_to_actor
and pass in pos, len, and private_data instead?

I.e.:

/**
 * vfs_splice_to_actor - call an actor on data read from a file
 * @in:		file to read from
 * @pos:	file offset
 * @count:	maximum number of bytes to read
 * @actor:	callback to process a pipes worth of data
 * @private:	private data passed to @actor
 *
 * Read up to @count worth of data from @in at @pos, and call @actor when the
 * hidden pipe used to buffer the data is full.
 *
 * Ensures the read is allowed using rw_verify_area() and emits fsnotify access
 * events. @in must be seekable (FMODE_LSEEK).
 *
 * Return: The number of bytes spliced, or a negative errno.
 */